fix(serve-static): compare If-Modified-Since at whole-second precision#1394
Conversation
The `last-modified` header is emitted via `toUTCString()`, which truncates to whole seconds (HTTP-date granularity). When `meta.mtime` carries sub-second milliseconds, the `if-modified-since` comparison used the full-precision mtime, so a client echoing back our own `last-modified` value never satisfied `ifModifiedSince >= mtime` and received a full 200 response instead of 304 on every conditional request. Truncate the mtime to whole seconds before comparing, matching `handleCacheHeaders`.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe ChangesHTTP Date Precision Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What
serveStaticnow compares theIf-Modified-Sincerequest value against the file mtime truncated to whole-second precision, matching the granularity of theLast-Modifiedresponse header it emits.Why
Last-Modifiedis serialized viaDate.toUTCString(), which has whole-second granularity. The conditional-request check, however, compared the client'sIf-Modified-Sinceagainst the full-precisionmtime. When a file's mtime carries sub-second milliseconds, a conditional request echoing back the server's ownLast-Modifiedvalue never satisfiedifModifiedSince >= mtime, so the server returned a full 200 instead of 304 — silently defeating conditional-request caching for any asset whose mtime is not exactly on a second boundary (the common case on many filesystems).Safety
Behaviour-preserving for whole-second mtimes. The only change is that sub-second-mtime assets now correctly return
304on a matching conditional request. This mirrors the existing whole-second handling already used inhandleCacheHeaders(cache.ts), so the two paths are now consistent.Testing
Added "Handles cache (if-modified-since) for sub-second mtime" to
test/static.test.ts(both web and node targets). It fails on the current code (expected 200 to deeply equal 304) and passes after the fix. Full test suite green (1154 passed / 14 skipped), typecheck clean, oxlint clean on changed files.Impact
Restores correct
304 Not Modifiedresponses for conditional requests on static assets with sub-second mtimes — fixing avoidable full-body re-transfers.Summary by CodeRabbit