Skip to content

Commit 512c8de

Browse files
committed
caddyhttp: keep absent optional placeholders empty
1 parent 56e3a88 commit 512c8de

3 files changed

Lines changed: 102 additions & 9 deletions

File tree

modules/caddyhttp/replacer.go

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.Respo
8787
return cookie.Value, true
8888
}
8989
}
90+
return "", true
9091
}
9192

9293
// http.request.tls.*
@@ -417,18 +418,31 @@ func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.Respo
417418
}
418419

419420
func getReqTLSReplacement(req *http.Request, key string) (any, bool) {
420-
if req == nil || req.TLS == nil {
421+
if req == nil {
421422
return nil, false
422423
}
424+
state := req.TLS
425+
if state == nil {
426+
state = new(tls.ConnectionState)
427+
}
428+
value, known := getTLSReplacement(state, key)
429+
if req.TLS == nil {
430+
// Use the same field parser to recognise valid placeholders but
431+
// do not substitute values from the empty state for a plain HTTP request.
432+
return nil, known
433+
}
434+
return value, known
435+
}
423436

437+
func getTLSReplacement(state *tls.ConnectionState, key string) (any, bool) {
424438
if len(key) < len(reqTLSReplPrefix) {
425439
return nil, false
426440
}
427441

428442
field := strings.ToLower(key[len(reqTLSReplPrefix):])
429443

430444
if strings.HasPrefix(field, "client.") {
431-
cert := getTLSPeerCert(req.TLS)
445+
cert := getTLSPeerCert(state)
432446
if cert == nil {
433447
// Instead of returning (nil, false) here, we set it to a dummy
434448
// value to fix #7530. This way, even if there is no client cert,
@@ -533,20 +547,20 @@ func getReqTLSReplacement(req *http.Request, key string) (any, bool) {
533547

534548
switch field {
535549
case "version":
536-
return caddytls.ProtocolName(req.TLS.Version), true
550+
return caddytls.ProtocolName(state.Version), true
537551
case "cipher_suite":
538-
return tls.CipherSuiteName(req.TLS.CipherSuite), true
552+
return tls.CipherSuiteName(state.CipherSuite), true
539553
case "resumed":
540-
return req.TLS.DidResume, true
554+
return state.DidResume, true
541555
case "proto":
542-
return req.TLS.NegotiatedProtocol, true
556+
return state.NegotiatedProtocol, true
543557
case "proto_mutual":
544558
// req.TLS.NegotiatedProtocolIsMutual is deprecated - it's always true.
545559
return true, true
546560
case "server_name":
547-
return req.TLS.ServerName, true
561+
return state.ServerName, true
548562
case "ech":
549-
return req.TLS.ECHAccepted, true
563+
return state.ECHAccepted, true
550564
}
551565
return nil, false
552566
}

modules/caddyhttp/replacer_test.go

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,62 @@ import (
2727
"github.com/caddyserver/caddy/v2"
2828
)
2929

30+
func TestMissingCookiePlaceholder(t *testing.T) {
31+
req := httptest.NewRequest(http.MethodGet, "/", nil)
32+
req = req.WithContext(context.WithValue(req.Context(), VarsCtxKey, map[string]any{}))
33+
repl := NewTestReplacer(req)
34+
const input = "before-{http.request.cookie.session}-after"
35+
if got := repl.ReplaceKnown(input, ""); got != "before--after" {
36+
t.Fatalf("missing cookie = %q, want %q", got, "before--after")
37+
}
38+
req.AddCookie(&http.Cookie{Name: "session", Value: "present"})
39+
if got := repl.ReplaceKnown(input, ""); got != "before-present-after" {
40+
t.Fatalf("present cookie = %q, want %q", got, "before-present-after")
41+
}
42+
}
43+
44+
func TestTLSPlaceholdersWithoutTLS(t *testing.T) {
45+
req := httptest.NewRequest(http.MethodGet, "/", nil)
46+
req = req.WithContext(context.WithValue(req.Context(), VarsCtxKey, map[string]any{}))
47+
repl := NewTestReplacer(req)
48+
for _, field := range []string{
49+
"version", "cipher_suite", "resumed", "proto", "proto_mutual", "server_name", "ech",
50+
"client.fingerprint", "client.public_key", "client.public_key_sha256",
51+
"client.issuer", "client.serial", "client.subject", "client.certificate_pem", "client.certificate_der_base64",
52+
"client.san.dns_names", "client.san.emails", "client.san.ips", "client.san.uris",
53+
"client.san.dns_names.0", "client.san.emails.0", "client.san.ips.0", "client.san.uris.0",
54+
} {
55+
t.Run(field, func(t *testing.T) {
56+
key := "http.request.tls." + field
57+
value, known := repl.Get(key)
58+
if !known || caddy.ToString(value) != "" {
59+
t.Fatalf("Get(%q) = %v, %v; want empty, known", key, value, known)
60+
}
61+
if got := repl.ReplaceKnown("before-{"+key+"}-after", ""); got != "before--after" {
62+
t.Fatalf("replacement = %q, want %q", got, "before--after")
63+
}
64+
})
65+
}
66+
}
67+
68+
func TestUnknownTLSPlaceholdersRemainLiteral(t *testing.T) {
69+
for _, state := range []*tls.ConnectionState{nil, {ServerName: "example.com"}} {
70+
req := httptest.NewRequest(http.MethodGet, "/", nil)
71+
req.TLS = state
72+
req = req.WithContext(context.WithValue(req.Context(), VarsCtxKey, map[string]any{}))
73+
repl := NewTestReplacer(req)
74+
for _, field := range []string{
75+
"unknown", "client.unknown", "client.san.unknown",
76+
"client.san.dns_names_extra", "client.san.dns_names.-1", "client.san.dns_names.nope",
77+
} {
78+
input := "{http.request.tls." + field + "}"
79+
if got := repl.ReplaceKnown(input, ""); got != input {
80+
t.Errorf("replacement = %q, want literal %q", got, input)
81+
}
82+
}
83+
}
84+
}
85+
3086
func TestHTTPVarReplacement(t *testing.T) {
3187
req, _ := http.NewRequest(http.MethodGet, "/foo/bar.tar.gz?a=1&b=2", nil)
3288
repl := caddy.NewReplacer()
@@ -334,4 +390,3 @@ func TestHTTPVarReplacementUUID(t *testing.T) {
334390
t.Errorf("expected stable uuid across references: %q != %q", first, second)
335391
}
336392
}
337-

modules/caddyhttp/staticresp_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,30 @@ func fakeRequest() *http.Request {
6565
return r
6666
}
6767

68+
func TestStaticResponseHeadersWithAbsentRequestData(t *testing.T) {
69+
req := httptest.NewRequest(http.MethodGet, "/", nil)
70+
req = req.WithContext(context.WithValue(req.Context(), VarsCtxKey, map[string]any{}))
71+
NewTestReplacer(req)
72+
response := StaticResponse{Headers: http.Header{
73+
"Location": []string{"/login?session={http.request.cookie.session}"},
74+
"X-Tls": []string{"before-{http.request.tls.server_name}-after"},
75+
"X-Unknown": []string{"before-{unknown}-after"},
76+
}}
77+
w := httptest.NewRecorder()
78+
if err := response.ServeHTTP(w, req, nil); err != nil {
79+
t.Fatal(err)
80+
}
81+
for field, want := range map[string]string{
82+
"Location": "/login?session=",
83+
"X-Tls": "before--after",
84+
"X-Unknown": "before-{unknown}-after",
85+
} {
86+
if got := w.Header().Get(field); got != want {
87+
t.Errorf("%s = %q, want %q", field, got, want)
88+
}
89+
}
90+
}
91+
6892
func TestStaticResponseHeadersKeepUnknownPlaceholders(t *testing.T) {
6993
r := fakeRequest()
7094
w := httptest.NewRecorder()

0 commit comments

Comments
 (0)