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

Is strict-uri-encode really necessary? #42

Closed
wmertens opened this issue Dec 10, 2015 · 6 comments
Closed

Is strict-uri-encode really necessary? #42

wmertens opened this issue Dec 10, 2015 · 6 comments

Comments

@wmertens
Copy link

I am using https://github.com/Sage/jsurl to efficiently encode JSON objects in the search string, and it uses the looser URI encoding range used by browsers, including the ' character.

I also use rackt/history which uses query-string, which converts ' to %27, which grows my urls, which makes me sad. Is there a reason the strict URI range should be used?

@sindresorhus
Copy link
Owner

It was introduced in #29.

// @go-oleg @kevva

@wmertens
Copy link
Author

Aww :-(

How about putting strict mode behind a flag?

On Thu, Dec 10, 2015, 6:19 PM Sindre Sorhus notifications@github.com
wrote:

It was introduced in #29
#29.

// @go-oleg https://github.com/go-oleg @kevva https://github.com/kevva


Reply to this email directly or view it on GitHub
#42 (comment)
.

Wout.
(typed on mobile, excuse terseness)

@SamVerschueren
Copy link
Contributor

Maybe it's browser support related? Not sure though, just guessing here.

@wmertens
Copy link
Author

@SamVerschueren the reason for #29 afaict was that markdown parsers were parsing urls with strict interpretation of the relevant RFCs which caused URLs to not be completely linkified.

IMHO the markdown parser should be fixed, because markdown is a loose format that needs to be generous regarding its inputs. After all, anybody can copy a URL from a browser and it will have the looser URI character range.

So I would be really happy if query-string could revert its ways and make the behavior in #29 optional…

@sindresorhus
Copy link
Owner

I'm willing to put it behind an option if anyone does a good pull request with tests and docs, but it should be strict by default.

@wmertens
Copy link
Author

<3 thanks @SamVerschueren! Now to get rackt/history to do the same :)

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

Successfully merging a pull request may close this issue.

3 participants