diff --git a/.changeset/brave-cycles-suffer.md b/.changeset/brave-cycles-suffer.md new file mode 100644 index 000000000000..1a8de425661d --- /dev/null +++ b/.changeset/brave-cycles-suffer.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes a bug in the Astro rewrite logic, where rewriting the index with parameters - `next("/?foo=bar")` - didn't work as expected. diff --git a/.changeset/cold-dolls-do.md b/.changeset/cold-dolls-do.md new file mode 100644 index 000000000000..4169415285b0 --- /dev/null +++ b/.changeset/cold-dolls-do.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Handle image-size errors by displaying a clearer message diff --git a/.changeset/lazy-rockets-raise.md b/.changeset/lazy-rockets-raise.md new file mode 100644 index 000000000000..44882eee67b1 --- /dev/null +++ b/.changeset/lazy-rockets-raise.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes a bug where `astro build` didn't create custom `404.html` and `500.html` when a certain combination of i18n options was applied diff --git a/.changeset/mean-geckos-sell.md b/.changeset/mean-geckos-sell.md new file mode 100644 index 000000000000..5a72faa07f99 --- /dev/null +++ b/.changeset/mean-geckos-sell.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Update generator.ts to allow %23 (#) in dynamic urls diff --git a/.changeset/silly-parents-repair.md b/.changeset/silly-parents-repair.md new file mode 100644 index 000000000000..9157e6cc6ec6 --- /dev/null +++ b/.changeset/silly-parents-repair.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Improves debug logging for on-demand pages diff --git a/packages/astro/src/assets/utils/metadata.ts b/packages/astro/src/assets/utils/metadata.ts index 953349eb523e..8076d702fa19 100644 --- a/packages/astro/src/assets/utils/metadata.ts +++ b/packages/astro/src/assets/utils/metadata.ts @@ -6,22 +6,28 @@ export async function imageMetadata( data: Uint8Array, src?: string ): Promise> { - const result = probe(data); + try { + const result = probe(data); + if (!result.height || !result.width || !result.type) { + throw new AstroError({ + ...AstroErrorData.NoImageMetadata, + message: AstroErrorData.NoImageMetadata.message(src), + }); + } - if (!result.height || !result.width || !result.type) { + const { width, height, type, orientation } = result; + const isPortrait = (orientation || 0) >= 5; + + return { + width: isPortrait ? height : width, + height: isPortrait ? width : height, + format: type as ImageInputFormat, + orientation, + }; + } catch (e) { throw new AstroError({ ...AstroErrorData.NoImageMetadata, message: AstroErrorData.NoImageMetadata.message(src), }); } - - const { width, height, type, orientation } = result; - const isPortrait = (orientation || 0) >= 5; - - return { - width: isPortrait ? height : width, - height: isPortrait ? width : height, - format: type as ImageInputFormat, - orientation, - }; } diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 1ba5d9479833..5b941357839c 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -280,6 +280,14 @@ export class App { this.#logRenderOptionsDeprecationWarning(); } } + if (routeData) { + this.#logger.debug( + 'router', + 'The adapter ' + this.#manifest.adapterName + ' provided a custom RouteData for ', + request.url + ); + this.#logger.debug('router', 'RouteData:\n' + routeData); + } if (locals) { if (typeof locals !== 'object') { this.#logger.error(null, new AstroError(AstroErrorData.LocalsNotAnObject).stack!); @@ -296,8 +304,12 @@ export class App { } if (!routeData) { routeData = this.match(request); + this.#logger.debug('router', 'Astro matched the following route for ' + request.url); + this.#logger.debug('router', 'RouteData:\n' + routeData); } if (!routeData) { + this.#logger.debug('router', "Astro hasn't found routes that match " + request.url); + this.#logger.debug('router', "Here's the available routes:\n", this.#manifestData); return this.#renderError(request, { locals, status: 404 }); } const pathname = this.#getPathnameFromRequest(request); diff --git a/packages/astro/src/core/app/pipeline.ts b/packages/astro/src/core/app/pipeline.ts index f62dc84ed983..0f59a8b047d2 100644 --- a/packages/astro/src/core/app/pipeline.ts +++ b/packages/astro/src/core/app/pipeline.ts @@ -71,7 +71,10 @@ export class AppPipeline extends Pipeline { return module.page(); } - async tryRewrite(payload: RewritePayload): Promise<[RouteData, ComponentInstance]> { + async tryRewrite( + payload: RewritePayload, + request: Request + ): Promise<[RouteData, ComponentInstance]> { let foundRoute; for (const route of this.#manifestData!.routes) { @@ -86,9 +89,12 @@ export class AppPipeline extends Pipeline { foundRoute = route; break; } - } else if (route.pattern.test(decodeURI(payload))) { - foundRoute = route; - break; + } else { + const newUrl = new URL(payload, new URL(request.url).origin); + if (route.pattern.test(decodeURI(newUrl.pathname))) { + foundRoute = route; + break; + } } } diff --git a/packages/astro/src/core/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts index 11cff7c809f5..b4ba9e960407 100644 --- a/packages/astro/src/core/base-pipeline.ts +++ b/packages/astro/src/core/base-pipeline.ts @@ -71,7 +71,10 @@ export abstract class Pipeline { * * @param {RewritePayload} rewritePayload */ - abstract tryRewrite(rewritePayload: RewritePayload): Promise<[RouteData, ComponentInstance]>; + abstract tryRewrite( + rewritePayload: RewritePayload, + request: Request + ): Promise<[RouteData, ComponentInstance]>; /** * Tells the pipeline how to retrieve a component give a `RouteData` diff --git a/packages/astro/src/core/build/pipeline.ts b/packages/astro/src/core/build/pipeline.ts index 7661b6fc4d66..664d30c9ad7a 100644 --- a/packages/astro/src/core/build/pipeline.ts +++ b/packages/astro/src/core/build/pipeline.ts @@ -276,7 +276,10 @@ export class BuildPipeline extends Pipeline { } } - async tryRewrite(payload: RewritePayload): Promise<[RouteData, ComponentInstance]> { + async tryRewrite( + payload: RewritePayload, + request: Request + ): Promise<[RouteData, ComponentInstance]> { let foundRoute: RouteData | undefined; // options.manifest is the actual type that contains the information for (const route of this.options.manifest.routes) { @@ -291,9 +294,12 @@ export class BuildPipeline extends Pipeline { foundRoute = route; break; } - } else if (route.pattern.test(decodeURI(payload))) { - foundRoute = route; - break; + } else { + const newUrl = new URL(payload, new URL(request.url).origin); + if (route.pattern.test(decodeURI(newUrl.pathname))) { + foundRoute = route; + break; + } } } if (foundRoute) { diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 5f35c5f11309..4a5cad1da54c 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -4,7 +4,6 @@ import type { AstroGlobalPartial, ComponentInstance, MiddlewareHandler, - MiddlewareNext, RewritePayload, RouteData, SSRResult, @@ -118,7 +117,7 @@ export class RenderContext { if (payload) { if (this.pipeline.manifest.rewritingEnabled) { try { - const [routeData, component] = await pipeline.tryRewrite(payload); + const [routeData, component] = await pipeline.tryRewrite(payload, this.request); this.routeData = routeData; componentInstance = component; } catch (e) { @@ -212,7 +211,7 @@ export class RenderContext { const rewrite = async (reroutePayload: RewritePayload) => { pipeline.logger.debug('router', 'Called rewriting to:', reroutePayload); try { - const [routeData, component] = await pipeline.tryRewrite(reroutePayload); + const [routeData, component] = await pipeline.tryRewrite(reroutePayload, this.request); this.routeData = routeData; if (reroutePayload instanceof Request) { this.request = reroutePayload; @@ -398,7 +397,7 @@ export class RenderContext { const rewrite = async (reroutePayload: RewritePayload) => { try { pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload); - const [routeData, component] = await pipeline.tryRewrite(reroutePayload); + const [routeData, component] = await pipeline.tryRewrite(reroutePayload, this.request); this.routeData = routeData; if (reroutePayload instanceof Request) { this.request = reroutePayload; diff --git a/packages/astro/src/core/routing/manifest/generator.ts b/packages/astro/src/core/routing/manifest/generator.ts index 9395a3de2f05..734aee726dc9 100644 --- a/packages/astro/src/core/routing/manifest/generator.ts +++ b/packages/astro/src/core/routing/manifest/generator.ts @@ -2,6 +2,24 @@ import type { AstroConfig, RoutePart } from '../../../@types/astro.js'; import { compile } from 'path-to-regexp'; +/** + * Sanitizes the parameters object by normalizing string values and replacing certain characters with their URL-encoded equivalents. + * @param {Record} params - The parameters object to be sanitized. + * @returns {Record} The sanitized parameters object. + */ +function sanitizeParams( + params: Record +): Record { + return Object.fromEntries( + Object.entries(params).map(([key, value]) => { + if (typeof value === 'string') { + return [key, value.normalize().replace(/#/g, '%23').replace(/\?/g, '%3F')]; + } + return [key, value]; + }) + ); +} + export function getRouteGenerator( segments: RoutePart[][], addTrailingSlash: AstroConfig['trailingSlash'] @@ -37,8 +55,9 @@ export function getRouteGenerator( trailing = '/'; } const toPath = compile(template + trailing); - return (params: object): string => { - const path = toPath(params); + return (params: Record): string => { + const sanitizedParams = sanitizeParams(params); + const path = toPath(sanitizedParams); // When generating an index from a rest parameter route, `path-to-regexp` will return an // empty string instead "/". This causes an inconsistency with static indexes that may result diff --git a/packages/astro/src/i18n/index.ts b/packages/astro/src/i18n/index.ts index b8114019bdcf..3a12f699231f 100644 --- a/packages/astro/src/i18n/index.ts +++ b/packages/astro/src/i18n/index.ts @@ -19,6 +19,11 @@ export function requestHasLocale(locales: Locales) { }; } +export function requestIs404Or500(request: Request, base = '') { + const url = new URL(request.url); + + return url.pathname.startsWith(`${base}/404`) || url.pathname.startsWith(`${base}/500`); +} // Checks if the pathname has any locale export function pathHasLocale(path: string, locales: Locales): boolean { const segments = path.split('/'); @@ -317,7 +322,7 @@ export function notFound({ base, locales }: MiddlewarePayload) { if (!(isRoot || pathHasLocale(url.pathname, locales))) { if (response) { response.headers.set(REROUTE_DIRECTIVE_HEADER, 'no'); - return new Response(null, { + return new Response(response.body, { status: 404, headers: response.headers, }); diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts index df5d63761c74..9f9c7348e081 100644 --- a/packages/astro/src/i18n/middleware.ts +++ b/packages/astro/src/i18n/middleware.ts @@ -8,6 +8,7 @@ import { redirectToDefaultLocale, redirectToFallback, requestHasLocale, + requestIs404Or500, } from './index.js'; export function createI18nMiddleware( @@ -69,6 +70,11 @@ export function createI18nMiddleware( return response; } + // 404 and 500 are **known** routes (users can have their custom pages), so we need to let them be + if (requestIs404Or500(context.request, base)) { + return response; + } + const { currentLocale } = context; switch (i18n.strategy) { diff --git a/packages/astro/src/vite-plugin-astro-server/pipeline.ts b/packages/astro/src/vite-plugin-astro-server/pipeline.ts index 797c5d29abdd..260f7fa1fa86 100644 --- a/packages/astro/src/vite-plugin-astro-server/pipeline.ts +++ b/packages/astro/src/vite-plugin-astro-server/pipeline.ts @@ -191,7 +191,10 @@ export class DevPipeline extends Pipeline { } } - async tryRewrite(payload: RewritePayload): Promise<[RouteData, ComponentInstance]> { + async tryRewrite( + payload: RewritePayload, + request: Request + ): Promise<[RouteData, ComponentInstance]> { let foundRoute; if (!this.manifestData) { throw new Error('Missing manifest data. This is an internal error, please file an issue.'); @@ -209,9 +212,12 @@ export class DevPipeline extends Pipeline { foundRoute = route; break; } - } else if (route.pattern.test(decodeURI(payload))) { - foundRoute = route; - break; + } else { + const newUrl = new URL(payload, new URL(request.url).origin); + if (route.pattern.test(decodeURI(newUrl.pathname))) { + foundRoute = route; + break; + } } } diff --git a/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/500.astro b/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/500.astro new file mode 100644 index 000000000000..8bd0a9e216c5 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/500.astro @@ -0,0 +1,4 @@ + + Error + Unexpected error. + diff --git a/packages/astro/test/fixtures/reroute/src/middleware.js b/packages/astro/test/fixtures/reroute/src/middleware.js index 4d7c2a7956c8..09f3ef73c4ad 100644 --- a/packages/astro/test/fixtures/reroute/src/middleware.js +++ b/packages/astro/test/fixtures/reroute/src/middleware.js @@ -18,6 +18,10 @@ export const second = async (context, next) => { if (context.url.pathname.includes('/auth/base')) { return await next('/'); } + + if (context.url.pathname.includes('/auth/params')) { + return next('/?foo=bar'); + } } return next(); }; diff --git a/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro b/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro new file mode 100644 index 000000000000..3f4dfad8754b --- /dev/null +++ b/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro @@ -0,0 +1,10 @@ +--- +--- + + + Index with params + + +

Index with params

+ + diff --git a/packages/astro/test/i18n-routing-manual-with-default-middleware.test.js b/packages/astro/test/i18n-routing-manual-with-default-middleware.test.js index bf0f6e8b97a9..1e609f7487b1 100644 --- a/packages/astro/test/i18n-routing-manual-with-default-middleware.test.js +++ b/packages/astro/test/i18n-routing-manual-with-default-middleware.test.js @@ -26,7 +26,7 @@ describe('Dev server manual routing', () => { const response = await fixture.fetch('/blog'); const text = await response.text(); assert.equal(response.status, 404); - assert.equal(text.includes('Blog should not render'), false); + assert.match(text, /Blog should not render/); }); it('should return a 200 because the custom middleware allows it', async () => { @@ -83,7 +83,8 @@ describe('SSR manual routing', () => { let request = new Request('http://example.com/blog'); let response = await app.render(request); assert.equal(response.status, 404); - assert.equal((await response.text()).includes('Blog should not render'), false); + const text = await response.text(); + assert.match(text, /Blog should not render/); }); it('should return a 200 because the custom middleware allows it', async () => { diff --git a/packages/astro/test/i18n-routing.test.js b/packages/astro/test/i18n-routing.test.js index 9723b69dfadb..09581c899732 100644 --- a/packages/astro/test/i18n-routing.test.js +++ b/packages/astro/test/i18n-routing.test.js @@ -580,6 +580,16 @@ describe('[SSG] i18n routing', () => { assert.equal($('body').text().includes('Lo siento'), true); }); + it('should create a custom 404.html and 505.html', async () => { + let html = await fixture.readFile('/404.html'); + let $ = cheerio.load(html); + assert.equal($('body').text().includes("Can't find the page you're looking for."), true); + + html = await fixture.readFile('/500.html'); + $ = cheerio.load(html); + assert.equal($('body').text().includes('Unexpected error.'), true); + }); + it("should NOT render the default locale if there isn't a fallback and the route is missing", async () => { try { await fixture.readFile('/it/start/index.html'); diff --git a/packages/astro/test/rewrite.test.js b/packages/astro/test/rewrite.test.js index 1c76ce10af74..ef6a90e5dcc0 100644 --- a/packages/astro/test/rewrite.test.js +++ b/packages/astro/test/rewrite.test.js @@ -220,4 +220,11 @@ describe('Middleware', () => { assert.equal($('h1').text(), 'Index'); assert.equal($('p').text(), ''); }); + + it('should render the index when rewriting with params', async () => { + const html = await fixture.fetch('/auth/params').then((res) => res.text()); + const $ = cheerioLoad(html); + + assert.match($('h1').text(), /Index/); + }); }); diff --git a/packages/astro/test/ssr-params.test.js b/packages/astro/test/ssr-params.test.js index c5d376fbfce0..13c071e7d0f4 100644 --- a/packages/astro/test/ssr-params.test.js +++ b/packages/astro/test/ssr-params.test.js @@ -42,11 +42,11 @@ describe('Astro.params in SSR', () => { it('No double URL decoding', async () => { const app = await fixture.loadTestAdapterApp(); - const request = new Request('http://example.com/users/houston/%25'); + const request = new Request('http://example.com/users/houston/%25%23%3F'); const response = await app.render(request); assert.equal(response.status, 200); const html = await response.text(); const $ = cheerio.load(html); - assert.equal($('.category').text(), '%'); + assert.equal($('.category').text(), '%#?'); }); });