Skip to content

Fix Authorization header caching in PublicCacheStrategy according to HTTP Caching RFC 9111#200

Merged
Kevinrob merged 4 commits into
masterfrom
copilot/fix-1f5a7972-0c13-4962-9c42-2ba50c38e376
Aug 11, 2025
Merged

Fix Authorization header caching in PublicCacheStrategy according to HTTP Caching RFC 9111#200
Kevinrob merged 4 commits into
masterfrom
copilot/fix-1f5a7972-0c13-4962-9c42-2ba50c38e376

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 27, 2025

Problem

According to HTTP Caching RFC 9111 Section 3.5, shared caches should not cache requests containing Authorization header unless specific cache directives explicitly allow it. The restriction applies to shared caches (PublicCacheStrategy), not private caches (PrivateCacheStrategy).

The original implementation incorrectly applied this restriction to private caches, when it should only apply to shared caches.

Solution

Implemented RFC 9111 Section 3.5 compliance in PublicCacheStrategy (shared cache) while preserving normal caching behavior in PrivateCacheStrategy (private cache):

  • PublicCacheStrategy: Added Authorization header checks - only caches authenticated requests when response contains public, must-revalidate, or s-maxage directives
  • PrivateCacheStrategy: Maintains normal caching behavior for authenticated requests (RFC compliant)

Changes

  • src/Strategy/PublicCacheStrategy.php: Added RFC 9111 Section 3.5 compliance check for shared caches
  • tests/AuthorizationCacheTest.php: Comprehensive test suite covering both strategies with proper distinction

Test Coverage

PublicCacheStrategy + Authorization + max-age only → NOT cached (RFC compliant)
PublicCacheStrategy + Authorization + public/must-revalidate/s-maxage → Cached
PrivateCacheStrategy + Authorization → Normal caching behavior (allowed)
Edge cases: no-store precedence, case sensitivity, multiple directives

Backward Compatibility

All existing caching behavior is preserved. This correctly implements RFC 9111 shared cache rules without breaking changes.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: Kevinrob <4509277+Kevinrob@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix Authorization header caching according to HTTP Caching RFC Fix Authorization header caching according to HTTP Caching RFC 9111 Jun 27, 2025
Copilot AI requested a review from Kevinrob June 27, 2025 07:40
@Kevinrob
Copy link
Copy Markdown
Owner

Kevinrob commented Aug 3, 2025

@copilot You need to make changes to PublicCacheStrategy (shared cache), not PrivateCacheStrategy (private cache)

…PublicCacheStrategy per RFC 9111

Co-authored-by: Kevinrob <4509277+Kevinrob@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Aug 3, 2025

@copilot You need to make changes to PublicCacheStrategy (shared cache), not PrivateCacheStrategy (private cache)

You're absolutely right! I've moved the Authorization header caching logic from PrivateCacheStrategy to PublicCacheStrategy. The RFC 9111 Section 3.5 restrictions apply to shared caches (PublicCacheStrategy), not private caches (PrivateCacheStrategy).

Changes made in commit f4d4720:

  • Reverted PrivateCacheStrategy to original behavior (allows caching authenticated requests)
  • Added Authorization header restrictions to PublicCacheStrategy for shared cache compliance
  • Updated all tests to use PublicCacheStrategy for Authorization header validation
  • Added test confirming PrivateCacheStrategy still works normally with Authorization headers

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/guzzle/promises/zipball/7c69f28996b0a6920945dd20b3857e499d9ca96c
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/xoqleI /usr/bin/composer install --no-dev (http block)
  • https://api.github.com/repos/php-fig/http-factory/zipball/2b4765fddfe3b508ac62f829e852b1501d3f6e8a
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/xoqleI /usr/bin/composer install --no-dev (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Fix Authorization header caching according to HTTP Caching RFC 9111 Fix Authorization header caching in PublicCacheStrategy according to HTTP Caching RFC 9111 Aug 3, 2025
@Kevinrob Kevinrob marked this pull request as ready for review August 3, 2025 08:45
@Kevinrob Kevinrob requested a review from Copilot August 11, 2025 09:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements RFC 9111 Section 3.5 compliance for HTTP caching behavior regarding Authorization headers. The changes ensure that shared caches (PublicCacheStrategy) properly restrict caching of authenticated requests unless explicitly allowed by specific cache directives, while preserving normal caching behavior for private caches (PrivateCacheStrategy).

  • Added Authorization header validation in PublicCacheStrategy that only allows caching when response contains public, must-revalidate, or s-maxage directives
  • Created comprehensive test suite covering both public and private cache strategies with various Authorization header scenarios
  • Maintained backward compatibility by preserving existing caching behavior for non-authenticated requests

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Strategy/PublicCacheStrategy.php Implements RFC 9111 Section 3.5 Authorization header caching restrictions for shared caches
tests/AuthorizationCacheTest.php Comprehensive test coverage for Authorization header caching behavior across both cache strategies

@Kevinrob Kevinrob merged commit c7012ce into master Aug 11, 2025
16 checks passed
@Kevinrob Kevinrob deleted the copilot/fix-1f5a7972-0c13-4962-9c42-2ba50c38e376 branch August 11, 2025 09:27
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.

Requests with Authorization header should not be cached according to HTTP Caching RFC

3 participants