fix(node): Fix malformed URLs crashing the server in certain cases (#…

…6746)
withastro · Apr 4, 2023 · 4cc1bf6 · 4cc1bf6
1 parent 1ec1df1
commit 4cc1bf6
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 220 deletions.
diff --git a/.changeset/cyan-sheep-smile.md b/.changeset/cyan-sheep-smile.md
@@ -0,0 +1,5 @@
+---
+'@astrojs/node': patch
+---
+
+Fix malformed URLs crashing the server in certain cases
diff --git a/packages/astro/test/fixtures/ssr-api-route/package.json b/packages/astro/test/fixtures/ssr-api-route/package.json
@@ -3,7 +3,7 @@
   "version": "0.0.0",
   "private": true,
   "dependencies": {
-    "@astrojs/node": "^1.1.0",
+    "@astrojs/node": "workspace:*",
     "astro": "workspace:*"
   }
 }
diff --git a/packages/integrations/node/src/http-server.ts b/packages/integrations/node/src/http-server.ts
@@ -12,16 +12,32 @@ interface CreateServerOptions {
 	removeBase: (pathname: string) => string;
 }
 
+function parsePathname(pathname: string, host: string | undefined, port: number) {
+	try {
+		const urlPathname = new URL(pathname, `http://${host}:${port}`).pathname;
+		return decodeURI(encodeURI(urlPathname));
+	} catch (err) {
+		return undefined;
+	}
+}
+
 export function createServer(
 	{ client, port, host, removeBase }: CreateServerOptions,
 	handler: http.RequestListener
 ) {
 	const listener: http.RequestListener = (req, res) => {
 		if (req.url) {
-			let pathname = removeBase(req.url);
+			let pathname: string | undefined = removeBase(req.url);
 			pathname = pathname[0] === '/' ? pathname : '/' + pathname;
-			pathname = new URL(pathname, `http://${host}:${port}`).pathname;
-			const stream = send(req, encodeURI(decodeURI(pathname)), {
+			const encodedURI = parsePathname(pathname, host, port);
+
+			if (!encodedURI) {
+				res.writeHead(400);
+				res.end('Bad request.');
+				return res;
+			}
+
+			const stream = send(req, encodedURI, {
 				root: fileURLToPath(client),
 				dotfiles: pathname.startsWith('/.well-known/') ? 'allow' : 'deny',
 			});

diff --git a/packages/integrations/node/test/bad-urls.test.js b/packages/integrations/node/test/bad-urls.test.js
@@ -0,0 +1,46 @@
+import { expect } from 'chai';
+import nodejs from '../dist/index.js';
+import { loadFixture } from './test-utils.js';
+
+describe('API routes', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+	let devPreview;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/bad-urls/',
+			output: 'server',
+			adapter: nodejs({ mode: 'standalone' }),
+		});
+		await fixture.build();
+		devPreview = await fixture.preview();
+	});
+
+	after(async () => {
+		await devPreview.stop();
+	});
+
+	it('Does not crash on bad urls', async () => {
+		const weirdURLs = [
+			'/\\xfs.bxss.me%3Fastrojs.com/hello-world',
+			'/asdasdasd@ax_zX=.zxczas🐥%/úadasd000%/',
+			'%',
+			'%80',
+			'%c',
+			'%c0%80',
+			'%20foobar%',
+		];
+
+		for (const weirdUrl of weirdURLs) {
+			const fetchResult = await fixture.fetch(weirdUrl);
+			expect([400, 500]).to.include(
+				fetchResult.status,
+				`${weirdUrl} returned something else than 400 or 500`
+			);
+		}
+		const stillWork = await fixture.fetch('/');
+		const text = await stillWork.text();
+		expect(text).to.equal('<!DOCTYPE html>\nHello!');
+	});
+});
diff --git a/packages/integrations/node/test/fixtures/bad-urls/package.json b/packages/integrations/node/test/fixtures/bad-urls/package.json
@@ -0,0 +1,9 @@
+{
+  "name": "@test/nodejs-badurls",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/node": "workspace:*"
+  }
+}
diff --git a/packages/integrations/node/test/fixtures/bad-urls/src/pages/index.astro b/packages/integrations/node/test/fixtures/bad-urls/src/pages/index.astro
@@ -0,0 +1 @@
+Hello!