Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions .changeset/tidy-pumas-travel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sveltejs/kit': patch
'@sveltejs/adapter-node': patch
---

fix: preserve stripped path prefixes by making trailing-slash redirects relative
19 changes: 16 additions & 3 deletions packages/adapter-node/src/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,19 @@ function serve(path, client = false) {
: undefined;
}

/**
* Relative reference from `from` to `to`, which must differ only by a trailing slash.
* Keep in sync with the copy in `packages/kit/src/utils/url.js`
* @param {string} from
* @param {string} to
* @returns {string}
*/
function relative_pathname(from, to) {
const segment = to.replace(/\/$/, '').split('/').at(-1);

return from.endsWith('/') ? `../${segment}` : `${segment}/`;
}

// required because the static file server ignores trailing slashes
/** @returns {import('polka').Middleware} */
function serve_prerendered() {
Expand All @@ -79,9 +92,9 @@ function serve_prerendered() {
}

// remove or add trailing slash as appropriate
let location = pathname.at(-1) === '/' ? pathname.slice(0, -1) : pathname + '/';
if (prerendered.has(location)) {
if (query) location += search;
const inverted = pathname.at(-1) === '/' ? pathname.slice(0, -1) : pathname + '/';
if (prerendered.has(inverted)) {
const location = relative_pathname(pathname, inverted) + (query ? search : '');
res.writeHead(308, { location }).end();
} else {
void next();
Expand Down
12 changes: 8 additions & 4 deletions packages/kit/src/runtime/server/respond.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import { respond_with_error } from './page/respond_with_error.js';
import { get_self_origin, is_csrf_forbidden, is_remote_forbidden } from './csrf.js';
import { has_prerendered_path, method_not_allowed, redirect_response } from './utils.js';
import { handle_fatal_error } from './errors.js';
import { decode_pathname, disable_search, normalize_path } from '../../utils/url.js';
import {
decode_pathname,
disable_search,
normalize_path,
relative_pathname
} from '../../utils/url.js';
import { find_route } from '../../utils/routing.js';
import { redirect_json_response, render_data } from './data/index.js';
import { add_cookies_to_headers, get_cookies } from './cookie.js';
Expand Down Expand Up @@ -378,10 +383,9 @@ export async function internal_respond(request, options, manifest, state) {
status: 308,
headers: {
'x-sveltekit-normalize': '1',
// relative so (possibly invisible) path prefixes are preserved
location:
// ensure paths starting with '//' are not treated as protocol-relative
(normalized.startsWith('//') ? url.origin + normalized : normalized) +
(url.search === '?' ? '' : url.search)
relative_pathname(url.pathname, normalized) + (url.search === '?' ? '' : url.search)
}
});
}
Expand Down
12 changes: 12 additions & 0 deletions packages/kit/src/utils/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ export function is_root_relative(path) {
return path[0] === '/' && path[1] !== '/';
}

/**
* Relative reference from `from` to `to`, which must differ only by a trailing slash
* @param {string} from
* @param {string} to
* @returns {string}
*/
export function relative_pathname(from, to) {
const segment = to.replace(/\/$/, '').split('/').at(-1);

return from.endsWith('/') ? `../${segment}` : `${segment}/`;
}

/**
* @param {string} location
* @param {string} allowed
Expand Down
24 changes: 24 additions & 0 deletions packages/kit/src/utils/url.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { assert, describe } from 'vitest';
import {
resolve,
normalize_path,
relative_pathname,
make_trackable,
disable_search,
matches_external_allowlist_entry
Expand Down Expand Up @@ -73,6 +74,29 @@ describe('resolve', (test) => {
});
});

describe('relative_pathname', (test) => {
test('converts trailing-slash redirects to relative URL references', () => {
const cases = [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In the future, we can use test.each for better support by Vitest https://vitest.dev/api/test.html#test-each

['/a/b', '/a/b/', 'b/'],
['/a/b/', '/a/b', '../b'],
['/path-base/slash', '/path-base/slash/', 'slash/'],
['//x', '//x/', 'x/'],
['//x/', '//x', '../x'],
['/a/b%2Fc', '/a/b%2Fc/', 'b%2Fc/']
];

for (const [from, to, expected] of cases) {
const result = relative_pathname(from, to);
const base = new URL('http://internal');
base.pathname = from;

assert.equal(result, expected);
assert.equal(result.startsWith('/'), false);
assert.equal(new URL(result, base).pathname, to);
}
});
});

describe('matches_external_allowlist_entry', (test) => {
test('matches allowed origins', () => {
assert.equal(matches_external_allowlist_entry('https://google.de', 'https://google.de'), true);
Expand Down
16 changes: 7 additions & 9 deletions packages/kit/test/apps/options/test/test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as http from 'node:http';
import process from 'node:process';
import { expect } from '@playwright/test';
import { test } from '../../../utils.js';
Expand Down Expand Up @@ -108,14 +107,9 @@
test.describe('trailingSlash', () => {
test('adds trailing slash', async ({ baseURL, page, clicknav }) => {
// we can't use Playwright's `request` here, because it resolves redirects
const status = await new Promise((fulfil, reject) => {
const request = http.get(`${baseURL}/path-base/slash`);
request.on('error', reject);
request.on('response', (response) => {
fulfil(response.statusCode);
});
});
expect(status).toBe(308);
const response = await fetch(`${baseURL}/path-base/slash`, { redirect: 'manual' });
expect(response.status).toBe(308);
expect(response.headers.get('location')).toBe('slash/');

await page.goto('/path-base/slash');

Expand All @@ -128,6 +122,10 @@
});

test('removes trailing slash on endpoint', async ({ baseURL, request }) => {
const response = await fetch(`${baseURL}/path-base/endpoint/`, { redirect: 'manual' });
expect(response.status).toBe(308);
expect(response.headers.get('location')).toBe('../endpoint');

const r1 = await request.get('/path-base/endpoint/');
expect(r1.url()).toBe(`${baseURL}/path-base/endpoint`);
expect(await r1.text()).toBe('hi');
Expand Down Expand Up @@ -195,7 +193,7 @@
expect(requests).toEqual([]);
});

test('accounts for base path when running data-sveltekit-preload-code', async ({

Check warning on line 196 in packages/kit/test/apps/options/test/test.js

View workflow job for this annotation

GitHub Actions / test-kit (24, ubuntu-latest, chromium, current)

flaky test: accounts for base path when running data-sveltekit-preload-code

retries: 2
page,
javaScriptEnabled
}) => {
Expand Down
Loading