diff --git a/.changeset/tidy-pumas-travel.md b/.changeset/tidy-pumas-travel.md new file mode 100644 index 000000000000..cee4088e7800 --- /dev/null +++ b/.changeset/tidy-pumas-travel.md @@ -0,0 +1,6 @@ +--- +'@sveltejs/kit': patch +'@sveltejs/adapter-node': patch +--- + +fix: preserve stripped path prefixes by making trailing-slash redirects relative diff --git a/packages/adapter-node/src/handler.js b/packages/adapter-node/src/handler.js index a32a63007332..e059b200d727 100644 --- a/packages/adapter-node/src/handler.js +++ b/packages/adapter-node/src/handler.js @@ -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() { @@ -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(); diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 188eab860e9e..4d6b101b0264 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -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'; @@ -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) } }); } diff --git a/packages/kit/src/utils/url.js b/packages/kit/src/utils/url.js index 42b4d9f3bea1..0e44ad3591aa 100644 --- a/packages/kit/src/utils/url.js +++ b/packages/kit/src/utils/url.js @@ -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 diff --git a/packages/kit/src/utils/url.spec.js b/packages/kit/src/utils/url.spec.js index 69dcb23a37c3..25e3f6b8d853 100644 --- a/packages/kit/src/utils/url.spec.js +++ b/packages/kit/src/utils/url.spec.js @@ -2,6 +2,7 @@ import { assert, describe } from 'vitest'; import { resolve, normalize_path, + relative_pathname, make_trackable, disable_search, matches_external_allowlist_entry @@ -73,6 +74,29 @@ describe('resolve', (test) => { }); }); +describe('relative_pathname', (test) => { + test('converts trailing-slash redirects to relative URL references', () => { + const cases = [ + ['/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); diff --git a/packages/kit/test/apps/options/test/test.js b/packages/kit/test/apps/options/test/test.js index d460728f4e4b..895cb9ef6212 100644 --- a/packages/kit/test/apps/options/test/test.js +++ b/packages/kit/test/apps/options/test/test.js @@ -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'; @@ -108,14 +107,9 @@ test.describe('env', () => { 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'); @@ -128,6 +122,10 @@ test.describe('trailingSlash', () => { }); 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');