From 6035bb35f222fc6a80b418f13998b21c59da85b6 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Thu, 3 Aug 2023 13:13:39 -0500 Subject: [PATCH] Fix duplicate slash handling (#7935) * fix(#7806): collapse duplicate slashes * refactor: handle request.url with duplicate slashes * chore: improve duplicate slash test * fix: only collapse duplicate slashes once * chore: appease TS --- .changeset/clean-tools-yawn.md | 5 +++++ .changeset/flat-toes-fold.md | 5 +++++ packages/astro/src/core/app/index.ts | 10 +++++++--- .../astro/src/vite-plugin-astro-server/request.ts | 4 ++-- .../fixtures/ssr-request/src/pages/request.astro | 6 +++++- packages/astro/test/ssr-request.test.js | 12 ++++++++++++ packages/internal-helpers/src/path.ts | 4 ++++ 7 files changed, 40 insertions(+), 6 deletions(-) create mode 100644 .changeset/clean-tools-yawn.md create mode 100644 .changeset/flat-toes-fold.md diff --git a/.changeset/clean-tools-yawn.md b/.changeset/clean-tools-yawn.md new file mode 100644 index 000000000..5c8ee36f7 --- /dev/null +++ b/.changeset/clean-tools-yawn.md @@ -0,0 +1,5 @@ +--- +'@astrojs/internal-helpers': patch +--- + +Add `collapseDuplicateSlashes` helper diff --git a/.changeset/flat-toes-fold.md b/.changeset/flat-toes-fold.md new file mode 100644 index 000000000..d5cdc341a --- /dev/null +++ b/.changeset/flat-toes-fold.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Properly handle routing when multiple slashes are present in the request by collapsing them to a single `/` diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index da025609a..da498fa48 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -10,7 +10,7 @@ import type { SinglePageBuiltModule } from '../build/types'; import { attachToResponse, getSetCookiesFromResponse } from '../cookies/index.js'; import { consoleLogDestination } from '../logger/console.js'; import { error, type LogOptions } from '../logger/core.js'; -import { prependForwardSlash, removeTrailingForwardSlash } from '../path.js'; +import { prependForwardSlash, removeTrailingForwardSlash, collapseDuplicateSlashes } from '../path.js'; import { RedirectSinglePageBuiltModule } from '../redirects/index.js'; import { isResponse } from '../render/core.js'; import { @@ -126,13 +126,17 @@ export class App { const url = new URL(request.url); // ignore requests matching public assets if (this.#manifest.assets.has(url.pathname)) return undefined; - let pathname = prependForwardSlash(this.removeBase(url.pathname)); - let routeData = matchRoute(pathname, this.#manifestData); + const pathname = prependForwardSlash(this.removeBase(url.pathname)); + const routeData = matchRoute(pathname, this.#manifestData); // missing routes fall-through, prerendered are handled by static layer if (!routeData || routeData.prerender) return undefined; return routeData; } async render(request: Request, routeData?: RouteData, locals?: object): Promise { + // Handle requests with duplicate slashes gracefully by cloning with a cleaned-up request URL + if (request.url !== collapseDuplicateSlashes(request.url)) { + request = new Request(collapseDuplicateSlashes(request.url), request); + } if (!routeData) { routeData = this.match(request); } diff --git a/packages/astro/src/vite-plugin-astro-server/request.ts b/packages/astro/src/vite-plugin-astro-server/request.ts index 2e6360e35..f11382319 100644 --- a/packages/astro/src/vite-plugin-astro-server/request.ts +++ b/packages/astro/src/vite-plugin-astro-server/request.ts @@ -7,7 +7,7 @@ import { collectErrorMetadata } from '../core/errors/dev/index.js'; import { createSafeError } from '../core/errors/index.js'; import { error } from '../core/logger/core.js'; import * as msg from '../core/messages.js'; -import { removeTrailingForwardSlash } from '../core/path.js'; +import { removeTrailingForwardSlash, collapseDuplicateSlashes } from '../core/path.js'; import { eventError, telemetry } from '../events/index.js'; import { isServerLikeOutput } from '../prerender/utils.js'; import { runWithErrorHandling } from './controller.js'; @@ -37,7 +37,7 @@ export async function handleRequest({ const origin = `${moduleLoader.isHttps() ? 'https' : 'http'}://${incomingRequest.headers.host}`; const buildingToSSR = isServerLikeOutput(config); - const url = new URL(origin + incomingRequest.url); + const url = new URL(collapseDuplicateSlashes(origin + incomingRequest.url)); let pathname: string; if (config.trailingSlash === 'never' && !incomingRequest.url) { pathname = ''; diff --git a/packages/astro/test/fixtures/ssr-request/src/pages/request.astro b/packages/astro/test/fixtures/ssr-request/src/pages/request.astro index 8027e784d..c0d1da784 100644 --- a/packages/astro/test/fixtures/ssr-request/src/pages/request.astro +++ b/packages/astro/test/fixtures/ssr-request/src/pages/request.astro @@ -1,5 +1,7 @@ --- const origin = Astro.url.origin; +const requestPathname = new URL(Astro.request.url).pathname; +const pathname = Astro.url.pathname; --- @@ -15,6 +17,8 @@ const origin = Astro.url.origin; -

{origin}

+

{origin}

+

{pathname}

+

{requestPathname}

diff --git a/packages/astro/test/ssr-request.test.js b/packages/astro/test/ssr-request.test.js index 276eb2551..7bdce20b5 100644 --- a/packages/astro/test/ssr-request.test.js +++ b/packages/astro/test/ssr-request.test.js @@ -42,6 +42,18 @@ describe('Using Astro.request in SSR', () => { expect($('#origin').text()).to.equal('http://example.com'); }); + it('Duplicate slashes are collapsed', async () => { + const app = await fixture.loadTestAdapterApp(); + const request = new Request('http://example.com/subpath////request/////'); + const response = await app.render(request); + expect(response.status).to.equal(200); + const html = await response.text(); + const $ = cheerioLoad(html); + expect($('#origin').text()).to.equal('http://example.com'); + expect($('#pathname').text()).to.equal('/subpath/request/'); + expect($('#request-pathname').text()).to.equal('/subpath/request/'); + }); + it('public file is copied over', async () => { const json = await fixture.readFile('/client/cars.json'); expect(json).to.not.be.undefined; diff --git a/packages/internal-helpers/src/path.ts b/packages/internal-helpers/src/path.ts index 501665542..cc9954ef2 100644 --- a/packages/internal-helpers/src/path.ts +++ b/packages/internal-helpers/src/path.ts @@ -15,6 +15,10 @@ export function prependForwardSlash(path: string) { return path[0] === '/' ? path : '/' + path; } +export function collapseDuplicateSlashes(path: string) { + return path.replace(/(?