diff --git a/.changeset/fix-app-match-malformed-uri.md b/.changeset/fix-app-match-malformed-uri.md new file mode 100644 index 000000000000..e6858fde45e5 --- /dev/null +++ b/.changeset/fix-app-match-malformed-uri.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Prevents `App.match()` from throwing on request paths that contain an invalid percent-sequence. diff --git a/packages/astro/src/core/app/base.ts b/packages/astro/src/core/app/base.ts index 37f2b9e69a99..aa5670c7a8d7 100644 --- a/packages/astro/src/core/app/base.ts +++ b/packages/astro/src/core/app/base.ts @@ -261,20 +261,35 @@ export abstract class BaseApp

{ } /** - * Extracts the base-stripped, decoded pathname from a request. - * Used by adapters to compute the pathname for dev-mode route matching. + * Decodes a pathname with `decodeURI`, falling back to the raw pathname when it + * contains an invalid percent-sequence (e.g. `%C0%AF`, an overlong-UTF-8 encoding of + * `/` commonly sent by path-traversal scanners). A raw `decodeURI()` would throw + * `URIError: URI malformed`, and because `match()` runs before `render()` that error + * escapes the adapter's request handler as an uncaught exception (HTTP 500) that user + * middleware can't catch. */ - public getPathnameFromRequest(request: Request): string { - const url = new URL(request.url); - const pathname = prependForwardSlash(this.removeBase(url.pathname)); + private safeDecodeURI(pathname: string): string { try { return decodeURI(pathname); } catch (e: any) { - this.adapterLogger.error(e.toString()); + // Malformed request paths are expected client input (commonly from automated + // scanners) rather than a server fault, and this runs per-request on the hot + // path. Log at `debug` so it stays diagnosable without flooding error logs. + this.adapterLogger.debug(e.toString()); return pathname; } } + /** + * Extracts the base-stripped, decoded pathname from a request. + * Used by adapters to compute the pathname for dev-mode route matching. + */ + public getPathnameFromRequest(request: Request): string { + const url = new URL(request.url); + const pathname = prependForwardSlash(this.removeBase(url.pathname)); + return this.safeDecodeURI(pathname); + } + /** * Given a `Request`, it returns the `RouteData` that matches its `pathname`. By default, prerendered * routes aren't returned, even if they are matched. @@ -291,7 +306,7 @@ export abstract class BaseApp

{ if (!pathname) { pathname = prependForwardSlash(this.removeBase(url.pathname)); } - const routeData = this.pipeline.matchRoute(decodeURI(pathname)); + const routeData = this.pipeline.matchRoute(this.safeDecodeURI(pathname)); if (!routeData) return undefined; if (allowPrerenderedRoutes) { return routeData; @@ -303,7 +318,7 @@ export abstract class BaseApp

{ // the same pattern should handle all other URLs. if (routeData.prerender) { if (routeData.params.length > 0) { - const allMatches = this.pipeline.matchAllRoutes(decodeURI(pathname)); + const allMatches = this.pipeline.matchAllRoutes(this.safeDecodeURI(pathname)); return allMatches.find((r) => !r.prerender); } return undefined; @@ -444,7 +459,7 @@ export abstract class BaseApp

{ if (!routeData) { const domainPathname = this.computePathnameFromDomain(request); if (domainPathname) { - routeData = this.pipeline.matchRoute(decodeURI(domainPathname)); + routeData = this.pipeline.matchRoute(this.safeDecodeURI(domainPathname)); } } const resolvedOptions: ResolvedRenderOptions = { diff --git a/packages/astro/test/units/app/malformed-uri.test.ts b/packages/astro/test/units/app/malformed-uri.test.ts new file mode 100644 index 000000000000..724e99e3c9b2 --- /dev/null +++ b/packages/astro/test/units/app/malformed-uri.test.ts @@ -0,0 +1,68 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +import { App } from '../../../dist/core/app/app.js'; +import { parseRoute } from '../../../dist/core/routing/parse-route.js'; +import { createComponent, render } from '../../../dist/runtime/server/index.js'; +import { createManifest, createRouteInfo } from './test-helpers.ts'; + +/** + * Tests that a request path containing an invalid percent-sequence (one that is not + * valid UTF-8, e.g. `%C0%AF`) does not crash route matching. + * + * `App.match()` decodes the pathname with `decodeURI()`, which throws + * `URIError: URI malformed` on such input. Because matching happens before + * `App.render()`, that error used to escape the adapter request handler as an + * uncaught exception (HTTP 500) that user middleware could not catch. These paths + * are extremely common from automated path-traversal / `.env` scanners. + */ + +const routeOptions: Parameters[1] = { + config: { base: '/', trailingSlash: 'ignore' }, + pageExtensions: [], +} as any; + +const indexRouteData = parseRoute('index.astro', routeOptions, { + component: 'src/pages/index.astro', +}); + +const page = createComponent((_result: any, _props: any, _slots: any) => { + return render`

Page

`; +}); + +const pageModule = async () => ({ + page: async () => ({ + default: page, + }), +}); + +const pageMap = new Map([[indexRouteData.component, pageModule]]); + +const app = new App( + createManifest({ + routes: [createRouteInfo(indexRouteData)], + pageMap: pageMap as any, + }) as any, +); + +describe('Malformed URI handling in App.match', () => { + it('match() does not throw on an invalid percent-sequence', () => { + const request = new Request('http://example.com/%C0%AF'); + assert.doesNotThrow(() => app.match(request)); + assert.equal(app.match(request), undefined, 'no route should match a malformed path'); + }); + + it('render() returns a 404 for a malformed percent-sequence', async () => { + const request = new Request('http://example.com/%C0%AF'); + const response = await app.render(request); + assert.equal( + response.status, + 404, + 'a malformed path must resolve to a normal 404, not an uncaught 500', + ); + }); + + it('valid routes still match', () => { + const request = new Request('http://example.com/'); + assert.ok(app.match(request), '/ should still match'); + }); +});