diff --git a/.changeset/afraid-peas-smash.md b/.changeset/afraid-peas-smash.md new file mode 100644 index 0000000000..72cf6c1dca --- /dev/null +++ b/.changeset/afraid-peas-smash.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug where Astro attempted to decode a request URL multiple times, resulting in an unexpected behaviour when decoding the character `%` diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index b232ebbff5..a4b62656ae 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -149,10 +149,22 @@ export class App { return pathname; } + /** + * It removes the base from the request URL, prepends it with a forward slash and attempts to decoded it. + * + * If the decoding fails, it logs the error and return the pathname as is. + * @param request + * @private + */ #getPathnameFromRequest(request: Request): string { const url = new URL(request.url); const pathname = prependForwardSlash(this.removeBase(url.pathname)); - return pathname; + try { + return decodeURI(pathname); + } catch (e: any) { + this.getAdapterLogger().error(e.toString()); + return pathname; + } } match(request: Request): RouteData | undefined { diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 8d3c36568b..8e9f510f89 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -1,3 +1,6 @@ +import type { OutgoingHttpHeaders } from 'node:http'; +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; import type { ShikiConfig, RehypePlugin as _RehypePlugin, @@ -6,10 +9,6 @@ import type { } from '@astrojs/markdown-remark'; import { markdownConfigDefaults } from '@astrojs/markdown-remark'; import { type BuiltinTheme, bundledThemes } from 'shiki'; - -import type { OutgoingHttpHeaders } from 'node:http'; -import path from 'node:path'; -import { fileURLToPath, pathToFileURL } from 'node:url'; import { z } from 'zod'; import type { SvgRenderMode } from '../../assets/utils/svg.js'; import { EnvSchema } from '../../env/schema.js'; diff --git a/packages/astro/src/core/middleware/sequence.ts b/packages/astro/src/core/middleware/sequence.ts index 99d506695e..082ac81539 100644 --- a/packages/astro/src/core/middleware/sequence.ts +++ b/packages/astro/src/core/middleware/sequence.ts @@ -62,7 +62,11 @@ export function sequence(...handlers: MiddlewareHandler[]): MiddlewareHandler { ) { throw new AstroError({ ...ForbiddenRewrite, - message: ForbiddenRewrite.message(pathname, pathname, routeData.component), + message: ForbiddenRewrite.message( + handleContext.url.pathname, + pathname, + routeData.component, + ), hint: ForbiddenRewrite.hint(routeData.component), }); } diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index e001461593..db88275961 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -45,6 +45,7 @@ export class RenderContext { readonly pipeline: Pipeline, public locals: App.Locals, readonly middleware: MiddlewareHandler, + // It must be a DECODED pathname public pathname: string, public request: Request, public routeData: RouteData, @@ -90,7 +91,7 @@ export class RenderContext { pipeline, locals, sequence(...pipeline.internalMiddleware, middleware ?? pipelineMiddleware), - decodeURI(pathname), + pathname, request, routeData, status, @@ -102,7 +103,6 @@ export class RenderContext { partial, ); } - /** * The main function of the RenderContext. * diff --git a/packages/astro/src/core/render/params-and-props.ts b/packages/astro/src/core/render/params-and-props.ts index 7b3f60b251..4fae3d01ad 100644 --- a/packages/astro/src/core/render/params-and-props.ts +++ b/packages/astro/src/core/render/params-and-props.ts @@ -50,7 +50,7 @@ export async function getProps(opts: GetParamsAndPropsOptions): Promise { // The pathname used here comes from the server, which already encoded. // Since we decided to not mess up with encoding anymore, we need to decode them back so the parameters can match // the ones expected from the users - const params = getParams(route, decodeURI(pathname)); + const params = getParams(route, pathname); const matchedStaticPath = findPathItemByKey(staticPaths, params, route, logger); if (!matchedStaticPath && (serverLike ? route.prerender : true)) { throw new AstroError({ diff --git a/packages/astro/src/vite-plugin-astro-server/base.ts b/packages/astro/src/vite-plugin-astro-server/base.ts index 562b89ba22..84608ba508 100644 --- a/packages/astro/src/vite-plugin-astro-server/base.ts +++ b/packages/astro/src/vite-plugin-astro-server/base.ts @@ -26,7 +26,7 @@ export function baseMiddleware( try { pathname = decodeURI(new URL(url, 'http://localhost').pathname); } catch (e) { - /* malform uri */ + /* malformed uri */ return next(e); } diff --git a/packages/astro/src/vite-plugin-astro-server/request.ts b/packages/astro/src/vite-plugin-astro-server/request.ts index e0e3c32d17..656e3a8277 100644 --- a/packages/astro/src/vite-plugin-astro-server/request.ts +++ b/packages/astro/src/vite-plugin-astro-server/request.ts @@ -34,7 +34,9 @@ export async function handleRequest({ if (config.trailingSlash === 'never' && !incomingRequest.url) { pathname = ''; } else { - pathname = url.pathname; + // We already have a middleware that checks if there's an incoming URL that has invalid URI, so it's safe + // to not handle the error: packages/astro/src/vite-plugin-astro-server/base.ts + pathname = decodeURI(url.pathname); } // Add config.base back to url before passing it to SSR diff --git a/packages/astro/test/fixtures/ssr-params/src/pages/[category].astro b/packages/astro/test/fixtures/ssr-params/src/pages/[category].astro index 2e2ca3a82d..07e3150f53 100644 --- a/packages/astro/test/fixtures/ssr-params/src/pages/[category].astro +++ b/packages/astro/test/fixtures/ssr-params/src/pages/[category].astro @@ -5,6 +5,7 @@ export function getStaticPaths() { { params: { category: "%23something" } }, { params: { category: "%2Fsomething" } }, { params: { category: "%3Fsomething" } }, + { params: { category: "%25something" } }, { params: { category: "[page]" } }, { params: { category: "你好" } }, ] diff --git a/packages/astro/test/fixtures/ssr-params/src/pages/東西/[category].astro b/packages/astro/test/fixtures/ssr-params/src/pages/東西/[group].astro similarity index 57% rename from packages/astro/test/fixtures/ssr-params/src/pages/東西/[category].astro rename to packages/astro/test/fixtures/ssr-params/src/pages/東西/[group].astro index 8aaf4de24b..9cc3142e6f 100644 --- a/packages/astro/test/fixtures/ssr-params/src/pages/東西/[category].astro +++ b/packages/astro/test/fixtures/ssr-params/src/pages/東西/[group].astro @@ -1,10 +1,10 @@ --- export function getStaticPaths() { return [ - { params: { category: "food" } }, + { params: { group: "food" } }, ] } -const { category } = Astro.params +const { group } = Astro.params --- @@ -12,6 +12,6 @@ const { category } = Astro.params

Testing

-

{ category }

+

{ group }

diff --git a/packages/astro/test/params.test.js b/packages/astro/test/params.test.js index 7508b73528..60890637d3 100644 --- a/packages/astro/test/params.test.js +++ b/packages/astro/test/params.test.js @@ -148,4 +148,10 @@ describe('Astro.params in static mode', () => { const $ = cheerio.load(html); assert.equal($('.category').text(), '%3Fsomething'); }); + + it("It doesn't encode/decode URI characters such as %25 (%)", async () => { + const html = await fixture.readFile(encodeURI('/%25something/index.html')); + const $ = cheerio.load(html); + assert.equal($('.category').text(), '%25something'); + }); });