fix URL with unicode characters in MS Edge #344
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In MS Edge, the `location.pathname` is not url encoded, but other browsers do. This results in not found route when a url contains a special character. To fix the issue, we now make sure the `pathname` is always encoded, but we also make sure it's not encoded twice.
|
@ryanflorence @blainekasten I'm poking you because you seem to be the 2 most active code owners. Thanks in advance ! |
|
@ardeois I'm going to be tackling a bunch of reach/router issues on Friday. I'll dig into this then! |
|
Great thanks @blainekasten |
|
hi @blainekasten any update on this PR? Thanks ! |
|
@blainekasten Hi Blaine was this issue resolved using @ardeois pr? |
|
@ardeois thank you for your solution. my issue with special chars in ie 11 and ms edge has been fixed. @blainekasten please merge his pr. |
|
Sorry for the delay! Thanks for doing this. Looks like it's a safe change! |
|
Thanks @blainekasten for merging it ! 🎉 |
sophialittlejohn
added a commit
to rasch-dtc/router
that referenced
this pull request
Mar 23, 2021
* initialPathname for createMemorySource should split out search params * Fix typos in RouteComponent.md changed "renderd" -> "rendered" and one instance of "it's" -> "its" * Update Match.md * Fix typo in Link.md * Failing test, showing location.search and location.hash aren't parsed out from the url in <ServerLocation /> - all of url is put in pathname * Implement parsing location.search and location.hash from the url in <ServerLocation />, and also remove them from pathname, to make behaviour same as client. Tests now pass. * Add new passing test (with snapshot) showing correct client behaviour of parsing out pathname, search & host from url. Other tests' snapshots now fail due to addition of hash to test history util Forgot to add history.js changes in last commit, test now passes * update snapshots with expected changes to make failing tests pass * Fixed bug when `navigate(...)` is called before `LocationProvider` is mounted * Adds missing quotation mark in error message * Fix small typo * fix: escape path in isCurrent check * Add back functionality to navigate route * Fix typo in nesting.md * Pre-populate states This is to ensure it stays in sync with `stack` as `index` is used for both. Otherwise, they're always off-by-one. * Update corresponding jest snapshot. * Remove hash from ServerLocation Servers don’t get hashes, so we don’t need this code. * fixup search string handling in createMemoryHistory * Add support for fragments * Run all the tests, thanks :P * add proptype for link * Exporting the match helper * s/due/do * Typo in nesting.md * Pin create-react-context due to non open source licence See: react-navigation/react-navigation#4934 for general gist and: jamiebuilds/create-react-context@392ef2b for the breaking change in a minor version. Jamiebuilds is acting in bad faith to the open source community and this should not propagate down the line. * Lint code on TravisCI * Added a basic guide for use with TypeScript * Added overview content * GitHub has a capital H * fix: pass router location prop to nested paths * Remove deep on warning * Link to typescript guide * Reword a bit * 1.2.2-0 * 1.3.0-beta.0 * Add server configuration guide * Add displayName to Link * Don’t copy everything from history.location Closes reach#252 Closes reach#264 * Failing test for memory history location.search Refs reach#251 * Add `?` to location.search in memory history Fixes reach#251 * Upgrade create-react-context to 0.3.0 This version is back to the MIT license, and avoids the dependency on fbjs which triggers warnings about unmaintained dependencies. * Update accessibility.md (reach#317) * Support for named trailing wildcard (reach#323) * Add support for named trailing wildcard * Add tests for Match * Update doc * Fix the getProps example (reach#306) According to https://github.com/reach/router/blob/master/src/index.js#L414 the `getProps` function cannot return `null`. * Fix/active routes push new state to history (reach#302) * add shallowCompare function * prevent Link from pushing new state to history when on same path and with the same state * Fix failing unit tests (reach#333) Run prettier and clean up * remove group role from focus wrapper * bump to 1.3 in docs * update snapshots * Bump lodash from 4.17.10 to 4.17.15 in /website (reach#331) Bumps [lodash](https://github.com/lodash/lodash) from 4.17.10 to 4.17.15. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.10...4.17.15) Signed-off-by: dependabot[bot] <[email protected]> * Bump mixin-deep from 1.3.1 to 1.3.2 in /website (reach#330) Bumps [mixin-deep](https://github.com/jonschlinkert/mixin-deep) from 1.3.1 to 1.3.2. - [Release notes](https://github.com/jonschlinkert/mixin-deep/releases) - [Commits](jonschlinkert/mixin-deep@1.3.1...1.3.2) Signed-off-by: dependabot[bot] <[email protected]> * Bump react-dom from 16.4.1 to 16.4.2 (reach#329) Bumps [react-dom](https://github.com/facebook/react/tree/HEAD/packages/react-dom) from 16.4.1 to 16.4.2. - [Release notes](https://github.com/facebook/react/releases) - [Changelog](https://github.com/facebook/react/blob/master/CHANGELOG.md) - [Commits](https://github.com/facebook/react/commits/v16.4.2/packages/react-dom) Signed-off-by: dependabot[bot] <[email protected]> * fix createNamedContext (reach#305) * Add Hooks APIs (reach#346) * Add Hooks APIs * Documenation & change useMatch response to match Match * add hook API navigation links * some hooks documentation in the intro * address comments * yarn format * 1.3.0 * useRouteMatch => useMatch, add null context warnings (reach#347) * Fix reach#348 - Pass href to location (reach#350) * 1.3.1 * fix MS Edge with encoded characters url (reach#344) In MS Edge, the `location.pathname` is not url encoded, but other browsers do. This results in not found route when a url contains a special character. To fix the issue, we now make sure the `pathname` is always encoded, but we also make sure it's not encoded twice. * Access `navigate` via `props` in example (reach#355) * Crash on old android devices fixed. Location pathname does not exist sometimes. (reach#339) * go method added into MemorySource for best compatibility (reach#307) * 1.3.3 * Update copyright year * Support forwarding url params with wildcard paths (reach#311) * fix for query params in insert params * fix for query params in insert params * fix for query params in insert params * fix for query params in insert params * adding test for location search * 1.3.4 * Fixes useNavigate inequality with props.navigate (reach#388) * tests(useNavigate): useNavigate not equals to props.navigate * fix(useNavigate): match the useNavigate with props.navigate * fix(useNavigate): add navigate to BaseContext * Typo fix "makes sure" 2x (reach#379) * Fix small typo in Server Configuration docs (reach#378) * Bump acorn from 5.5.3 to 5.7.4 in /website (reach#376) Bumps [acorn](https://github.com/acornjs/acorn) from 5.5.3 to 5.7.4. - [Release notes](https://github.com/acornjs/acorn/releases) - [Commits](acornjs/acorn@5.5.3...5.7.4) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump websocket-extensions from 0.1.3 to 0.1.4 in /website (reach#395) Bumps [websocket-extensions](https://github.com/faye/websocket-extensions-node) from 0.1.3 to 0.1.4. - [Release notes](https://github.com/faye/websocket-extensions-node/releases) - [Changelog](https://github.com/faye/websocket-extensions-node/blob/master/CHANGELOG.md) - [Commits](faye/websocket-extensions-node@0.1.3...0.1.4) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Intro.md: fix import instructions (reach#380) * Issue: wrong pathname encoding (reach#390) * Issue: wrong pathname encoding The encoding on pathname fails when the URL contains `%2F`, and returns the pathname with `%252F` instead. this is because `encodeURI(decodeURI('%2F')) === "%252F"` Also see: https://codesandbox.io/s/reach-router-encoding-issue-pofyo * fix: Encoded pathname Encoding can now handle side cases where URLs include both encoded and decoded parts that `encode/decodeURI` can't handle, such as `/folder/a%2Fb` * Bump http-proxy from 1.17.0 to 1.18.1 in /website (reach#420) Bumps [http-proxy](https://github.com/http-party/node-http-proxy) from 1.17.0 to 1.18.1. - [Release notes](https://github.com/http-party/node-http-proxy/releases) - [Changelog](https://github.com/http-party/node-http-proxy/blob/master/CHANGELOG.md) - [Commits](http-party/node-http-proxy@1.17.0...1.18.1) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump serve from 6.5.8 to 10.1.2 in /website (reach#419) Bumps [serve](https://github.com/zeit/serve) from 6.5.8 to 10.1.2. - [Release notes](https://github.com/zeit/serve/releases) - [Commits](vercel/serve@6.5.8...10.1.2) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump prismjs from 1.14.0 to 1.21.0 in /website (reach#411) Bumps [prismjs](https://github.com/PrismJS/prism) from 1.14.0 to 1.21.0. - [Release notes](https://github.com/PrismJS/prism/releases) - [Changelog](https://github.com/PrismJS/prism/blob/master/CHANGELOG.md) - [Commits](PrismJS/prism@v1.14.0...v1.21.0) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump lodash from 4.17.15 to 4.17.19 in /website (reach#407) Bumps [lodash](https://github.com/lodash/lodash) from 4.17.15 to 4.17.19. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.15...4.17.19) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump prismjs from 1.17.1 to 1.21.0 (reach#412) Bumps [prismjs](https://github.com/PrismJS/prism) from 1.17.1 to 1.21.0. - [Release notes](https://github.com/PrismJS/prism/releases) - [Changelog](https://github.com/PrismJS/prism/blob/master/CHANGELOG.md) - [Commits](PrismJS/prism@v1.17.1...v1.21.0) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump elliptic from 6.4.0 to 6.5.3 in /website (reach#410) Bumps [elliptic](https://github.com/indutny/elliptic) from 6.4.0 to 6.5.3. - [Release notes](https://github.com/indutny/elliptic/releases) - [Commits](indutny/elliptic@v6.4.0...v6.5.3) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * DTCPP-1578: Re-add query after merging * DTCPP-1578: Update build * DTCPP-1578: Move typing to package to add query to ServerLocation types Co-authored-by: Jake Richan <[email protected]> Co-authored-by: Daniel Schlabach <[email protected]> Co-authored-by: Hu Chen <[email protected]> Co-authored-by: Eric Jinks <[email protected]> Co-authored-by: Dave Williams <[email protected]> Co-authored-by: Michael Tamm <[email protected]> Co-authored-by: Joel Gallant <[email protected]> Co-authored-by: Pavel Zastavnitskiy <[email protected]> Co-authored-by: Stefan Probst <[email protected]> Co-authored-by: Anna Stasiuk <[email protected]> Co-authored-by: devisscher <[email protected]> Co-authored-by: Alexander Trauzzi <[email protected]> Co-authored-by: Ryan Florence <[email protected]> Co-authored-by: Kristofer Selbekk <[email protected]> Co-authored-by: Colin Taylor <[email protected]> Co-authored-by: SmirnovW <[email protected]> Co-authored-by: Cameron Matheson <[email protected]> Co-authored-by: Kuirak <[email protected]> Co-authored-by: Aaron Haaf <[email protected]> Co-authored-by: Rouven Weßling <[email protected]> Co-authored-by: Josh Tynjala <[email protected]> Co-authored-by: devuxer <[email protected]> Co-authored-by: David Baumgold <[email protected]> Co-authored-by: Eric Taylor <[email protected]> Co-authored-by: rubenmoya <[email protected]> Co-authored-by: Christophe Coevoet <[email protected]> Co-authored-by: Blaine Kasten <[email protected]> Co-authored-by: Mae Capozzi <[email protected]> Co-authored-by: Kai Hao <[email protected]> Co-authored-by: Stefano Magni <[email protected]> Co-authored-by: Johnny Zabala <[email protected]> Co-authored-by: chancestrickland <[email protected]> Co-authored-by: Chance Strickland <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Alex Rohleder <[email protected]> Co-authored-by: Corentin Ardeois <[email protected]> Co-authored-by: ray hatfield <[email protected]> Co-authored-by: Maksim <[email protected]> Co-authored-by: VariableVasas <[email protected]> Co-authored-by: Sudhir Mitharwal <[email protected]> Co-authored-by: Dan Dascalescu <[email protected]> Co-authored-by: Amr Salama <[email protected]> Co-authored-by: Sveta Slepner <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In MS Edge, the
location.pathnameis not url encoded, but other browsers encode it.This results in not found route when a url contains a unicode character.
To fix the issue, we now make sure the
pathnameis always encoded, but we also make sure it's not encoded twice.See unit tests for more details.
I've opened this pull requests without any prior discussion, so feel free to challenge the way this was done !
Related Issues
This fixes #343