diff --git a/.changeset/afraid-kangaroos-hope.md b/.changeset/afraid-kangaroos-hope.md new file mode 100644 index 0000000000..0c137293cb --- /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 0000000000..f2c88fd163 --- /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 2588d4c497..00f71b784a 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 2f4b84bc52..1f5ff26c68 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 similarity index 90% rename from packages/astro/test/ssr-trailing-slash.js rename to packages/astro/test/ssr-trailing-slash.test.js index b34430a8e8..d512aa6099 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 158cb58c78..c7dda9d890 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(/(?