michael/astro
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix duplicate slash handling (#7935)

Browse source 
* 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
This commit is contained in:
Nate Moore 2023-08-03 13:13:39 -05:00 • committed by GitHub
parent 705432f8d2
commit 6035bb35f2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 40 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/clean-tools-yawn.md Normal file
View file

@ -0,0 +1,5 @@
---
'@astrojs/internal-helpers': patch
---
Add `collapseDuplicateSlashes` helper

5
.changeset/flat-toes-fold.md Normal file
View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Properly handle routing when multiple slashes are present in the request by collapsing them to a single `/`

10
packages/astro/src/core/app/index.ts
View file

@ -10,7 +10,7 @@ import type { SinglePageBuiltModule } from '../build/types';
import { attachToResponse, getSetCookiesFromResponse } from '../cookies/index.js'; import { attachToResponse, getSetCookiesFromResponse } from '../cookies/index.js';
import { consoleLogDestination } from '../logger/console.js'; import { consoleLogDestination } from '../logger/console.js';
import { error, type LogOptions } from '../logger/core.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 { RedirectSinglePageBuiltModule } from '../redirects/index.js';
import { isResponse } from '../render/core.js'; import { isResponse } from '../render/core.js';
import { import {
@ -126,13 +126,17 @@ export class App {
const url = new URL(request.url); const url = new URL(request.url);
// ignore requests matching public assets // ignore requests matching public assets
if (this.#manifest.assets.has(url.pathname)) return undefined; if (this.#manifest.assets.has(url.pathname)) return undefined;
let pathname = prependForwardSlash(this.removeBase(url.pathname)); const pathname = prependForwardSlash(this.removeBase(url.pathname));
let routeData = matchRoute(pathname, this.#manifestData); const routeData = matchRoute(pathname, this.#manifestData);
// missing routes fall-through, prerendered are handled by static layer // missing routes fall-through, prerendered are handled by static layer
if (!routeData || routeData.prerender) return undefined; if (!routeData || routeData.prerender) return undefined;
return routeData; return routeData;
} }
async render(request: Request, routeData?: RouteData, locals?: object): Promise<Response> { 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) { if (!routeData) {
routeData = this.match(request); routeData = this.match(request);
} }

4
packages/astro/src/vite-plugin-astro-server/request.ts
View file

@ -7,7 +7,7 @@ import { collectErrorMetadata } from '../core/errors/dev/index.js';
import { createSafeError } from '../core/errors/index.js'; import { createSafeError } from '../core/errors/index.js';
import { error } from '../core/logger/core.js'; import { error } from '../core/logger/core.js';
import * as msg from '../core/messages.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 { eventError, telemetry } from '../events/index.js';
import { isServerLikeOutput } from '../prerender/utils.js'; import { isServerLikeOutput } from '../prerender/utils.js';
import { runWithErrorHandling } from './controller.js'; import { runWithErrorHandling } from './controller.js';
@ -37,7 +37,7 @@ export async function handleRequest({
const origin = `${moduleLoader.isHttps() ? 'https' : 'http'}://${incomingRequest.headers.host}`; const origin = `${moduleLoader.isHttps() ? 'https' : 'http'}://${incomingRequest.headers.host}`;
const buildingToSSR = isServerLikeOutput(config); const buildingToSSR = isServerLikeOutput(config);
const url = new URL(origin + incomingRequest.url); const url = new URL(collapseDuplicateSlashes(origin + incomingRequest.url));
let pathname: string; let pathname: string;
if (config.trailingSlash === 'never' && !incomingRequest.url) { if (config.trailingSlash === 'never' && !incomingRequest.url) {
pathname = ''; pathname = '';

6
packages/astro/test/fixtures/ssr-request/src/pages/request.astro vendored
View file

@ -1,5 +1,7 @@
--- ---
const origin = Astro.url.origin; const origin = Astro.url.origin;
const requestPathname = new URL(Astro.request.url).pathname;
const pathname = Astro.url.pathname;
--- ---
<html> <html>
@ -15,6 +17,8 @@ const origin = Astro.url.origin;
</script> </script>
</head> </head>
<body> <body>
<h1 id="origin">{origin}</h1> <p id="origin">{origin}</p>
<p id="pathname">{pathname}</p>
<p id="request-pathname">{requestPathname}</p>
</body> </body>
</html> </html>

12
packages/astro/test/ssr-request.test.js
View file

@ -42,6 +42,18 @@ describe('Using Astro.request in SSR', () => {
expect($('#origin').text()).to.equal('http://example.com'); 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 () => { it('public file is copied over', async () => {
const json = await fixture.readFile('/client/cars.json'); const json = await fixture.readFile('/client/cars.json');
expect(json).to.not.be.undefined; expect(json).to.not.be.undefined;

4
packages/internal-helpers/src/path.ts
View file

@ -15,6 +15,10 @@ export function prependForwardSlash(path: string) {
return path[0] === '/' ? path : '/' + path; return path[0] === '/' ? path : '/' + path;
} }
export function collapseDuplicateSlashes(path: string) {
return path.replace(/(?<!:)\/\/+/g, '/');
}
export function removeTrailingForwardSlash(path: string) { export function removeTrailingForwardSlash(path: string) {
return path.endsWith('/') ? path.slice(0, path.length - 1) : path; return path.endsWith('/') ? path.slice(0, path.length - 1) : path;
} }