Skip to content

Conversation

@jong-kyung
Copy link
Contributor

@jong-kyung jong-kyung commented Jan 4, 2026

fixes #6288

This PR fixes an issue where using a pipe character (|) in route parameters caused an infinite redirect loop (ERR_TOO_MANY_REDIRECTS)

The Problem

Browsers automatically encode | characters to %7C in the URL when navigating directly. However, the router's internal normalization logic (cleanPath) was treating | as a raw character.
This caused a mismatch between the current location (encoded by browser) and the expected location (unencoded by router), triggering a redirect loop where the router tried to redirect to the unencoded path, but the browser re-encoded it upon request.

The Fix

I updated the cleanPath utility function to explicitly normalize | characters to %7C. This ensures that the router's internal path representation matches the browser's encoding behavior, preventing the unnecessary redirect loop.

Changes

  • packages/router-core/src/path.ts: Updated cleanPath to replace | with %7C.
  • e2e/react-router/basic-file-based/tests/app.spec.ts: Added an E2E test case to verify that routes with pipe characters in parameters render correctly without redirects.

Summary by CodeRabbit

  • New Features

    • Added support for routes containing special characters (including pipe character) in their paths
    • Special characters in route paths are now automatically percent-encoded for proper URL handling
  • Tests

    • Added tests verifying correct percent-encoding and navigation to routes with special characters

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 4, 2026

📝 Walkthrough

Walkthrough

The pull request adds support for the pipe character (|) in route paths by updating the path encoding utility to percent-encode pipes as %7C. It includes a new test route with a pipe character, generated route tree updates, and e2e tests to verify proper URL encoding and navigation behavior.

Changes

Cohort / File(s) Summary
Path encoding fix
packages/router-core/src/path.ts
Updated cleanPath function to encode pipe character (`
Test route and generation
e2e/react-router/basic-file-based/src/routes/special|pipe.tsx, e2e/react-router/basic-file-based/src/routeTree.gen.ts
Added new route file with path `/special
E2E test coverage
e2e/react-router/basic-file-based/tests/app.spec.ts
Added test suite validating navigation to `/special

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

package: router-core

Suggested reviewers

  • schiller-manuel
  • nlynzaad

Poem

A rabbit hops down a winding trail,
Where pipes were breaking—a sad tale,
But now they're encoded, safe and sound,
%7C marks the spot where pipes are found! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: normalizing pipe characters in paths to prevent redirect loops, directly addressing the core issue resolved in the PR.
Linked Issues check ✅ Passed The PR directly addresses issue #6288 by updating cleanPath to normalize pipe characters to %7C [#6288] and adding E2E tests to verify the fix works correctly [#6288].
Out of Scope Changes check ✅ Passed All changes are scoped to the stated objectives: path normalization fix, E2E test for pipe characters, and generated route tree updates from the test route.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nlynzaad
Copy link
Contributor

nlynzaad commented Jan 5, 2026

@jong-kyung thanks for this. as part of my review process, I like to recreate the error with the e2e tests before validating the fix. I copied the e2e test over into main and the tests passed without a problem. I also tested this manually in the browser and could not replicate the problem.

We need to ensure the e2e test fails with the expected error and the solution resolves it correctly to ensure future breakages are avoided.

I do see the error in the linked issue but unfortunately, I don't see the source. Would it be possible to link to a github repo or stackblitz link where the error re-occurs?

@nlynzaad
Copy link
Contributor

nlynzaad commented Jan 5, 2026

I see now you aren't the author of the original issue. Do you possibly have a recreation of the original problem?

@jong-kyung
Copy link
Contributor Author

jong-kyung commented Jan 6, 2026

Hi @nlynzaad, thanks for reviewing

Apologies for the confusion regarding the E2E test. The test passed because I added it to a CSR-only. This issue (ERR_TOO_MANY_REDIRECTS) is specific to SSR environments (TanStack Start), caused by a mismatch between browser encoding and server-side normalization during direct navigation.

You can reproduce the issue using the links below:

Could you please confirm the issue using the reproduction links? Once confirmed, please let me know, and I will rewrite the tests to properly target the SSR environment based on your feedback.

Thanks again for your time and review, and sorry for the confusion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

App breaks when route params contain pipe character(i.e "|")

2 participants