[pull] main from withastro:main

.changeset/tiny-forks-yawn.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix skew protection query params not being applied to island hydration `component-url` and `renderer-url`, and ensure query params are appended safely for asset URLs with existing search/hash parts.

packages/astro/src/core/build/plugins/plugin-manifest.ts

@@ -149,15 +149,21 @@ async function buildManifest(
 
 	const routes: SerializedRouteInfo[] = [];
 	const domainLookupTable: Record<string, string> = {};
-	const entryModules = Object.fromEntries(internals.entrySpecifierToBundleMap.entries());
-	if (settings.scripts.some((script) => script.stage === 'page')) {
-		staticFiles.push(entryModules[PAGE_SCRIPT_ID]);
-	}
+	const rawEntryModules = Object.fromEntries(internals.entrySpecifierToBundleMap.entries());
 
 	const assetQueryParams = settings.adapter?.client?.assetQueryParams;
 	const assetQueryString = assetQueryParams ? assetQueryParams.toString() : undefined;
 
 	const appendAssetQuery = (pth: string) => (assetQueryString ? `${pth}?${assetQueryString}` : pth);
+	const entryModules = Object.fromEntries(
+		Object.entries(rawEntryModules).map(([key, value]) => [
+			key,
+			value ? appendAssetQuery(value) : value,
+		]),
+	);
+	if (settings.scripts.some((script) => script.stage === 'page')) {
+		staticFiles.push(rawEntryModules[PAGE_SCRIPT_ID]);
+	}
 
 	const prefixAssetPath = (pth: string) => {
 		let result = '';
@@ -195,7 +201,7 @@ async function buildManifest(
 
 		const scripts: SerializedRouteInfo['scripts'] = [];
 		if (settings.scripts.some((script) => script.stage === 'page')) {
-			const src = entryModules[PAGE_SCRIPT_ID];
+			const src = rawEntryModules[PAGE_SCRIPT_ID];
 
 			scripts.push({
 				type: 'external',

packages/astro/src/core/render/ssr-element.ts

@@ -3,23 +3,52 @@ import { fileExtension, joinPaths, prependForwardSlash, slash } from '../../core
 import type { SSRElement } from '../../types/public/internal.js';
 import type { AssetsPrefix, StylesheetAsset } from '../app/types.js';
 
+const URL_PARSE_BASE = 'https://astro.build';
+
+function splitAssetPath(path: string): { pathname: string; suffix: string } {
+	const parsed = new URL(path, URL_PARSE_BASE);
+	const isAbsolute = URL.canParse(path);
+	const pathname =
+		!isAbsolute && !path.startsWith('/') ? parsed.pathname.slice(1) : parsed.pathname;
+
+	return {
+		pathname,
+		suffix: `${parsed.search}${parsed.hash}`,
+	};
+}
+
+function appendQueryParams(path: string, queryParams: URLSearchParams): string {
+	const queryString = queryParams.toString();
+	if (!queryString) {
+		return path;
+	}
+
+	const hashIndex = path.indexOf('#');
+	const basePath = hashIndex === -1 ? path : path.slice(0, hashIndex);
+	const hash = hashIndex === -1 ? '' : path.slice(hashIndex);
+	const separator = basePath.includes('?') ? '&' : '?';
+
+	return `${basePath}${separator}${queryString}${hash}`;
+}
+
 export function createAssetLink(
 	href: string,
 	base?: string,
 	assetsPrefix?: AssetsPrefix,
 	queryParams?: URLSearchParams,
 ): string {
+	const { pathname, suffix } = splitAssetPath(href);
 	let url = '';
 	if (assetsPrefix) {
-		const pf = getAssetsPrefix(fileExtension(href), assetsPrefix);
-		url = joinPaths(pf, slash(href));
+		const pf = getAssetsPrefix(fileExtension(pathname), assetsPrefix);
+		url = joinPaths(pf, slash(pathname)) + suffix;
 	} else if (base) {
-		url = prependForwardSlash(joinPaths(base, slash(href)));
+		url = prependForwardSlash(joinPaths(base, slash(pathname))) + suffix;
 	} else {
 		url = href;
 	}
 	if (queryParams) {
-		url += '?' + queryParams.toString();
+		url = appendQueryParams(url, queryParams);
 	}
 	return url;
 }

packages/astro/test/asset-query-params.test.js

@@ -1,9 +1,17 @@
 import assert from 'node:assert/strict';
 import { before, describe, it } from 'node:test';
 import * as cheerio from 'cheerio';
+import woof from './fixtures/multiple-jsx-renderers/renderers/woof/index.mjs';
+import meow from './fixtures/multiple-jsx-renderers/renderers/meow/index.mjs';
 import testAdapter from './test-adapter.js';
 import { loadFixture } from './test-utils.js';
 
+const multiCdnAssetsPrefix = {
+	js: 'https://js.example.com',
+	css: 'https://css.example.com',
+	fallback: 'https://example.com',
+};
+
 describe('Asset Query Parameters (Adapter Client Config)', () => {
 	/** @type {import('./test-utils').Fixture} */
 	let fixture;
@@ -94,3 +102,89 @@ describe('Asset Query Parameters with Fonts', () => {
 		});
 	});
 });
+
+describe('Asset Query Parameters with Islands', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/multiple-jsx-renderers/',
+			output: 'server',
+			integrations: [woof({ include: '**/*.woof.jsx' }), meow({ include: '**/*.meow.jsx' })],
+			adapter: testAdapter({
+				extendAdapter: {
+					client: {
+						assetQueryParams: new URLSearchParams({ dpl: 'test-deploy-id' }),
+					},
+				},
+			}),
+		});
+		await fixture.build();
+	});
+
+	it('appends assetQueryParams to astro-island component and renderer URLs', async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const response = await app.render(new Request('http://example.com/client-load'));
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		const $ = cheerio.load(html);
+		const island = $('astro-island').first();
+
+		assert.ok(island.length > 0, 'Should have at least one astro-island');
+		assert.match(
+			island.attr('component-url'),
+			/\?dpl=test-deploy-id/,
+			`astro-island component-url should include assetQueryParams: ${island.attr('component-url')}`,
+		);
+		assert.match(
+			island.attr('renderer-url'),
+			/\?dpl=test-deploy-id/,
+			`astro-island renderer-url should include assetQueryParams: ${island.attr('renderer-url')}`,
+		);
+	});
+});
+
+describe('Asset Query Parameters with Islands and assetsPrefix map', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/astro-assets-prefix/',
+			output: 'server',
+			adapter: testAdapter({
+				extendAdapter: {
+					client: {
+						assetQueryParams: new URLSearchParams({ dpl: 'test-deploy-id' }),
+					},
+				},
+			}),
+			build: {
+				assetsPrefix: multiCdnAssetsPrefix,
+			},
+		});
+		await fixture.build();
+	});
+
+	it('uses js assetsPrefix for island URLs while appending assetQueryParams', async () => {
+		const app = await fixture.loadTestAdapterApp();
+		const response = await app.render(new Request('http://example.com/custom-base/'));
+		assert.equal(response.status, 200);
+		const html = await response.text();
+		const $ = cheerio.load(html);
+		const island = $('astro-island').first();
+
+		assert.ok(island.length > 0, 'Should have at least one astro-island');
+		assert.match(
+			island.attr('component-url'),
+			/^https:\/\/js\.example\.com\/_astro\/.*\?dpl=test-deploy-id$/,
+			`astro-island component-url should use js assetsPrefix and include assetQueryParams: ${island.attr('component-url')}`,
+		);
+		assert.match(
+			island.attr('renderer-url'),
+			/^https:\/\/js\.example\.com\/_astro\/.*\?dpl=test-deploy-id$/,
+			`astro-island renderer-url should use js assetsPrefix and include assetQueryParams: ${island.attr('renderer-url')}`,
+		);
+	});
+});