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

path should also be escaped #25

Open
domenkozar opened this issue Dec 19, 2018 · 5 comments
Open

path should also be escaped #25

domenkozar opened this issue Dec 19, 2018 · 5 comments
Labels
breaking would require a MAJOR release request

Comments

@domenkozar
Copy link

domenkozar commented Dec 19, 2018

Url.Builder.absolute ["file", "/path/bla"] []

should result to

/file/%2Fpath%2Fbla

but currently results to

/file//path/bla

This is especially surprising due to documentation

Use Url.Builder instead! Functions like absolute, relative, and crossOrigin already do this automatically! percentEncode is only available so that extremely custom cases are possible, if needed.

@hpate-omicron
Copy link

This bug bit us as well, we were using an email address as a path param and it was working for simple cases, but not when there was a + in the email address.

This is the spec section for this piece that mentions percent encoding the path segements.
https://tools.ietf.org/html/rfc3986#section-3.3

@joshuakb2
Copy link

This caused a bug in my code. The behavior directly contradicts the documentation.

@drewwestphal
Copy link

Upvote.

This also caused a bug in my code. The part of the documentation that says it escapes is in the function you ultimately need to use to fix your url https://package.elm-lang.org/packages/elm/url/latest/Url#percentEncode

I will say that it DOES escape query parameters... but if you are building an API url for example and some parameters are path params (as they often are) and they contain spaces (as they sometimes do), this bug will get you.

@evancz
Copy link
Member

evancz commented Feb 10, 2021

If the proposed fix is to call percentEncode on all segments, a user from Russia or Denmark might prefer the current behavior. For example:

UB.absolute ["искать", "книги"] []
-- current:  "/искать/книги"
-- proposed: "/%D0%B8%D1%81%D0%BA%D0%B0%D1%82%D1%8C/%D0%BA%D0%BD%D0%B8%D0%B3%D0%B8"

UB.absolute ["søg", "bøger"] []
-- current:  "/søg/bøger"
-- proposed: "/s%C3%B8g/b%C3%B8ger"

So the current defaults assume that wanting UTF-8 chars in a path is more common than having path segments that contain / characters. I marked this issue as "breaking" and as a "request" since so many people could have very different opinions on the defaults than the people participating in this thread.

The meta issue #42 attempts to summarize the options and solicit examples and evidence that could inform any future change.

@pravdomil
Copy link

pravdomil commented Feb 11, 2021

Thought about this:
Encode char if char code is lower then 128?

["søg", "bø/ger"].map(a => a.split("").map(v => v.charCodeAt(0) < 128 ? encodeURIComponent(v) : v).join("")).join("/")

Produces:

"søg/bø%2Fger"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking would require a MAJOR release request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants