fix: redirect trailing slash normalization relatively instead of against root by HoldYourWaffle · Pull Request #13719 · sveltejs/kit

HoldYourWaffle · 2025-04-16T13:57:36Z

Fixes #13718.
As mentioned there: Location is allowed to be relative, so I just... did that.
For the scenario listed in the issue that means /sveltekit/sverdle/ is redirected with Location: ../sverdle, which correctly resolves to /sveltekit/sverdle.

Remaining things to consider

1. Still need to inline the get-relative-path code that does the heavy lifting, but I'd like to confirm this is the correct solution first.
2. What should tests for this look like? Should I create a whole adapter-node+dynamic base project or keep it simple and just check if the path is relative?
3. adapter-node currently imports the internal relative_pathname utility through @sveltejs/kit/node, which feels a bit weird, is there a better place for this?
4. I'm not 100% confident this doesn't regress #2515, unfortunately there's no minimal reproduction and #4414 didn't add a test. I tried various combinations of slashes and protocol-ish paths, but even taking out #4414's fix I always got 404 rather than redirected. Is this perhaps taken care of elsewhere now? Perhaps @wbster or @benmccann can clarify.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
This message body should clearly illustrate what problems it solves.
Ideally, include a test that fails without this PR but passes with it.

Tests

Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

Sharing internal utilities through `@sveltejs/kit/node` feels... odd

changeset-bot · 2025-04-16T13:57:40Z

🦋 Changeset detected

Latest commit: 2f84edc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@sveltejs/adapter-node	Patch
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

teemingc · 2026-02-05T19:52:07Z

+		const inverted_trailing_slash =
+			pathname.at(-1) === '/' ? pathname.slice(0, -1) : pathname + '/';
+		if (prerendered.has(inverted_trailing_slash)) {
+			// ensure preservation of (possibly invisible) path prefixes
+			let location = relative_pathname(pathname, inverted_trailing_slash);


Shouldn't this have the correct path when the request comes in? If it doesn't, maybe that's something that should be fixed in Polka?

teemingc · 2026-02-05T19:56:01Z

Sorry for the late review. Here are some thoughts:

1. Still need to inline the get-relative-path code that does the heavy lifting, but I'd like to confirm this is the correct solution first.

I think we may be able to do without the extra dependency. We may be able to use our own solution. See

kit/packages/kit/src/runtime/server/page/render.js

Lines 114 to 119 in c690579

    
           const segments = event.url.pathname.slice(paths.base.length).split('/').slice(2); 
        
           base = segments.map(() => '..').join('/') || '.'; 
        
           // resolve e.g. '../..' against current location, then remove trailing slash 
        
           base_expression = `new URL(${s(base)}, location).pathname.slice(0, -1)`;

2. What should tests for this look like? Should I create a whole adapter-node+dynamic base project or keep it simple and just check if the path is relative?

Yeah, I think it's good to introduce a test app for adapter-node although we have none set up currently.

3. adapter-node currently imports the internal relative_pathname utility through @sveltejs/kit/node, which feels a bit weird, is there a better place for this?

We can't export internals through @sveltejs/kit/node because it's public in nature.

4. I'm not 100% confident this doesn't regress #2515, unfortunately there's no minimal reproduction and #4414 didn't add a test. I tried various combinations of slashes and protocol-ish paths, but even taking out #4414's fix I always got 404 rather than redirected. Is this perhaps taken care of elsewhere now? Perhaps @wbster or @benmccann can clarify.

We'll likely want to keep the ternary expression so that it doesn't regress.

teemingc

.

@HoldYourWaffle

) Fixes #13718. Supersedes #13719, credit to @HoldYourWaffle for the diagnosis and approach. The trailing slash redirects use absolute pathnames, so any prefix a parent server strips before the request reaches kit (an Express mount, #13702) is dropped and the browser lands outside the app. The prefix never reaches Polka's parser, so it can't be reconstructed server side. A relative Location resolves against the full browser URL and is prefix-agnostic. Per the review on #13719, the helper is implemented in kit rather than adding `get-relative-path`, stays internal instead of being exported from `@sveltejs/kit/node`, and adapter-node gets a local copy since its handler is bundled standalone. Both call sites only produce pathnames differing by a trailing slash, so it handles exactly that case. The #2515 guard becomes unnecessary, a relative reference never starts with `/`. The options app trailingSlash tests now assert the Location values and fail on the old code. The prerender crawler resolves locations with `new URL()`, so it follows the relative form unchanged. Verified end to end with an adapter-node build mounted under a stripped prefix, both the SSR and the prerendered redirect. No adapter-node test app here since #16305 is establishing that infrastructure. --- ### Please don't delete this checklist! Before submitting the PR, please make sure you do the following: - [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] This message body should clearly illustrate what problems it solves. - [x] Ideally, include a test that fails without this PR but passes with it. ### Tests - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check` ### Changesets - [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. Changesets that add features should be `minor` and those that fix bugs should be `patch`. Please prefix changeset messages with `feat:`, `fix:`, or `chore:`. ### Edits - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed. --------- Co-authored-by: HoldYourWaffle <holdyourwaffle@gmail.com>

teemingc · 2026-07-21T00:32:32Z

Closing in favour of #16431

HoldYourWaffle and others added 3 commits April 16, 2025 14:20

Temporarily install get-relative-path

949a949

Basic fix based on get-relative-path

502e6c2

Fix prerendered paths for adapter-node

2f84edc

Sharing internal utilities through `@sveltejs/kit/node` feels... odd

HoldYourWaffle mentioned this pull request Apr 20, 2025

Trailing slash normalization always redirects against root #13718

Closed

teemingc reviewed Feb 5, 2026

View reviewed changes

teemingc added the pkg:adapter-node label Feb 5, 2026

teemingc requested changes Feb 5, 2026

View reviewed changes

Nic-Polumeyv mentioned this pull request Jul 20, 2026

fix: preserve stripped path prefixes in trailing-slash redirects #16431

Merged

6 tasks

teemingc closed this Jul 21, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: redirect trailing slash normalization relatively instead of against root#13719

fix: redirect trailing slash normalization relatively instead of against root#13719
HoldYourWaffle wants to merge 3 commits into
sveltejs:mainfrom
HoldYourWaffle:fix--dynamic-base-trailing-slash-redirects

HoldYourWaffle commented Apr 16, 2025

Uh oh!

changeset-bot Bot commented Apr 16, 2025

Uh oh!

teemingc Feb 5, 2026

Uh oh!

teemingc commented Feb 5, 2026

Uh oh!

teemingc left a comment

Uh oh!

teemingc commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

HoldYourWaffle commented Apr 16, 2025

Remaining things to consider

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

Tests

Changesets

Edits

Uh oh!

changeset-bot Bot commented Apr 16, 2025

🦋 Changeset detected

Uh oh!

teemingc Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

teemingc commented Feb 5, 2026

Uh oh!

teemingc left a comment

Choose a reason for hiding this comment

Uh oh!

teemingc commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants