fix(handleCacheHeaders): round `modifiedTime` to seconds by amondnet · Pull Request #1262 · h3js/h3

amondnet · 2025-12-12T01:55:04Z

Summary

This PR fixes issue #1261 where handleCacheHeaders compares modifiedTime with millisecond precision, causing cache validation to always fail when the source date has non-zero milliseconds.

The Problem

Server's modifiedTime retains millisecond precision (e.g., 2025-01-01T12:00:00.123Z)
Last-Modified header is formatted without milliseconds per HTTP spec (e.g., Wed, 01 Jan 2025 12:00:00 GMT)
Client sends back If-Modified-Since with the truncated timestamp
Comparison fails: 1735732800000 >= 1735732800123 is false

This causes unnecessary 200 responses instead of 304, wasting bandwidth and server resources.

The Fix

Truncate milliseconds from modifiedTime before both setting the header and making the comparison:

const modifiedTime = new Date(opts.modifiedTime);
modifiedTime.setMilliseconds(0);  // Added line

Test

Added a test case that specifically uses a date with milliseconds to verify the fix works correctly.

Test Plan

Existing tests pass
New test case added for millisecond precision handling
pnpm test passes

Closes #1261

Summary by CodeRabbit

Bug Fixes
- Enhanced cache header timestamp handling for consistent comparison logic.
Tests
- Added tests to verify cache header timestamp precision.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…eHeaders Truncate milliseconds from modifiedTime before comparing with If-Modified-Since header. HTTP dates have second precision, so a modifiedTime with milliseconds (e.g., 12:00:00.123) would never match the client's rounded If-Modified-Since header (12:00:00.000), causing unnecessary 200 responses instead of 304. Closes h3js#1261

coderabbitai · 2025-12-12T01:55:13Z

📝 Walkthrough

Walkthrough

The implementation truncates milliseconds from modifiedTime before comparing with If-Modified-Since headers and setting Last-Modified headers, normalizing HTTP cache validation. Two new tests validate the millisecond-precision handling behavior to prevent false cache mismatches.

Changes

Cohort / File(s)	Summary
Cache header millisecond truncation `src/utils/cache.ts`	Adds `.setMilliseconds(0)` to normalize `modifiedTime` before If-Modified-Since comparison and Last-Modified header assignment, eliminating sub-second precision mismatches
Cache header millisecond tests `test/utils.test.ts`	Adds two test cases verifying 304 responses when If-Modified-Since header matches millisecond-precision timestamps

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Straightforward, focused fix with minimal code surface area
Tests directly validate the single behavior change
Clear correspondence between implementation and test coverage

Poem

🐰 Milliseconds trimmed with a gentle snip,
No more false mismatches cause cache to flip!
Headers align at the second's sweet beat,
304s return, the victory complete! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR directly addresses issue #1261 by truncating milliseconds from modifiedTime before header formatting and comparison, exactly as suggested in the issue.
Out of Scope Changes check	✅ Passed	All changes are scoped to the cache handling functionality described in issue #1261, with no unrelated modifications present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title 'fix(handleCacheHeaders): round `modifiedTime` to seconds' directly summarizes the main change—rounding modifiedTime to seconds precision to fix cache header handling. This accurately reflects the primary purpose of the PR.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

test/utils.test.ts (1)
380-393: Good test coverage for the millisecond precision bug.

The test correctly validates that a modifiedTime with milliseconds (00:00:00.500Z) matches an If-Modified-Since header without milliseconds, returning a 304 status as expected.

Optional enhancement: Consider also asserting that the Last-Modified response header is formatted without milliseconds:
     expect(res.status).toBe(304);
+    expect(res.headers.get("last-modified")).toBe("Fri, 01 Jan 2021 00:00:00 GMT");
This would provide more comprehensive validation that the header format is correct, not just that the comparison logic works.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3fc0b8b and 214fe06.

📒 Files selected for processing (2)

src/utils/cache.ts (1 hunks)
test/utils.test.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

test/utils.test.ts (3)

src/utils/cache.ts (1)

handleCacheHeaders (15-52)

src/index.ts (1)

handleCacheHeaders (172-172)

src/event.ts (1)

res (77-79)

🔇 Additional comments (1)

src/utils/cache.ts (1)

26-34: LGTM! Clean fix for millisecond precision issue.

The placement of setMilliseconds(0) is correct—it truncates milliseconds immediately after constructing the Date object and before both setting the Last-Modified header and performing the If-Modified-Since comparison. This ensures HTTP-date format compliance (second precision per RFC 7231) and fixes the cache validation mismatch described in issue #1261.

pi0

Thanks!

amondnet requested a review from pi0 as a code owner December 12, 2025 01:55

coderabbitai Bot reviewed Dec 12, 2025

View reviewed changes

pi0 changed the title ~~fix(cache): normalize modifiedTime to seconds precision in handleCacheHeaders~~ fix(handleCacheHeaders): round modifiedTime to seconds Dec 12, 2025

Merge branch 'main' into fix/cache-headers-milliseconds-1261

350bcc9

pi0 approved these changes Dec 30, 2025

View reviewed changes

pi0 merged commit fdbbb5f into h3js:main Dec 30, 2025
4 checks passed

amondnet deleted the fix/cache-headers-milliseconds-1261 branch December 30, 2025 14:39

coderabbitai Bot mentioned this pull request May 24, 2026

fix(serve-static): compare If-Modified-Since at whole-second precision #1394

Merged

coderabbitai Bot mentioned this pull request Jul 10, 2026

fix(handleCacheHeaders): correct conditional-request precedence and Cache-Control default #1454

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(handleCacheHeaders): round `modifiedTime` to seconds#1262

fix(handleCacheHeaders): round `modifiedTime` to seconds#1262
pi0 merged 2 commits into
h3js:mainfrom
amondnet:fix/cache-headers-milliseconds-1261

amondnet commented Dec 12, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Dec 12, 2025 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

pi0 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

amondnet commented Dec 12, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The Problem

The Fix

Test

Test Plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

pi0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

amondnet commented Dec 12, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Dec 12, 2025 •

edited

Loading