Skip to content

[HttpKernel] [HttpCache] AbstractSurrogate doesn't treat 304 response as successful #43997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
aleho opened this issue Nov 10, 2021 · 2 comments
Closed

[HttpKernel] [HttpCache] AbstractSurrogate doesn't treat 304 response as successful #43997

aleho opened this issue Nov 10, 2021 · 2 comments
Labels
Bug HttpKernel Status: Needs Review

Comments

@aleho
Copy link
Contributor

aleho commented Nov 10, 2021

Symfony version(s) affected

4.4+

Description

When HttpCache is allowed to preserve Last-Modified (see #42355) clients will execute subsequent requests with If-Modified-Since. If cache then returns a 304 Not Modified this will result in an exception.

This cycle will continue:

  • Request: initial
  • Response: sucessful, cache store, Last-Modified header
  • Request: with If-Modified-Since
  • Response: 500 (client drops cache), error page
  • Request: initial without timestamp
  • Response: sucessful
  • …

How to reproduce

Merge PR #42355 or check out https://github.com/aleho/symfony-httpcache for a minimal demo.

Possible Solution

No response

Additional Context

No response
@aleho aleho added the Bug label Nov 10, 2021
@aleho aleho mentioned this issue Nov 10, 2021
Merged
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@aleho
Copy link
Contributor Author

aleho commented May 11, 2022

Yes, this is still relevant.

@carsonbot carsonbot removed the Stalled label May 11, 2022
@fabpot fabpot closed this as completed Jul 20, 2022
fabpot added a commit that referenced this issue Jul 20, 2022
@fabpot
bug #43998 [HttpKernel] [HttpCache] Don't throw on 304 Not Modified (…
fa063a1 
…aleho)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpKernel] [HttpCache] Don't throw on 304 Not Modified

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fixes  #43997
| License       | MIT
| Doc PR        | ~

If the response cache keeps a `Last-Modified` header clients will request with `If-Modified-Since`.
The surrogate will not handle a `304 Not Modified` correctly, resulting in a 500 and a failed request.

This fixes that request / response cycle, as observed in testing PR #42355.

Commits
-------

d27f02a [HttpKernel] [HttpCache] Don't throw on 304 Not Modified
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpKernel Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fabpot @aleho @carsonbot