refactor(httputil): consolidate conditional-request serving#340
Conversation
ETag/If-Match/If-None-Match handling was duplicated across the apiv1 and git handlers and missing entirely from the generic caching handler builder. Add ServeCacheHit, ServeCacheStat and CheckConditionals to httputil and route apiv1, the handler builder, and git snapshots through them. This fixes a latent bug where builder-based strategies (github-releases, host, hermit, proxy, artifactory) ignored conditional requests on cache hits, and removes git's upward dependency on the strategy package for CheckConditionals.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c91ed13ec
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| maps.Copy(w.Header(), headers) | ||
| if status := CheckConditionals(r, headers.Get(ETagHeader)); status != 0 { | ||
| w.WriteHeader(status) |
There was a problem hiding this comment.
When a cached object has Content-Length and an If-Match request fails, this branch writes a 412 after copying the object's headers but sends no body. Unlike 304, Go does not suppress Content-Length for 412 responses, so clients can hang or fail with an unexpected EOF waiting for the advertised cached-object bytes. Strip or replace entity headers before writing the 412 response.
Useful? React with 👍 / 👎.
1 participant
ETag/If-Match/If-None-Match handling was duplicated across the apiv1 and git handlers and missing entirely from the generic caching handler builder.
Add ServeCacheHit, ServeCacheStat and CheckConditionals to httputil and route apiv1, the handler builder, and git snapshots through them. This fixes a latent bug where builder-based strategies (github-releases, host, hermit, proxy, artifactory) ignored conditional requests on cache hits, and removes git's upward dependency on the strategy package for CheckConditionals.