Skip to content

Conversation

@bneradt
Copy link
Contributor

@bneradt bneradt commented Nov 18, 2025

Detect and ignore Cache-Control directives that are malformed (e.g.,
using semicolons instead of commas). When the parser cannot fully
consume a Cache-Control directive value, leaving unparsed non-whitespace
content, the directive is treated as unrecognized and ignored per RFC
7234 Section 5.2.

For example, a malformed header such as 'public; max-age=30' (note the
incorrect semicolon separator) would previously set the 'public' bit but
ignore the unparsed 'max-age=30', causing the response to be cached with
incorrect heuristic lifetimes. Not only is this incorrect per the RFC,
but the intended max-age was, from the user's perspective, mysteriously
ignored. Now the entire malformed directive is ignored, allowing default
caching rules to apply correctly.

Fixes: #12029

@bneradt bneradt added this to the 10.2.0 milestone Nov 18, 2025
@bneradt bneradt self-assigned this Nov 18, 2025
@bneradt bneradt force-pushed the fix_malformed_cc_processing branch from 1590274 to aacd81e Compare November 18, 2025 18:13
@bneradt bneradt changed the title Reject caching for malformed Cache-Control headers Ignore malformed Cache-Control directives per RFC 7234 Nov 18, 2025
@bneradt bneradt force-pushed the fix_malformed_cc_processing branch 4 times, most recently from 882f78f to 26748d1 Compare November 24, 2025 20:35
@mlibbey
Copy link
Contributor

mlibbey commented Nov 24, 2025

Not to prevent this one ... but is it worth doing a quick look at
https://cache-tests.fyi/#cc-parse
to see if there are other quick wins nearby?
For instance, I think the current web page is saying ATS isn't handling

  • Does HTTP cache ignore max-age with space before the =?
  • Does HTTP cache ignore max-age with space after the =?
  • HTTP cache must not reuse a response with a single-quoted Cache-Control: max-age

 
 

@bryancall bryancall requested a review from zwoop December 1, 2025 23:19
Detect and ignore Cache-Control directives that are malformed (e.g.,
using semicolons instead of commas). When the parser cannot fully
consume a Cache-Control directive value, leaving unparsed non-whitespace
content, the directive is treated as unrecognized and ignored per RFC
7234 Section 5.2.

For example, a malformed header such as 'public; max-age=30' (note the
incorrect semicolon separator) would previously set the 'public' bit but
ignore the unparsed 'max-age=30', causing the response to be cached with
incorrect heuristic lifetimes. Not only is this incorrect per the RFC,
but the intended max-age was, from the user's perspective, mysteriously
ignored. Now the entire malformed directive is ignored, allowing default
caching rules to apply correctly.

Fixes: apache#12029
@bneradt bneradt force-pushed the fix_malformed_cc_processing branch from 26748d1 to 5daad76 Compare December 4, 2025 18:24
@bneradt
Copy link
Contributor Author

bneradt commented Dec 4, 2025

Not to prevent this one ... but is it worth doing a quick look at https://cache-tests.fyi/#cc-parse to see if there are other quick wins nearby? For instance, I think the current web page is saying ATS isn't handling

  • Does HTTP cache ignore max-age with space before the =?
  • Does HTTP cache ignore max-age with space after the =?
  • HTTP cache must not reuse a response with a single-quoted Cache-Control: max-age

Thank you @mlibbey . I updated the code for these conditions as well and beefed up the tests for it too.

@bneradt bneradt force-pushed the fix_malformed_cc_processing branch from 5daad76 to 11d2a6e Compare December 4, 2025 18:37
Copy link
Contributor

@zwoop zwoop left a comment

Choose a reason for hiding this comment

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

Looks good. Once landed, please do a cache-test comparison before / after this PR (compare master to 10.1 I guess?) and see that we only have fixes and no new defects.

}
}

// Detect whether there is any more non-whitespace content after the
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels little heavy on the comments, but oh well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stale negative responses returned under unknown conditions

3 participants