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

Plus sign is not escaped correctly #48

Closed
anatoliykmetyuk opened this issue Nov 21, 2017 · 9 comments
Closed

Plus sign is not escaped correctly #48

anatoliykmetyuk opened this issue Nov 21, 2017 · 9 comments

Comments

@anatoliykmetyuk
Copy link

scala> uri"http://localhost:8081/+1"
res5: com.softwaremill.sttp.Uri = http://localhost:8081/%201

scala> uri"http://localhost:8081/ 1"
res6: com.softwaremill.sttp.Uri = http://localhost:8081/%201

scala> res5 == res6
res7: Boolean = true

Workaround:

scala> uri"http://localhost:8081/%2B1"
res10: com.softwaremill.sttp.Uri = http://localhost:8081/+1
@adamw
Copy link
Member

adamw commented Nov 21, 2017

Hmm I suppose we'll need context-sensitive decoding, not only encoding, wich makes sense :) The + in the query string should be converted to a (space), but not in the path

@anatoliykmetyuk
Copy link
Author

anatoliykmetyuk commented Nov 21, 2017

By query string, do you mean the named URI parameters? Actually my use case is a URI as follows:

http://localhost:8081/user?phone=+123456

The context is looking up by phone that is. Come to think about it, "+" is really used as a space in this context. However, does it make sense to still have it as a space in sttp? After all, you can perfectly use spaces in query string and they will be encoded properly.

@adamw
Copy link
Member

adamw commented Nov 21, 2017

If you want to include + in the query string, it should be %-encoded, as + is the escaped form of space (in that context).

The interpolator assumes that when you write down a literal URI (as a String, without embedding any expressions), it's in encoded form - hence it decodes any special characters. So:

uri"http://localhost:8081/user?phone=+123456" will be parsed as an URI with a single query parameter phone with the value 123456

uri"http://localhost:8081/user?phone=${"+123456"}" will be parsed as an URI with a single query parameter phone with the value +123456

@anatoliykmetyuk
Copy link
Author

anatoliykmetyuk commented Nov 21, 2017

Hm, then this does not seem to be working with get requests (maybe also with others). When I do:

sttp.get(uri"http://localhost:8888/appointment?phone=${"+1234"}").send()

And on the server side (http4s):

  val root = Root / "appointment"
/*...*/
    case req @ GET -> `root` :? PhoneParam(phoneRaw) =>
      println(s"Raw request parameter: $phoneRaw")

This prints:

Raw request parameter:  1234

@anatoliykmetyuk
Copy link
Author

Ah, come to think about it this may be http4s issue.

@adamw
Copy link
Member

adamw commented Nov 21, 2017

No, you are right, somehow the + is not escaped correctly. I'll take a look tomorrow.

adamw added a commit that referenced this issue Nov 21, 2017
@adamw
Copy link
Member

adamw commented Nov 21, 2017

I think it's an easy fix (only the + is problematic), see the commit. I'm publishing a snapshot (for some reason it gets published as 0.0.20-SNAPSHOT), but the release will definitely have to wait until tomorrow ;)

@anatoliykmetyuk
Copy link
Author

Awesome, thank you for the quick fix @adamw! I will check it out tomorrow.

@adamw
Copy link
Member

adamw commented Nov 22, 2017

Released 1.0.6

@adamw adamw closed this as completed Nov 28, 2017
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

No branches or pull requests

2 participants