Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/dry-hornets-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/adapter-node': minor
---

validate ORIGIN env var at startup
13 changes: 11 additions & 2 deletions packages/adapter-node/src/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,22 @@ import { getRequest, setResponse, createReadableStream } from '@sveltejs/kit/nod
import { Server } from 'SERVER';
import { manifest, prerendered, base } from 'MANIFEST';
import { env } from 'ENV';
import { parse_as_bytes } from '../utils.js';
import { parse_as_bytes, parse_origin } from '../utils.js';

/* global ENV_PREFIX */

const server = new Server(manifest);

const origin = env('ORIGIN', undefined);
const origin = parse_origin(env('ORIGIN', undefined));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've slightly changed how we parse origin stuff (see get_origin, which we fall back to when the env var isn't set). It'd be nice to be able to share logic between these somehow...


if (origin === undefined && env('ORIGIN', undefined) !== undefined) {
throw new Error(
`Invalid ORIGIN: '${env('ORIGIN', undefined)}'. ` +
`ORIGIN must be a valid URL with http:// or https:// protocol. ` +
`For example: 'http://localhost:3000' or 'https://my.site'`
);
}

const xff_depth = parseInt(env('XFF_DEPTH', '1'));
const address_header = env('ADDRESS_HEADER', '').toLowerCase();
const protocol_header = env('PROTOCOL_HEADER', '').toLowerCase();
Expand Down
46 changes: 45 additions & 1 deletion packages/adapter-node/tests/utils.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect, test, describe } from 'vitest';
import { parse_as_bytes } from '../utils.js';
import { parse_as_bytes, parse_origin } from '../utils.js';

describe('parse_as_bytes', () => {
test('parses correctly', () => {
Expand All @@ -19,3 +19,47 @@ describe('parse_as_bytes', () => {
});
});
});

describe('parse_origin', () => {
test('valid origins return normalized origin', () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use test.each here as it allows us to give each test its own name and lines up with the rest of the codebase better

const testCases = [
{ input: 'http://localhost:3000', expected: 'http://localhost:3000' },
{ input: 'https://example.com', expected: 'https://example.com' },
{ input: 'http://192.168.1.1:8080', expected: 'http://192.168.1.1:8080' },
{ input: 'https://my-site.com', expected: 'https://my-site.com' },
{ input: 'http://localhost', expected: 'http://localhost' },
{ input: 'https://example.com:443', expected: 'https://example.com' },
{ input: 'http://example.com:80', expected: 'http://example.com' },

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we stripping the port numbers here? This should be valid AFAIK

{ input: undefined, expected: undefined }
];

testCases.forEach(({ input, expected }) => {
const actual = parse_origin(input);
expect(actual, `Testing input '${input}'`).toBe(expected);
});
});

test('URLs with path/query/hash are normalized to origin', () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should just be an error. You shouldn't be providing an invalid origin as ORIGIN.

const testCases = [
{ input: 'http://localhost:3000/path', expected: 'http://localhost:3000' },
{ input: 'http://localhost:3000?query=1', expected: 'http://localhost:3000' },
{ input: 'http://localhost:3000#hash', expected: 'http://localhost:3000' },
{ input: 'https://example.com/path/to/page', expected: 'https://example.com' },
{ input: 'https://example.com:443/path?query=1#hash', expected: 'https://example.com' }
];

testCases.forEach(({ input, expected }) => {
const actual = parse_origin(input);
expect(actual, `Testing input '${input}'`).toBe(expected);
});
});

test('invalid origins return undefined', () => {
const invalidInputs = ['localhost:3000', 'example.com', '', ' ', 'ftp://localhost:3000'];

invalidInputs.forEach((input) => {
const actual = parse_origin(input);
expect(actual, `Testing input '${input}'`).toBeUndefined();
});
});
});
31 changes: 31 additions & 0 deletions packages/adapter-node/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,34 @@ export function parse_as_bytes(value) {
}[value[value.length - 1]?.toUpperCase()] ?? 1;
return Number(multiplier != 1 ? value.substring(0, value.length - 1) : value) * multiplier;
}

/**
* Parses and validates an origin URL.
*
* @param {string | undefined} value - Origin URL with http:// or https:// protocol
* @returns {string | undefined} The validated origin, or undefined if value is undefined
*/
export function parse_origin(value) {
if (value === undefined) {
return undefined;
}

const trimmed = value.trim();

if (trimmed === '') {
return undefined;
}

try {
const url = new URL(trimmed);

// Verify protocol is http or https
if (url.protocol !== 'http:' && url.protocol !== 'https:') {
return undefined;
}

return url.origin;
} catch {
return undefined;
}
}
Loading