Skip to content

Conversation

@ricksbrown
Copy link
Member

This is basically reverting to previous behaviour.
This needs to be thought out in detail before we start messing with URLs.
There is one unit test, my hunch is it needs at least 10 to cover the various permutations.

Before this change this http://www.example.com/&foo=fu%2Fbar would be rendered as this http://www.example.com/&foo=fu/bar and that's not right at all.

My gut feeling is that URL encoding (all or part of the URL) is the responsibility of the application developer. If we give it some thought maybe there is a way.... I guess if there's a space in the URL it hasn't been encoded (but maybe part of it has?)

@ghost
Copy link

ghost commented May 24, 2017

Sorry @ricksbrown I am closing this PR and re-doing it. Code change is good but test utility wasn't updated and you aren't available to grab your branch. It won't take long.

@ghost ghost closed this May 24, 2017
@ghost ghost mentioned this pull request May 24, 2017
ghost pushed a commit that referenced this pull request May 24, 2017
* Revert URL percent encoding

See PR #1211:

> This is basically reverting to previous behaviour. This needs to be
thought out in detail before we start messing with URLs. There is one
unit test, my hunch is it needs at least 10 to cover the various
permutations.

> Before this change this `http://www.example.com/&foo=fu%2Fbar` would
be rendered as this `http://www.example.com/&foo=fu/bar` and that's not
right at all.

> My gut feeling is that URL encoding (all or part of the URL) is the
responsibility of the application developer. If we give it some thought
maybe there is a way.... I guess if there's a space in the URL it
hasn't been encoded (but maybe part of it has?)

* changelog
@ricksbrown ricksbrown deleted the urldecoding branch May 26, 2017 02:01
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant