Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Fix issue with double encoding on space in secret history route #10596

Merged
merged 6 commits into from
Jan 4, 2021

Conversation

Monkeychip
Copy link
Contributor

@Monkeychip Monkeychip commented Dec 17, 2020

This fix address PR issue 10418

We call a method encodePath which is apart of a third-party library. By default, this method encodes spaces from to %20. The problem is that we have already encoded white spaces before it gets sent to this method, such that the white spaces essential get encoded twice resulting in %2520 instead of %20. To fix this, I replaced the already encoded space using regex back to whitespace before it gets sent to this method.

Because the URL is getting sent to encodePath no matter what, I don't see any issues here, but it's worth some consideration.

Here is the library for the encodePath and here is a PR that talks about the special characters it does not encode (white space is not included).

With fix: The version page now shows where before it didn't.
image

Without fix: Double encoding on the spaces shows up at %2520
image

From a helpful Stackoverflow article

The common space character is encoded as %20 as you noted yourself. The % character is encoded as %25.
The way you get %2520 is when your url already has a %20 in it, and gets urlencoded again, which transforms the %20 to %2520

@Monkeychip Monkeychip added ui bug Used to indicate a potential bug labels Dec 17, 2020
@Monkeychip Monkeychip modified the milestones: 1.7, 1.6.2 Dec 17, 2020
Copy link
Contributor

@chelshaw chelshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

Copy link
Contributor

@chelshaw chelshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Monkeychip Monkeychip merged commit 702929e into master Jan 4, 2021
Monkeychip added a commit that referenced this pull request Jan 4, 2021
…10596)

* setup for concept it works, but probably not the best solution

* add comment and remove console and test var

* use normalize path higher up to fix issu

* add test for bug that fixing

* forgot a couple of changes

* changelog
Monkeychip added a commit that referenced this pull request Jan 4, 2021
…10596)

* setup for concept it works, but probably not the best solution

* add comment and remove console and test var

* use normalize path higher up to fix issu

* add test for bug that fixing

* forgot a couple of changes

* changelog
Monkeychip added a commit that referenced this pull request Jan 4, 2021
…10596) (#10642)

* setup for concept it works, but probably not the best solution

* add comment and remove console and test var

* use normalize path higher up to fix issu

* add test for bug that fixing

* forgot a couple of changes

* changelog
@Monkeychip Monkeychip deleted the ui/bug-fix-space-encoding branch June 16, 2022 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport bug Used to indicate a potential bug ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants