Skip to content

Commit 416857c

Browse files
matingathanimatingathani
authored
[pages-shared] fix: resolve relative link hrefs against base href in early hint Link headers (#13779)
1 parent e04e180 commit 416857c

3 files changed

Lines changed: 197 additions & 1 deletion

File tree

‎.changeset/fix-pages-link-header-base-href.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@cloudflare/pages-shared": patch
3+
---
4+
5+
fix: resolve relative link hrefs against the document's `<base href>` when generating early hint Link headers

‎packages/pages-shared/__tests__/asset-server/handler.test.ts‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,171 @@ describe("asset-server handler", () => {
632632
expect(response2.headers.get("link")).toBeNull();
633633
});
634634

635+
test("early hints should resolve relative link hrefs against base href", async ({
636+
expect,
637+
}) => {
638+
const deploymentId = "deployment-" + Math.random();
639+
const metadata = createMetadataObject({ deploymentId }) as Metadata;
640+
641+
const findAssetEntryForPath = async (path: string) => {
642+
if (path === "/index.html") {
643+
return "asset-key-index-with-base.html";
644+
}
645+
return null;
646+
};
647+
const fetchAsset = () =>
648+
Promise.resolve(
649+
Object.assign(
650+
new Response(`
651+
<!DOCTYPE html>
652+
<html>
653+
<head>
654+
<base href="/" />
655+
<link rel="modulepreload" href="module.js" />
656+
</head>
657+
</html>`),
658+
{ contentType: "text/html" }
659+
)
660+
);
661+
662+
const getResponse = async () =>
663+
getTestResponse({
664+
request: new Request("https://example.com/"),
665+
metadata,
666+
findAssetEntryForPath,
667+
caches,
668+
fetchAsset,
669+
});
670+
671+
const { response, spies } = await getResponse();
672+
expect(response.status).toBe(200);
673+
await Promise.all(spies.waitUntil);
674+
675+
const earlyHintsCache = await caches.open(`eh:${deploymentId}`);
676+
const earlyHintsRes = await earlyHintsCache.match(
677+
"https://example.com/asset-key-index-with-base.html"
678+
);
679+
if (!earlyHintsRes) {
680+
throw new Error(
681+
"Did not match early hints cache on https://example.com/asset-key-index-with-base.html"
682+
);
683+
}
684+
685+
const linkHeader = earlyHintsRes.headers.get("Link");
686+
// Relative href "module.js" resolved against base "/" → absolute URL
687+
expect(linkHeader).toContain("<https://example.com/module.js>");
688+
expect(linkHeader).not.toContain("<module.js");
689+
});
690+
691+
test("early hints should resolve relative hrefs using URL semantics, not string concat", async ({
692+
expect,
693+
}) => {
694+
const deploymentId = "deployment-" + Math.random();
695+
const metadata = createMetadataObject({ deploymentId }) as Metadata;
696+
697+
const findAssetEntryForPath = async (path: string) => {
698+
if (path === "/index.html") {
699+
return "asset-key-url-semantics.html";
700+
}
701+
return null;
702+
};
703+
const fetchAsset = () =>
704+
Promise.resolve(
705+
Object.assign(
706+
new Response(`
707+
<!DOCTYPE html>
708+
<html>
709+
<head>
710+
<base href="/subdir/" />
711+
<link rel="preload" href="module.js" as="script" />
712+
<link rel="preload" href="../other.js" as="script" />
713+
</head>
714+
</html>`),
715+
{ contentType: "text/html" }
716+
)
717+
);
718+
719+
const { response, spies } = await getTestResponse({
720+
request: new Request("https://example.com/"),
721+
metadata,
722+
findAssetEntryForPath,
723+
caches,
724+
fetchAsset,
725+
});
726+
expect(response.status).toBe(200);
727+
await Promise.all(spies.waitUntil);
728+
729+
const earlyHintsCache = await caches.open(`eh:${deploymentId}`);
730+
const earlyHintsRes = await earlyHintsCache.match(
731+
"https://example.com/asset-key-url-semantics.html"
732+
);
733+
if (!earlyHintsRes) {
734+
throw new Error(
735+
"Did not match early hints cache on https://example.com/asset-key-url-semantics.html"
736+
);
737+
}
738+
739+
const linkHeader = earlyHintsRes.headers.get("Link");
740+
// "module.js" relative to "/subdir/" → "/subdir/module.js"
741+
expect(linkHeader).toContain("<https://example.com/subdir/module.js>");
742+
// "../other.js" relative to "/subdir/" → "/other.js"
743+
expect(linkHeader).toContain("<https://example.com/other.js>");
744+
});
745+
746+
test("early hints should only use the first <base href> element", async ({
747+
expect,
748+
}) => {
749+
const deploymentId = "deployment-" + Math.random();
750+
const metadata = createMetadataObject({ deploymentId }) as Metadata;
751+
752+
const findAssetEntryForPath = async (path: string) => {
753+
if (path === "/index.html") {
754+
return "asset-key-multi-base.html";
755+
}
756+
return null;
757+
};
758+
const fetchAsset = () =>
759+
Promise.resolve(
760+
Object.assign(
761+
new Response(`
762+
<!DOCTYPE html>
763+
<html>
764+
<head>
765+
<base href="/first/" />
766+
<base href="/second/" />
767+
<link rel="preload" href="module.js" as="script" />
768+
</head>
769+
</html>`),
770+
{ contentType: "text/html" }
771+
)
772+
);
773+
774+
const { response, spies } = await getTestResponse({
775+
request: new Request("https://example.com/"),
776+
metadata,
777+
findAssetEntryForPath,
778+
caches,
779+
fetchAsset,
780+
});
781+
expect(response.status).toBe(200);
782+
await Promise.all(spies.waitUntil);
783+
784+
const earlyHintsCache = await caches.open(`eh:${deploymentId}`);
785+
const earlyHintsRes = await earlyHintsCache.match(
786+
"https://example.com/asset-key-multi-base.html"
787+
);
788+
if (!earlyHintsRes) {
789+
throw new Error(
790+
"Did not match early hints cache on https://example.com/asset-key-multi-base.html"
791+
);
792+
}
793+
794+
const linkHeader = earlyHintsRes.headers.get("Link");
795+
// Should use /first/, not /second/
796+
expect(linkHeader).toContain("<https://example.com/first/module.js>");
797+
expect(linkHeader).not.toContain("/second/");
798+
});
799+
635800
test.todo("early hints should temporarily cache failures to parse links", async () => {
636801
// I couldn't figure out a way to make HTMLRewriter error out
637802
});

‎packages/pages-shared/asset-server/handler.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,8 +404,21 @@ export async function generateHandler<
404404
(async () => {
405405
try {
406406
const links: { href: string; rel: string; as?: string }[] = [];
407+
let baseHref: string | undefined;
407408

408409
const transformedResponse = new HTMLRewriter()
410+
.on("base[href]", {
411+
element(element) {
412+
// HTML spec: only the first <base href> defines the base URL
413+
if (baseHref !== undefined) {
414+
return;
415+
}
416+
const href = element.getAttribute("href");
417+
if (href !== null) {
418+
baseHref = href;
419+
}
420+
},
421+
})
409422
.on(
410423
"link[rel~=preconnect],link[rel~=preload],link[rel~=modulepreload]",
411424
{
@@ -435,7 +448,20 @@ export async function generateHandler<
435448
await transformedResponse.text();
436449

437450
links.forEach(({ href, rel, as }) => {
438-
let link = `<${href}>; rel="${rel}"`;
451+
let resolvedHref = href;
452+
if (baseHref !== undefined) {
453+
try {
454+
// Resolve href against the base, then against the request URL,
455+
// following WHATWG URL semantics for relative paths and `..` segments.
456+
resolvedHref = new URL(
457+
href,
458+
new URL(baseHref, request.url)
459+
).href;
460+
} catch {
461+
// Unparseable href — leave it as-is
462+
}
463+
}
464+
let link = `<${resolvedHref}>; rel="${rel}"`;
439465
if (as) {
440466
link += `; as=${as}`;
441467
}

0 commit comments

Comments
 (0)