Skip to content

headers: Check each ? default on its own and keep the response matcher - #8217

Open
jayhemnani9910 wants to merge 4 commits into
caddyserver:masterfrom
jayhemnani9910:fix-header-default-per-field
Open

jayhemnani9910 wants to merge 4 commits into
caddyserver:masterfrom
jayhemnani9910:fix-header-default-per-field

Conversation

@jayhemnani9910

Copy link
Copy Markdown
Contributor

Closes #8216

In a header block, all ? fields shared one require, so one field already on the response stopped every default in the block. A match in the block was also left off the ? fields, so they were set on every response.

Each ? field now gets its own handler that requires only that field to be absent, plus the match conditions when there are any. A block with a single ? field adapts to the same JSON as before, so the existing adapt tests are unchanged.

Test: TestHeaderDefaults sets Cache-Control first, then a block with ?Cache-Control and ?X-Frame-Options, and a match status 404 block with ?X-Robots-Tag. Without the change X-Frame-Options is missing and X-Robots-Tag is sent on a 200.

go test ./... passes.

Manual check with the config from the issue: v2.11.7 sends no X-Frame-Options on :18432 and X-Robots-Tag on the 200 from :18434; with the change X-Frame-Options: DENY is there and the 200 has no X-Robots-Tag.

Assistance Disclosure

AI used (Claude Code).

Several ? defaults in one header block shared one require, so a single
field already set on the response blocked all of them. A match in the
block was also dropped for the ? fields, so they were set on every
response.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@steadytao steadytao added the bug 🐞 Something isn't working label Oct 14, 2026
@steadytao steadytao added this to the v2.12.0 milestone Oct 14, 2026
Comment thread modules/caddyhttp/headers/caddyfile.go
Comment thread modules/caddyhttp/headers/caddyfile.go
A ? default already requires its field to be absent, so a match on the
same field in that block could only be dropped or never be true. It is
now a Caddyfile error.
Comment on lines +151 to +153
if _, ok := defaultHandler.Response.Set[http.CanonicalHeaderKey(field)]; ok {
return nil, h.Errf("cannot match on response header %s and also set a default for it", http.CanonicalHeaderKey(field))
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also rejects match header !X-Test with ?X-Test fallback. A nil response-matcher value already requires the field to be absent so it is compatible with the default and previously adapted successfully.

Only reject same-field matchers that require a value. Preserve or deduplicate the absence matcher and add that case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 13d7837. A !X-Test matcher next to ?X-Test loads again and is not added twice. Only a matcher that needs a value on that field is rejected. Added the !X-Absent / ?X-Absent case to TestHeaderDefaults, it fails on the previous commit.

Assistance Disclosure: AI used (Claude Code).

`match header !X-Test` asks for what `?X-Test` already requires, so it
loads again. Only a matcher that needs a value on that field is an error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🐞 Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

header: several ? defaults in one block are all skipped when any one field is set, and match is ignored for them

3 participants