Skip to content

HTTP/1.1: improve handling of malformed 304 responses#2400

Open
yadij wants to merge 4 commits intosquid-cache:masterfrom
yadij:fix-304-compliance
Open

HTTP/1.1: improve handling of malformed 304 responses#2400
yadij wants to merge 4 commits intosquid-cache:masterfrom
yadij:fix-304-compliance

Conversation

@yadij
Copy link
Copy Markdown
Contributor

@yadij yadij commented Apr 3, 2026

A number of bugs have been identified with 304 processing with
various attempts made to fix "wrong" header values being used
after a 304 response alters cached objects. Worst examples of
these historically being Content-Length:0 or Vary header change
producing wrong response messages from Squid.

Behind these issues lays the HTTP concept of resource
representation. A resource is identified by its URL, but each
resource has many possible representations (eg language
translations, versions, encodings, and more).

A revalidation request identifies a representation being
checked. If the server uses another representation with
different properties (eg. size, Vary key, etc) the 304 headers
produced may appear "malformed" and cause corruption problems
when the updates are applied to the Squid object.

Solution: do not trust the server blindly.

Compare the representation identifiers from the 304 received
with the cached object to determine whether the representations
match before applying the 304 updates. Enforcing RFC 9111
section 4.3.4 better.

Also, extend the skipUpdateHeader() to obey all MUST and most
SHOULD NOT update requirements in RFC 9111 section 3.1 and 3.2.

Copy link
Copy Markdown
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

This PR is missing a description of improvements. I cannot guess why removing HttpHeader::skipUpdateHeader() is an improvement, and what other code changes are meant to accomplish.

@bowling233
Copy link
Copy Markdown

This PR is missing a description of improvements. I cannot guess why removing HttpHeader::skipUpdateHeader() is an improvement, and what other code changes are meant to accomplish.

This PR is related to Bug 5538
(https://bugs.squid-cache.org/show_bug.cgi?id=5538#c5). The bug report
includes a description of the problem and debug logs showing how Akamai
CDN edge nodes intermittently return 304 responses with Content-Length: 0,
which corrupts the cached entry.

bowling233 added a commit to ZJUSCT/argo-cd.clusters.zjusct.io that referenced this pull request Apr 4, 2026
@yadij
Copy link
Copy Markdown
Contributor Author

yadij commented Apr 5, 2026

This PR is missing a description of improvements. I cannot guess why removing HttpHeader::skipUpdateHeader() is an improvement, and what other code changes are meant to accomplish.

It is removed because changing that header means the entire 304 cannot be used. No need to ignore only the one header field, when the entire message is being ignored.

yadij added 2 commits April 5, 2026 20:19
RFC 9110 section 3.1 and 3.2 define a lot more than
just Vary header to be skipped on 304 updates.
@yadij
Copy link
Copy Markdown
Contributor Author

yadij commented Apr 5, 2026

In c0cc5a9 I have restored skipUpdateHeader() with implementation of RFC9110 section 3.2 (and 3.1 per 3.2 inclusion) to ignore the easily ignored headers. I have not added detailed checks of the Connection and Cache-Control header lists here for simplicity and performance - if you wish we can add those more complex tests.
There is some overlap between the headers causing entire-304 to be skipped, and just that header to be skipped. I have left those explicitly coded to match the RFC requirements as-written.

@yadij yadij requested a review from rousskov April 5, 2026 21: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.

3 participants