Skip to content

fix: serve() 304 with body; etag log spam before condition#12

Draft
troglodyne-bot wants to merge 1 commit into
Troglodyne-Internet-Widgets:masterfrom
troglodyne-bot:koan/fix-serve-304-etag-log
Draft

fix: serve() 304 with body; etag log spam before condition#12
troglodyne-bot wants to merge 1 commit into
Troglodyne-Internet-Widgets:masterfrom
troglodyne-bot:koan/fix-serve-304-etag-log

Conversation

@troglodyne-bot

Copy link
Copy Markdown
Contributor

What

Two protocol and logging correctness fixes in TPSGI.pm.

Why

Bug 1 — RFC 7232 §4.1 violation: serve() computed a 304 status when If-Modified-Since indicated the file hadn't changed, but then opened the file and returned it as the body anyway. Clients following the spec stall or double-count content; caches and CDNs can misbehave.

Bug 2 — ETag log spam: _app() logged "METHOD 304 path" unconditionally whenever If-None-Match was present in the request — before checking whether the ETag actually matched. Every request carrying that header appeared as a 304 in logs even when a full 200 response was sent. fail2ban matching on 304 patterns would produce false positives.

How

  • serve(): early return [ 304, \@headers, [] ] after mtime/last_fetch comparison. Skips open(), gzip, and body construction for 304. Headers (Last-Modified, Vary, Content-Type, Cache-Control, Accept-Ranges) still present per RFC 7232.
  • _app() etag block: moved INFO call inside the matching branch. Non-matching ETags no longer generate false 304 log entries.
  • Bonus: ETag 304 body changed from [''] to [] for consistency.

Testing

7 subtests in t/09_serve_304_etag.t:

  • 304 has no body (serve, deflate and non-deflate paths)
  • 304 has no Content-Length or Content-Encoding headers
  • 304 includes Last-Modified
  • 200 still sends a body when file is newer
  • Non-matching ETag does not return 304
  • Matching ETag returns 304 with empty body

RFC 7232 §4.1 prohibits message bodies in 304 responses. serve() was
opening and returning the file regardless of whether $code was 304.
Added early return after the mtime/last_fetch check to skip the open()
entirely for 304 responses — no body, no Content-Length, no Content-Encoding.

_app() logged "METHOD 304 path" unconditionally when If-None-Match was
present, before checking whether the ETag actually matched. Every request
carrying If-None-Match appeared as a 304 in logs even when a full 200
was served. Moved the INFO call inside the matching branch. Also changed
the ETag 304 body from [''] to [] for consistency with the serve() fix.

Adds t/09_serve_304_etag.t (7 subtests) and t/lib/TPSGITestStubs.pm.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

1 participant