Skip to content

Commit c9a4a84

Browse files
Boyeepjames-elicx
andauthored
fix(server): use weak comparison for If-None-Match (#2710)
* fix(server): use weak comparison for If-None-Match * chore: retrigger CI * fix(server): validate conditional static requests * fix(server): tighten entity-tag parsing * test(server): document entity-tag comma behavior * fix(dev): weak-match public asset etags * fix(dev): normalize public etag lookup paths * fix(dev): canonicalize conditional public assets * fix(dev): honor Vite public file identity * fix(dev): mirror Vite public filesystem semantics * fix(dev): refresh public filesystem capabilities * fix(dev): verify public path aliases * fix(dev): index verified public request aliases * fix(dev): compose verified public path aliases * fix(dev): resolve Unicode public path classes * fix(dev): use Unicode simple case matching * fix(dev): match Unicode expansion tokens * fix(dev): keep public path node private * fix(dev): resolve public-root symlink redirects * refactor(dev): leave public etag handling to Vite * test(server): reject malformed etag list tails * fix(pages): share conditional etag matching * refactor(pages): remove reversed etag matcher --------- Co-authored-by: James <james@eli.cx>
1 parent b779754 commit c9a4a84

7 files changed

Lines changed: 155 additions & 51 deletions

File tree

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/**
2+
* Test an If-None-Match field value against a current entity tag.
3+
*
4+
* RFC 9110 requires weak comparison for If-None-Match: a weak and strong
5+
* validator with the same opaque tag compare equal.
6+
*/
7+
export function matchesIfNoneMatch(ifNoneMatch: string | undefined, etag: string): boolean {
8+
if (!ifNoneMatch) return false;
9+
10+
const current = parseEntityTag(etag, 0);
11+
if (!current || current.end !== etag.length) return false;
12+
const currentOpaqueTag = etag.slice(current.opaqueStart, current.opaqueEnd);
13+
14+
let index = skipOptionalWhitespace(ifNoneMatch, 0);
15+
if (ifNoneMatch[index] === "*") {
16+
return skipOptionalWhitespace(ifNoneMatch, index + 1) === ifNoneMatch.length;
17+
}
18+
19+
let matched = false;
20+
let sawTag = false;
21+
while (index < ifNoneMatch.length) {
22+
// RFC 9110's list extension allows recipients to ignore empty members.
23+
if (ifNoneMatch[index] === ",") {
24+
index = skipOptionalWhitespace(ifNoneMatch, index + 1);
25+
continue;
26+
}
27+
28+
const candidate = parseEntityTag(ifNoneMatch, index);
29+
if (!candidate) return false;
30+
sawTag = true;
31+
matched ||= ifNoneMatch.slice(candidate.opaqueStart, candidate.opaqueEnd) === currentOpaqueTag;
32+
33+
index = skipOptionalWhitespace(ifNoneMatch, candidate.end);
34+
if (index === ifNoneMatch.length) break;
35+
if (ifNoneMatch[index] !== ",") return false;
36+
index = skipOptionalWhitespace(ifNoneMatch, index + 1);
37+
}
38+
39+
return sawTag && matched;
40+
}
41+
42+
type ParsedEntityTag = {
43+
opaqueStart: number;
44+
opaqueEnd: number;
45+
end: number;
46+
};
47+
48+
function parseEntityTag(value: string, start: number): ParsedEntityTag | null {
49+
let index = start;
50+
if (value.startsWith("W/", index)) index += 2;
51+
if (value[index] !== '"') return null;
52+
53+
const opaqueStart = ++index;
54+
while (index < value.length && value[index] !== '"') {
55+
const code = value.charCodeAt(index);
56+
// etagc = %x21 / %x23-7E / obs-text. A backslash is an ordinary
57+
// character here; entity tags do not use quoted-string escaping.
58+
const isEtagCharacter =
59+
code === 0x21 || (code >= 0x23 && code <= 0x7e) || (code >= 0x80 && code <= 0xff);
60+
if (!isEtagCharacter) return null;
61+
index++;
62+
}
63+
if (value[index] !== '"') return null;
64+
65+
return { opaqueStart, opaqueEnd: index, end: index + 1 };
66+
}
67+
68+
function skipOptionalWhitespace(value: string, start: number): number {
69+
let index = start;
70+
while (value[index] === " " || value[index] === "\t") index++;
71+
return index;
72+
}

packages/vinext/src/server/pages-page-data.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ import { buildPagesCacheValue, type ISRCacheEntry } from "./isr-cache.js";
1616
import type { PagesPreviewData } from "./pages-preview.js";
1717
import {
1818
buildPagesNextDataScript,
19-
etagMatches,
2019
generatePagesETag,
2120
isPagesStreamingBot,
2221
requestsNoCache,
2322
type PagesGsspResponse,
2423
type PagesI18nRenderContext,
2524
type PagesNextDataExtras,
2625
} from "./pages-page-response.js";
26+
import { matchesIfNoneMatch } from "./http-conditional.js";
2727
import {
2828
createPagesGetInitialPropsRouter,
2929
hasPagesGetInitialProps,
@@ -973,7 +973,7 @@ function applyBotETagAndCheck(
973973
const etag = generatePagesETag(html);
974974
cachedResponse.headers.set("ETag", etag);
975975
const noCacheRequested = requestsNoCache(options.requestCacheControl);
976-
if (!noCacheRequested && options.ifNoneMatch && etagMatches(etag, options.ifNoneMatch)) {
976+
if (!noCacheRequested && options.ifNoneMatch && matchesIfNoneMatch(options.ifNoneMatch, etag)) {
977977
return {
978978
kind: "response",
979979
response: new Response(null, {

packages/vinext/src/server/pages-page-response.ts

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
} from "./pages-document-asset-props.js";
3434
import { isBotUserAgent } from "../utils/html-limited-bots.js";
3535
import { NEXTJS_CACHE_HEADER } from "./headers.js";
36+
import { matchesIfNoneMatch } from "./http-conditional.js";
3637

3738
// ---------------------------------------------------------------------------
3839
// Bot / crawler detection for Pages Router edge-runtime SSR
@@ -60,28 +61,6 @@ export function generatePagesETag(payload: string): string {
6061
return '"' + fnv1a52(payload).toString(36) + payload.length.toString(36) + '"';
6162
}
6263

63-
/**
64-
* Mirrors Next.js `sendEtagResponse` semantics (weak/strong comparison).
65-
*
66-
* A weak ETag `W/"..."` matches both `W/"..."` and `"..."` in `If-None-Match`.
67-
* A strong ETag `"..."` only matches the same strong token.
68-
* `*` always matches.
69-
*/
70-
export function etagMatches(etag: string, ifNoneMatch: string): boolean {
71-
if (ifNoneMatch === "*") return true;
72-
// Normalise: strip the W/ prefix for comparison. Next.js's
73-
// `sendEtagResponse` (packages/next/src/server/send-payload.ts) uses the
74-
// `fresh` package, which treats a weak token in `If-None-Match` as matching
75-
// the corresponding strong ETag and vice versa (RFC 7232 §2.3.2 weak
76-
// comparison). We replicate that behaviour here.
77-
const normalize = (t: string) => t.replace(/^W\//, "");
78-
const etagNorm = normalize(etag.trim());
79-
for (const token of ifNoneMatch.split(",")) {
80-
if (normalize(token.trim()) === etagNorm) return true;
81-
}
82-
return false;
83-
}
84-
8564
/**
8665
* Returns true when a request `Cache-Control` header asks to bypass the 304
8766
* short-circuit. Mirrors the `fresh` package's check used by Next.js's
@@ -768,7 +747,7 @@ export async function renderPagesPageResponse(
768747
const etag = generatePagesETag(fullHtml);
769748
responseHeaders.set("ETag", etag);
770749
const noCacheRequested = requestsNoCache(options.requestCacheControl);
771-
if (!noCacheRequested && options.ifNoneMatch && etagMatches(etag, options.ifNoneMatch)) {
750+
if (!noCacheRequested && options.ifNoneMatch && matchesIfNoneMatch(options.ifNoneMatch, etag)) {
772751
return new Response(null, {
773752
status: 304,
774753
headers: responseHeaders,

packages/vinext/src/server/prod-server.ts

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ import {
9292
parseAcceptedEncodings,
9393
selectContentEncoding,
9494
} from "./accept-encoding.js";
95+
import { matchesIfNoneMatch } from "./http-conditional.js";
9596
import type { NextI18nConfig } from "../config/next-config.js";
9697
import { readTrustedRevalidationHostname } from "./revalidation-host.js";
9798

@@ -410,15 +411,6 @@ function mergeVaryHeader(
410411
return merged;
411412
}
412413

413-
function matchesIfNoneMatchHeader(ifNoneMatch: string | undefined, etag: string): boolean {
414-
if (!ifNoneMatch) return false;
415-
if (ifNoneMatch === "*") return true;
416-
return ifNoneMatch
417-
.split(",")
418-
.map((value) => value.trim())
419-
.some((value) => value === etag);
420-
}
421-
422414
function installClientBuildManifestGlobals(
423415
clientDir: string,
424416
assetBase: string,
@@ -633,7 +625,7 @@ async function tryServeStatic(
633625
if (
634626
responseStatus === 200 &&
635627
typeof ifNoneMatch === "string" &&
636-
matchesIfNoneMatchHeader(ifNoneMatch, entry.etag)
628+
matchesIfNoneMatch(ifNoneMatch, entry.etag)
637629
) {
638630
const notModifiedHeaders = variesByEncoding
639631
? mergeVaryHeader({ ...entry.notModifiedHeaders, ...extraHeaders }, "Accept-Encoding")
@@ -721,7 +713,7 @@ async function tryServeStatic(
721713
if (
722714
responseStatus === 200 &&
723715
typeof ifNoneMatch === "string" &&
724-
matchesIfNoneMatchHeader(ifNoneMatch, etag)
716+
matchesIfNoneMatch(ifNoneMatch, etag)
725717
) {
726718
const notModifiedHeaders = mergeVaryHeader(baseHeaders, "Accept-Encoding");
727719
if (encoding !== "identity") notModifiedHeaders["Content-Encoding"] = encoding;
@@ -757,7 +749,7 @@ async function tryServeStatic(
757749
if (
758750
responseStatus === 200 &&
759751
typeof ifNoneMatch === "string" &&
760-
matchesIfNoneMatchHeader(ifNoneMatch, etag)
752+
matchesIfNoneMatch(ifNoneMatch, etag)
761753
) {
762754
res.writeHead(
763755
304,

tests/http-conditional.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { describe, expect, it } from "vitest";
2+
import { matchesIfNoneMatch } from "../packages/vinext/src/server/http-conditional.js";
3+
4+
describe("matchesIfNoneMatch", () => {
5+
it("matches an identical entity tag", () => {
6+
expect(matchesIfNoneMatch('"asset"', '"asset"')).toBe(true);
7+
});
8+
9+
it("uses weak comparison in either direction", () => {
10+
expect(matchesIfNoneMatch('"asset"', 'W/"asset"')).toBe(true);
11+
expect(matchesIfNoneMatch('W/"asset"', '"asset"')).toBe(true);
12+
});
13+
14+
it("matches a validator within a comma-separated field value", () => {
15+
expect(matchesIfNoneMatch('"other", W/"asset", "last"', '"asset"')).toBe(true);
16+
});
17+
18+
it("does not split commas inside an opaque tag", () => {
19+
// RFC 9110 permits commas in opaque tags. This is intentionally stricter
20+
// than Next.js 16.2.7's compiled `fresh` parser, which splits every comma.
21+
expect(matchesIfNoneMatch('"other", W/"asset,part"', '"asset,part"')).toBe(true);
22+
expect(matchesIfNoneMatch('"asset,part"', '"asset"')).toBe(false);
23+
});
24+
25+
it("treats backslashes as opaque characters rather than escapes", () => {
26+
expect(matchesIfNoneMatch('W/"asset\\part"', '"asset\\part"')).toBe(true);
27+
});
28+
29+
it("matches a wildcard with optional whitespace", () => {
30+
expect(matchesIfNoneMatch(" \t * \t ", 'W/"asset"')).toBe(true);
31+
});
32+
33+
it("rejects a wildcard mixed into an entity-tag list", () => {
34+
expect(matchesIfNoneMatch('*, "asset"', '"asset"')).toBe(false);
35+
expect(matchesIfNoneMatch('"other", *', '"asset"')).toBe(false);
36+
});
37+
38+
it("does not match different opaque tags or case", () => {
39+
expect(matchesIfNoneMatch('"other"', '"asset"')).toBe(false);
40+
expect(matchesIfNoneMatch('"ASSET"', '"asset"')).toBe(false);
41+
});
42+
43+
it("does not treat a lowercase weak prefix as W/", () => {
44+
expect(matchesIfNoneMatch('w/"asset"', '"asset"')).toBe(false);
45+
});
46+
47+
it("rejects malformed entity tags", () => {
48+
expect(matchesIfNoneMatch('asset, "match"', '"match"')).toBe(false);
49+
expect(matchesIfNoneMatch('"match", garbage', '"match"')).toBe(false);
50+
expect(matchesIfNoneMatch('"unterminated', '"unterminated"')).toBe(false);
51+
expect(matchesIfNoneMatch('W/ "asset"', '"asset"')).toBe(false);
52+
expect(matchesIfNoneMatch('"asset" suffix', '"asset"')).toBe(false);
53+
expect(matchesIfNoneMatch('"asset\x7fpart"', '"asset\x7fpart"')).toBe(false);
54+
});
55+
56+
it("ignores empty list members", () => {
57+
expect(matchesIfNoneMatch(' , W/"asset", ', '"asset"')).toBe(true);
58+
});
59+
60+
it("rejects an invalid current entity tag", () => {
61+
expect(matchesIfNoneMatch('"asset"', "asset")).toBe(false);
62+
expect(matchesIfNoneMatch('"asset"', 'W/ "asset"')).toBe(false);
63+
});
64+
65+
it("rejects an absent or empty field value", () => {
66+
expect(matchesIfNoneMatch(undefined, '"asset"')).toBe(false);
67+
expect(matchesIfNoneMatch("", '"asset"')).toBe(false);
68+
});
69+
});

tests/pages-page-response.test.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import {
44
renderPagesPageResponse,
55
isPagesStreamingBot,
66
generatePagesETag,
7-
etagMatches,
87
} from "../packages/vinext/src/server/pages-page-response.js";
98
import { resolvePagesPageData } from "../packages/vinext/src/server/pages-page-data.js";
109

@@ -1030,7 +1029,7 @@ describe("pages page response", () => {
10301029
// If-None-Match / 304 handling and ISR cache-HIT ETag
10311030
// ---------------------------------------------------------------------------
10321031

1033-
it("returns 304 when If-None-Match matches ETag on bot response (fresh-MISS path)", async () => {
1032+
it("returns 304 for a weak If-None-Match on a strong bot ETag", async () => {
10341033
const common = createCommonOptions();
10351034

10361035
// First request: compute the ETag from a full bot render.
@@ -1041,12 +1040,12 @@ describe("pages page response", () => {
10411040
const etag = firstResponse.headers.get("etag");
10421041
expect(etag).toBeTruthy();
10431042

1044-
// Second request: send If-None-Match matching the ETag.
1043+
// Second request: send the strong ETag as a weak validator.
10451044
const common2 = createCommonOptions();
10461045
const notModifiedResponse = await renderPagesPageResponse({
10471046
...common2.options,
10481047
userAgent: "Googlebot",
1049-
ifNoneMatch: etag as string,
1048+
ifNoneMatch: `W/${etag}`,
10501049
});
10511050

10521051
expect(notModifiedResponse.status).toBe(304);
@@ -1095,14 +1094,6 @@ describe("pages page response", () => {
10951094
expect(browserResponse.headers.get("etag")).toBeNull();
10961095
});
10971096

1098-
it("ETag weak-comparison: W/ prefix is ignored when matching If-None-Match", async () => {
1099-
expect(etagMatches('"abc123"', 'W/"abc123"')).toBe(true);
1100-
expect(etagMatches('W/"abc123"', '"abc123"')).toBe(true);
1101-
expect(etagMatches('"abc123"', '"abc123"')).toBe(true);
1102-
expect(etagMatches('"abc123"', '"other"')).toBe(false);
1103-
expect(etagMatches('"abc123"', "*")).toBe(true);
1104-
});
1105-
11061097
it("attaches ETag to ISR cache-HIT response for bot UAs", async () => {
11071098
// Build a fake cached ISR entry and confirm that cache-HIT + bot UA yields ETag.
11081099
const cachedHtml =

tests/serve-static.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,14 +338,15 @@ describe("tryServeStatic (with StaticFileCache)", () => {
338338

339339
// ── 304 Not Modified (conditional requests) ────────────────────
340340

341-
it("returns 304 when If-None-Match matches the ETag", async () => {
341+
it("returns 304 when a strong If-None-Match matches a weak ETag", async () => {
342342
await writeFile(clientDir, "_next/static/cached-aaa111.js", "cached content");
343343

344344
const cache = await StaticFileCache.create(clientDir);
345345
const entry = cache.lookup("/_next/static/cached-aaa111.js");
346346
const etag = entry!.etag;
347+
expect(etag).toMatch(/^W\//);
347348

348-
const req = mockReq(undefined, { "if-none-match": etag });
349+
const req = mockReq(undefined, { "if-none-match": etag.slice(2) });
349350
const { res, captured } = mockRes();
350351

351352
const served = await tryServeStatic(

0 commit comments

Comments
 (0)