Skip to content

Commit

Permalink
Merge branch 'main' into feat/improve-astroenv-config-error
Browse files Browse the repository at this point in the history
  • Loading branch information
florian-lefebvre authored Jan 8, 2025
2 parents 62638df + ad2a752 commit 7a505a6
Show file tree
Hide file tree
Showing 19 changed files with 63 additions and 29 deletions.
5 changes: 5 additions & 0 deletions .changeset/afraid-peas-smash.md
Original file line number Diff line number Diff line change
@@ -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 `%`
5 changes: 5 additions & 0 deletions .changeset/old-clouds-act.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/mdx': patch
---

Fixes a bug that caused Image component to be imported on MDX pages that did not include images
14 changes: 13 additions & 1 deletion packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 3 additions & 4 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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';
Expand Down
6 changes: 5 additions & 1 deletion packages/astro/src/core/middleware/sequence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
}
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -90,7 +91,7 @@ export class RenderContext {
pipeline,
locals,
sequence(...pipeline.internalMiddleware, middleware ?? pipelineMiddleware),
decodeURI(pathname),
pathname,
request,
routeData,
status,
Expand All @@ -102,7 +103,6 @@ export class RenderContext {
partial,
);
}

/**
* The main function of the RenderContext.
*
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/render/params-and-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export async function getProps(opts: GetParamsAndPropsOptions): Promise<Props> {
// 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({
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/vite-plugin-astro-server/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
4 changes: 3 additions & 1 deletion packages/astro/src/vite-plugin-astro-server/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/test/content-collections-render.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ describe('Content Collections - render()', () => {
assert.equal($('ul li').length, 3);

// Includes styles
assert.equal($('head > style').length, 2);
assert.equal($('head > style').length, 1);
assert.ok($('head > style').text().includes("font-family: 'Comic Sans MS'"));
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export async function GET({ request }) {
return fetch("https://http.im/status/500", request)
return fetch("https://httpstat.us/500", request)
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export function getStaticPaths() {
{ params: { category: "%23something" } },
{ params: { category: "%2Fsomething" } },
{ params: { category: "%3Fsomething" } },
{ params: { category: "%25something" } },
{ params: { category: "[page]" } },
{ params: { category: "你好" } },
]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
---
export function getStaticPaths() {
return [
{ params: { category: "food" } },
{ params: { group: "food" } },
]
}
const { category } = Astro.params
const { group } = Astro.params
---
<html>
<head>
<title>Testing</title>
</head>
<body>
<h1>Testing</h1>
<h2 class="category">{ category }</h2>
<h2 class="category">{ group }</h2>
</body>
</html>
6 changes: 6 additions & 0 deletions packages/astro/test/params.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
2 changes: 1 addition & 1 deletion packages/astro/test/ssr-api-route.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ describe('API routes in SSR', () => {
const response = await fixture.fetch('/fail');
const text = await response.text();
assert.equal(response.status, 500);
assert.equal(text, '');
assert.equal(text, '500 Internal Server Error');
});

it('Has valid api context', async () => {
Expand Down
8 changes: 4 additions & 4 deletions packages/astro/test/units/dev/collections-renderentry.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('Content Collections - render()', () => {
assert.equal($('ul li').length, 3);

// Rendered the styles
assert.equal($('style').length, 2);
assert.equal($('style').length, 1);
},
);
});
Expand Down Expand Up @@ -158,7 +158,7 @@ describe('Content Collections - render()', () => {
assert.equal($('ul li').length, 3);

// Rendered the styles
assert.equal($('style').length, 2);
assert.equal($('style').length, 1);
},
);
});
Expand Down Expand Up @@ -225,7 +225,7 @@ describe('Content Collections - render()', () => {
assert.equal($('ul li').length, 3);

// Rendered the styles
assert.equal($('style').length, 2);
assert.equal($('style').length, 1);
},
);
});
Expand Down Expand Up @@ -291,7 +291,7 @@ describe('Content Collections - render()', () => {
assert.equal($('ul li').length, 3);

// Rendered the styles
assert.equal($('style').length, 2);
assert.equal($('style').length, 1);
},
);
});
Expand Down
6 changes: 3 additions & 3 deletions packages/integrations/mdx/src/rehype-images-to-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ function getImageComponentAttributes(props: Properties): MdxJsxAttribute[] {

export function rehypeImageToComponent() {
return function (tree: Root, file: VFile) {
if (!file.data.astro?.imagePaths) return;

if (!file.data.astro?.imagePaths?.length) return;
const importsStatements: MdxjsEsm[] = [];
const importedImages = new Map<string, string>();

visit(tree, 'element', (node, index, parent) => {
if (!file.data.astro?.imagePaths || node.tagName !== 'img' || !node.properties.src) return;
if (!file.data.astro?.imagePaths?.length || node.tagName !== 'img' || !node.properties.src)
return;

const src = decodeURI(String(node.properties.src));

Expand Down
6 changes: 3 additions & 3 deletions packages/integrations/mdx/test/css-head-mdx.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Head injection w/ MDX', () => {
const { document } = parseHTML(html);

const links = document.querySelectorAll('head link[rel=stylesheet]');
assert.equal(links.length, 2);
assert.equal(links.length, 1);

const scripts = document.querySelectorAll('script[type=module]');
assert.equal(scripts.length, 1);
Expand Down Expand Up @@ -67,7 +67,7 @@ describe('Head injection w/ MDX', () => {
const $ = cheerio.load(html);

const headLinks = $('head link[rel=stylesheet]');
assert.equal(headLinks.length, 2);
assert.equal(headLinks.length, 1);

const bodyLinks = $('body link[rel=stylesheet]');
assert.equal(bodyLinks.length, 0);
Expand All @@ -92,7 +92,7 @@ describe('Head injection w/ MDX', () => {
const $ = cheerio.load(html);

const headLinks = $('head link[rel=stylesheet]');
assert.equal(headLinks.length, 2);
assert.equal(headLinks.length, 1);

const bodyLinks = $('body link[rel=stylesheet]');
assert.equal(bodyLinks.length, 0);
Expand Down
4 changes: 2 additions & 2 deletions packages/integrations/mdx/test/mdx-math.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('MDX math', () => {
const mjxContainer = document.querySelector('mjx-container[jax="SVG"]');
assert.notEqual(mjxContainer, null);

const mjxStyle = document.querySelectorAll('style')[1].innerHTML;
const mjxStyle = document.querySelectorAll('style')[0].innerHTML;
assert.equal(
mjxStyle.includes('mjx-container[jax="SVG"]'),
true,
Expand Down Expand Up @@ -62,7 +62,7 @@ describe('MDX math', () => {
const mjxContainer = document.querySelector('mjx-container[jax="CHTML"]');
assert.notEqual(mjxContainer, null);

const mjxStyle = document.querySelectorAll('style')[1].innerHTML;
const mjxStyle = document.querySelectorAll('style')[0].innerHTML;
assert.equal(
mjxStyle.includes('mjx-container[jax="CHTML"]'),
true,
Expand Down

0 comments on commit 7a505a6

Please sign in to comment.