fix: allow relative paths with colons without ./ prefix#14714
fix: allow relative paths with colons without ./ prefix#147140372hoanghoccode wants to merge 2 commits intoremix-run:devfrom
Conversation
🦋 Changeset detectedLatest commit: 580875f The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
|
Welcome, and thank you for contributing to React Router! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run. Thanks! - The Remix team |
|
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳 |
|
Done! I've updated the \isAbsoluteUrl\ function to use the more specific regex and removed the inline regex from |
Fixes remix-run#14711 Updated ABSOLUTE_URL_REGEX to only match: - Hierarchical URLs (scheme://) - Protocol-relative URLs (//) - Common non-hierarchical schemes (mailto:, tel:, etc.) This allows paths like 'my-path:with-colon' to be resolved as relative paths without requiring a ./ prefix, while still preserving support for actual absolute URLs like mailto:, tel:, data:, etc. The regex is defined in isAbsoluteUrl() to avoid recompiling on every resolvePath() call.
8822656 to
5a3a044
Compare
|
Updated the implementation after additional testing revealed that the previous regex broke support for non-hierarchical URL schemes like mailto:, tel:, etc. The new regex now:
The regex is defined once in isAbsoluteUrl() to avoid recompiling on every resolvePath() call, as you suggested. All 862 router tests pass including the SSR tests that verify mailto: and other schemes work correctly. |
5a3a044 to
580875f
Compare
|
Thank you for the PR - but I don't think we want to change our detection to include a list of acceptable protocols. I think the real bug here is that we shouldn't have bypassed |
Fixes regression in v7.9.6 where paths like 'my-path:value' were incorrectly treated as absolute URLs. Now only hierarchical URLs (scheme://) and protocol-relative URLs (//) are treated as absolute, allowing relative paths with colons to work as expected.
Fixes #14711