fix(handleCacheHeaders): correct conditional-request precedence and Cache-Control default by pi0x · Pull Request #1454 · h3js/h3

pi0x · 2026-07-10T08:29:47Z

Fixes #1442, fixes #1453.

handleCacheHeaders had three issues addressed here:

1. Forced `public` Cache-Control (#1442, #1453)

public was unconditionally prepended, so cacheControls: ["private"] produced the contradictory public, private — a CDN may treat that as publicly cacheable and expose personalized/authenticated responses. Now public is only the default when no explicit cacheControls are provided.

const cacheControls = opts.cacheControls ? [...opts.cacheControls] : ["public"];

2. Conditional-request precedence bug (#1453)

The old code evaluated If-None-Match and If-Modified-Since independently and OR'd the results. Per RFC 9110 §13.1.3, a recipient MUST ignore If-Modified-Since when If-None-Match is present. A resource modified twice within one second could otherwise wrongly return 304. If-None-Match now takes strict precedence; If-Modified-Since is only consulted when absent.

3. Weak ETag comparison + wildcard (#1453)

ETag matching used strict string comparison only. It now uses the RFC 9110 weak comparison function (ignoring the W/ indicator on either side) and supports the * wildcard.

Tests

Added regression tests to test/utils.test.ts:

explicit cacheControls no longer force public
If-Modified-Since ignored when If-None-Match is present
weak (W/) and wildcard (*) ETag matching

Existing cache tests plus the new ones pass; lint and typecheck are clean.

Note

#1442 proposed a dedicated cacheVisibility option; this follows #1453's simpler approach of skipping the implicit public when custom cacheControls are given — same outcome, backward-compatible, and keeps the API minimal.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved HTTP cache validation to follow RFC precedence: If-None-Match now overrides If-Modified-Since.
- Added correct weak ETag matching (including W/"...") and If-None-Match: * handling.
- Refined cache-control header generation to avoid contradictory directives and to apply s-maxage only for non-private responses.
- Enhanced conditional response headers (Last-Modified/ETag) and 304 behavior for static assets.
Documentation
- Updated caching documentation to clarify precedence and cache-control directive behavior.
Tests
- Expanded conditional request and directive parsing coverage.

…default `handleCacheHeaders` had three issues (#1442, #1453): - Forced `public` onto `Cache-Control` unconditionally, producing contradictory directives like `public, private` for authenticated responses. Now `public` is only the default when no explicit `cacheControls` are provided. - Evaluated `If-None-Match` and `If-Modified-Since` independently. Per RFC 9110 §13.1.3, `If-Modified-Since` must be ignored when `If-None-Match` is present; it is now only consulted when absent. - Matched ETags with strict string comparison only. Now uses RFC 9110 weak comparison (ignoring the `W/` indicator) and supports the `*` wildcard. Co-Authored-By: Claude Opus 4.8 <[email protected]>

coderabbitai · 2026-07-10T08:30:07Z

📝 Walkthrough

Walkthrough

handleCacheHeaders now supports non-public cache directives, correct conditional-request precedence, and weak or wildcard ETag matching. Static serving uses the shared ETag matcher, with tests and documentation covering the updated behavior.

Changes

Cache validation behavior

Layer / File(s)	Summary
Shared ETag matching `src/utils/internal/cache.ts`, `src/utils/cache.ts`, `src/utils/static.ts`	Adds weak, comma-aware, and wildcard `If-None-Match` matching and applies it to cache and static responses.
Cache header and conditional request logic `src/utils/cache.ts`, `src/utils/static.ts`	Respects explicit `private`/`no-store` directives, limits `s-maxage` to non-private responses, and prioritizes `If-None-Match` over `If-Modified-Since`.
Cache behavior validation and documentation `test/utils.test.ts`, `test/static.test.ts`, `docs/2.utils/1.request.md`	Tests and documentation cover cache-control generation, conditional-request precedence, static responses, and ETag matching.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

h3js/h3#1262: Updates overlapping modifiedTime normalization in handleCacheHeaders.
h3js/h3#1395: Updates related If-None-Match and ETag matching behavior.
h3js/h3#1396: Modifies the same ETag and conditional-request matching logic.

Suggested reviewers: pi0

Poem

I’m a rabbit guarding the cache,
Weak tags now hop into place.
Private stays private, dates know their turn,
Wildcard stars make matches return.
304 carrots glow in the burrow.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: cache header behavior fixes in handleCacheHeaders.
Linked Issues check	✅ Passed	The changes address both linked issues by allowing explicit Cache-Control directives, fixing If-None-Match precedence, and adding weak/* ETag matching.
Out of Scope Changes check	✅ Passed	The docs, helper, runtime, and test updates all support the cache-header and conditional-request fixes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/cache-headers

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.}

…tching - Restore default `public` for shared caches, suppressing it only when the caller sets `private`/`no-store` (previously any explicit `cacheControls` dropped it, breaking CDN storage of authenticated responses). - Omit the shared-cache `s-maxage` directive for private/no-store responses. - Treat a present-but-empty `If-None-Match` as absent so `If-Modified-Since` revalidation still applies. - Extract `matchETag` into `internal/cache.ts` and reuse it in `serveStatic` so static serving gets weak-tag/list/`*` handling from one implementation. Co-Authored-By: Claude Opus 4.8 <[email protected]>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/utils/static.ts`:
- Around line 174-175: Update the conditional-cache handling in the static
request logic around `ifNoneMatch` and the `If-Modified-Since` check so
`If-None-Match` is evaluated first whenever present, and only fall back to
`If-Modified-Since` when it is absent. Match the precedence behavior used by
`handleCacheHeaders`.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8b79e3b4-7ef4-4c0a-89e9-851bb3102b4b

📥 Commits

Reviewing files that changed from the base of the PR and between 54055cb and 3733b95.

📒 Files selected for processing (5)

docs/2.utils/1.request.md
src/utils/cache.ts
src/utils/internal/cache.ts
src/utils/static.ts
test/utils.test.ts

✅ Files skipped from review due to trivial changes (1)

docs/2.utils/1.request.md

codecov · 2026-07-10T13:21:04Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/utils/static.ts`:
- Around line 171-176: Update the conditional-cache validation logic around
ifNoneMatch and matchETag to detect header presence rather than truthiness, so
an empty If-None-Match value does not fall back to If-Modified-Since. Resolve
the selected response ETag from options.headers as well as meta.etag, and pass
it to matchETag; ensure wildcard matching succeeds even when no metadata ETag
exists.

In `@test/static.test.ts`:
- Around line 325-332: Replace the invalid If-None-Match value "w/999" in the
test with the valid weak entity-tag `W/"999"`, preserving the test’s intended
nonmatching ETag behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 98b0e32b-cc5f-4d50-a280-135e02d8d5bc

📥 Commits

Reviewing files that changed from the base of the PR and between 3733b95 and 104e8a4.

📒 Files selected for processing (5)

src/utils/cache.ts
src/utils/internal/cache.ts
src/utils/static.ts
test/static.test.ts
test/utils.test.ts

🚧 Files skipped from review as they are similar to previous changes (3)

src/utils/internal/cache.ts
src/utils/cache.ts
test/utils.test.ts

pi0x requested a review from pi0 as a code owner July 10, 2026 08:29

pi0 changed the title ~~fix(cache): correct conditional-request precedence and Cache-Control default~~ fix(handleCacheHeaders): correct conditional-request precedence and Cache-Control default Jul 10, 2026

pi0 approved these changes Jul 10, 2026

View reviewed changes

pi0 and others added 2 commits July 10, 2026 11:38

chore: apply automated updates

3733b95

coderabbitai Bot reviewed Jul 10, 2026

View reviewed changes

Comment thread src/utils/static.ts Outdated

up

104e8a4

pi0 approved these changes Jul 10, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 10, 2026

View reviewed changes

Comment thread src/utils/static.ts

Comment thread test/static.test.ts

pi0 merged commit 2af124c into main Jul 10, 2026
10 checks passed

pi0 deleted the fix/cache-headers branch July 10, 2026 13:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

fix(handleCacheHeaders): correct conditional-request precedence and Cache-Control default#1454
pi0 merged 4 commits into
mainfrom
fix/cache-headers

pi0x commented Jul 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

codecov Bot commented Jul 10, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pi0x commented Jul 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Forced public Cache-Control (#1442, #1453)

2. Conditional-request precedence bug (#1453)

3. Weak ETag comparison + wildcard (#1453)

Tests

Note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Jul 10, 2026

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pi0x commented Jul 10, 2026 •

edited by coderabbitai Bot

Loading

1. Forced `public` Cache-Control (#1442, #1453)

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading