Skip to content

bz69439 - fix: improper handling of conflicting cache-control directives#777

Closed
Chenjp wants to merge 1 commit into
apache:mainfrom
Chenjp:bz69439
Closed

bz69439 - fix: improper handling of conflicting cache-control directives#777
Chenjp wants to merge 1 commit into
apache:mainfrom
Chenjp:bz69439

Conversation

@Chenjp

@Chenjp Chenjp commented Nov 12, 2024

Copy link
Copy Markdown
Contributor

Currently ExpiresFilter pickup the first-added cache-control header as the effective field, cause follow-up directives (like no-store or max-age) to be ignored.

rfc9111 - 4.2.1 - If directives conflict (e.g., both max-age and no-cache are present), the most restrictive directive should be honored.

  • remove cacheControlHeader from servlet response wrapper ExpiresFilter.XHttpServletResponse.
  • go through all cache-control fields when verify add-expire-header possibility.
  • test case added.

rfc9111 - 4.2.1 - If directives conflict (e.g., both max-age and no-cache are present), the most restrictive directive should be honored.
markt-asf added a commit that referenced this pull request Nov 20, 2024
@markt-asf

Copy link
Copy Markdown
Contributor

Thanks for the PR. I have applied a fix based on this PR.

Removing the cache-control header caching removes a deliberate performance optimisation (and doesn't update the Javadoc to reflect that change either). I opted for the simpler approach of merging multiple headers into a single header in the cache. I also slightly changed the test cases.

@markt-asf markt-asf closed this Nov 20, 2024
markt-asf added a commit that referenced this pull request Nov 20, 2024
markt-asf added a commit that referenced this pull request Nov 20, 2024
markt-asf added a commit that referenced this pull request Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants