summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Nate Moore <natemoo-re@users.noreply.github.com> 2023-08-03 13:13:39 -0500
committerGravatar GitHub <noreply@github.com> 2023-08-03 13:13:39 -0500
commit6035bb35f222fc6a80b418f13998b21c59da85b6 (patch)
treec2074cfce02b13f7a0c21bd3e7e005ca9590af3d
parent705432f8d2c57274559430ea35aabe41090851da (diff)
downloadastro-6035bb35f222fc6a80b418f13998b21c59da85b6.tar.gz
astro-6035bb35f222fc6a80b418f13998b21c59da85b6.tar.zst
astro-6035bb35f222fc6a80b418f13998b21c59da85b6.zip
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
-rw-r--r--.changeset/clean-tools-yawn.md5
-rw-r--r--.changeset/flat-toes-fold.md5
-rw-r--r--packages/astro/src/core/app/index.ts10
-rw-r--r--packages/astro/src/vite-plugin-astro-server/request.ts4
-rw-r--r--packages/astro/test/fixtures/ssr-request/src/pages/request.astro6
-rw-r--r--packages/astro/test/ssr-request.test.js12
-rw-r--r--packages/internal-helpers/src/path.ts4
7 files changed, 40 insertions, 6 deletions
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<Response> {
+ // 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;
---
<html>
@@ -15,6 +17,8 @@ const origin = Astro.url.origin;
</script>
</head>
<body>
-<h1 id="origin">{origin}</h1>
+<p id="origin">{origin}</p>
+<p id="pathname">{pathname}</p>
+<p id="request-pathname">{requestPathname}</p>
</body>
</html>
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(/(?<!:)\/\/+/g, '/');
+}
+
export function removeTrailingForwardSlash(path: string) {
return path.endsWith('/') ? path.slice(0, path.length - 1) : path;
}