fix: correct handling of collapsing slashes (#13130)

7 files changed, 57 insertions, 9 deletions

diff --git a/.changeset/afraid-kangaroos-hope.md b/.changeset/afraid-kangaroos-hope.md
new file mode 100644
index 000000000..0c137293c
--- /dev/null
+++ b/.changeset/afraid-kangaroos-hope.md
@@ -0,0 +1,5 @@
+---
+'@astrojs/internal-helpers': patch
+---
+
+Fixes a bug that meant that internal as well as trailing duplicate slashes were collapsed
diff --git a/.changeset/pink-apes-invite.md b/.changeset/pink-apes-invite.md
new file mode 100644
index 000000000..f2c88fd16
--- /dev/null
+++ b/.changeset/pink-apes-invite.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a bug that caused duplicate slashes inside query params to be collapsed
diff --git a/packages/astro/src/vite-plugin-astro-server/trailing-slash.ts b/packages/astro/src/vite-plugin-astro-server/trailing-slash.ts
index 2588d4c49..00f71b784 100644
--- a/packages/astro/src/vite-plugin-astro-server/trailing-slash.ts
+++ b/packages/astro/src/vite-plugin-astro-server/trailing-slash.ts
@@ -9,15 +9,10 @@ export function trailingSlashMiddleware(settings: AstroSettings): vite.Connect.N
 	const { trailingSlash } = settings.config;
 
 	return function devTrailingSlash(req, res, next) {
-		const url = req.url!;
-
-		const destination = collapseDuplicateTrailingSlashes(url, true);
-		if (url && destination !== url) {
-			return writeRedirectResponse(res, 301, destination);
-		}
+		const url = new URL(`http://localhost${req.url}`);
 		let pathname: string;
 		try {
-			pathname = decodeURI(new URL(url, 'http://localhost').pathname);
+			pathname = decodeURI(url.pathname);
 		} catch (e) {
 			/* malformed uri */
 			return next(e);
@@ -25,6 +20,12 @@ export function trailingSlashMiddleware(settings: AstroSettings): vite.Connect.N
 		if (pathname.startsWith('/_') || pathname.startsWith('/@')) {
 			return next();
 		}
+
+		const destination = collapseDuplicateTrailingSlashes(pathname, true);
+		if (pathname && destination !== pathname) {
+			return writeRedirectResponse(res, 301, `${destination}${url.search}`);
+		}
+
 		if (
 			(trailingSlash === 'never' && pathname.endsWith('/') && pathname !== '/') ||
 			(trailingSlash === 'always' && !pathname.endsWith('/') && !hasFileExtension(pathname))
diff --git a/packages/astro/test/dev-routing.test.js b/packages/astro/test/dev-routing.test.js
index 2f4b84bc5..1f5ff26c6 100644
--- a/packages/astro/test/dev-routing.test.js
+++ b/packages/astro/test/dev-routing.test.js
@@ -60,6 +60,22 @@ describe('Development Routing', () => {
 			assert.equal(response.headers.get('Location'), '/');
 		});
 
+		it('does not redirect multiple internal slashes', async () => {
+			const response = await fixture.fetch('/another///here', { redirect: 'manual' });
+			assert.equal(response.status, 404);
+		});
+
+		it('does not redirect slashes on query params', async () => {
+			const response = await fixture.fetch('/another?foo=bar///', { redirect: 'manual' });
+			assert.equal(response.status, 200);
+		});
+
+		it('does redirect multiple trailing slashes with query params', async () => {
+			const response = await fixture.fetch('/another///?foo=bar///', { redirect: 'manual' });
+			assert.equal(response.status, 301);
+			assert.equal(response.headers.get('Location'), '/another/?foo=bar///');
+		});
+
 		it('404 when loading invalid dynamic route', async () => {
 			const response = await fixture.fetch('/2');
 			assert.equal(response.status, 404);
diff --git a/packages/astro/test/ssr-trailing-slash.js b/packages/astro/test/ssr-trailing-slash.test.js
index b34430a8e..d512aa609 100644
--- a/packages/astro/test/ssr-trailing-slash.js
+++ b/packages/astro/test/ssr-trailing-slash.test.js
@@ -33,6 +33,28 @@ describe('Redirecting trailing slashes in SSR', () => {
 			assert.equal(response.headers.get('Location'), '/another/');
 		});
 
+		it('Redirects to collapse multiple trailing slashes with query param', async () => {
+			const app = await fixture.loadTestAdapterApp();
+			const request = new Request('http://example.com/another///?hello=world');
+			const response = await app.render(request);
+			assert.equal(response.status, 301);
+			assert.equal(response.headers.get('Location'), '/another/?hello=world');
+		});
+
+		it('Does not redirect to collapse multiple internal slashes', async () => {
+			const app = await fixture.loadTestAdapterApp();
+			const request = new Request('http://example.com/another///path/');
+			const response = await app.render(request);
+			assert.equal(response.status, 404);
+		});
+
+		it('Does not redirect trailing slashes on query params', async () => {
+			const app = await fixture.loadTestAdapterApp();
+			const request = new Request('http://example.com/another/?hello=world///');
+			const response = await app.render(request);
+			assert.equal(response.status, 200);
+		});
+
 		it('Does not redirect when trailing slash is present', async () => {
 			const app = await fixture.loadTestAdapterApp();
 			const request = new Request('http://example.com/another/');
diff --git a/packages/internal-helpers/src/path.ts b/packages/internal-helpers/src/path.ts
index 158cb58c7..c7dda9d89 100644
--- a/packages/internal-helpers/src/path.ts
+++ b/packages/internal-helpers/src/path.ts
@@ -19,7 +19,7 @@ export function collapseDuplicateSlashes(path: string) {
 	return path.replace(/(?<!:)\/{2,}/g, '/');
 }
 
-export const MANY_TRAILING_SLASHES = /\/{2,}/g;
+export const MANY_TRAILING_SLASHES = /\/{2,}$/g;
 
 export function collapseDuplicateTrailingSlashes(path: string, trailingSlash: boolean) {
 	if (!path) {
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index 550ab510d..667562572 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -8938,7 +8938,6 @@ packages:
 
   libsql@0.4.5:
     resolution: {integrity: sha512-sorTJV6PNt94Wap27Sai5gtVLIea4Otb2LUiAUyr3p6BPOScGMKGt5F1b5X/XgkNtcsDKeX5qfeBDj+PdShclQ==}
-    cpu: [x64, arm64, wasm32]
     os: [darwin, linux, win32]
 
   lightningcss-darwin-arm64@1.29.1: