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

Slashes (/) are not URL-encoded in Uri string representation in query parameters #2445

Closed
alex-iov42 opened this issue Mar 7, 2019 · 9 comments
Labels
bug Determined to be a bug in http4s module:core uri raze it and salt the earth whence it grew
Milestone

Comments

@alex-iov42
Copy link

Hi all,

We have been using http4s in our project and we recently noticed that slashes (/) are not URL-encoded in query parameters.
For example, let's say we have the following URL:

/some/url?param1=1%2B2%2F3

where basically the value of param1 is 1+2/3.

What we get from the URI's string representation is:

scala> val uri = Uri.uri("/some/url?param1=1%2B2%2F3")
uri: org.http4s.Uri = /some/url?param1=1%2B2/3

The reason we need this is that our clients are expected to sign the URL-encoded uri.
However, most of the other libraries and tools, URL-encode slashes properly when they appear in query parameters, which causes us problems when trying to validate our clients' signatures.

Is there a reason why this is happening? Is it a bug or a conscious decision? And if it's the latter, what is the reasoning behind it?

@kevinmeredith
Copy link
Contributor

kevinmeredith commented Apr 26, 2019

I see the same behavior with the following:

$amm
Loading...
Welcome to the Ammonite Repl 1.0.3
(Scala 2.12.4 Java 1.8.0_112)
If you like Ammonite, please support our development at www.patreon.com/lihaoyi
@ import $ivy.`org.http4s::http4s-core:0.20.0` 
imimport $ivy.$                               

@ import org.http4s.Uri 
import org.http4s.Uri

@ Uri.uri("a.b.c").withQueryParam("param1", "1+2/3").renderString 
res3: String = "a.b.c?param1=1%2B2/3"

@kevinmeredith
Copy link
Contributor

kevinmeredith commented Apr 26, 2019

Uri#uri uses Uri#fromString.

Uri#fromString includes a comment:

/** Decodes the String to a [[Uri]] using the RFC 3986 uri decoding specification */

https://tools.ietf.org/html/rfc3986#section-3.4 notes:

The characters slash ("/") and question mark ("?") may represent data
within the query component. Beware that some older, erroneous
implementations may not handle such data correctly when it is used as
the base URI for relative references (Section 5.1), apparently
because they fail to distinguish query data from path data when
looking for hierarchical separators. However, as query components
are often used to carry identifying information in the form of
"key=value" pairs and one frequently used value is a reference to
another URI, it is sometimes better for usability to avoid percent-
encoding those characters.

Looking at the parser of the Query, I see ? and / handled separately. I'm not too familiar with parboiled2, so I'm going to look at this code to try to understand it.

Can you please comment, @rossabaker, since, as I understand, git blame shows that you wrote that code?

@rossabaker
Copy link
Member

I think the last sentence of that RFC is why we stopped encoding those. A compliant implementation should treat these as equivalent, but it's a notoriously hard spec to get right.

It would be nice if we kept the original encoding in cases where the spec permits two equivalent encodings, to work around buggy implementations. This is a use case to consider when we redesign the Uri.

@programaker
Copy link

Hi!

Is there some workaround to change the encoding behavior? I'm doing something with Spotify API and the authorization process requires a redirect_uri query string parameter. The request fails with "Illegal request_uri" because the encoding is not what they expect.

But if a manually build the url using java.net.URLEncoder.encode(_, UTF_8) to encode the query params e put it in the browser, it works and I can see Spotify's authorization page.

If I try to do the same with the query parameters before passing them to Uri, the encoding I did is replaced with something else which also does not work.

@programaker
Copy link

I've found a way:

scala> :paste
// Entering paste mode (ctrl-D to finish)

import org.http4s.Uri
val uri2 = Uri(path = "http://foo.com/frunfles?redirect_uri=http://bar.com/quux")

// Exiting paste mode, now interpreting.

import org.http4s.Uri
uri2: org.http4s.Uri = http://foo.com/frunfles?redirect_uri=http://bar.com/quux

If I put everything in path and the Uri kept the String intact. Then I can encode myself before creating the Uri. With this approach, I could complete the authorization in Spotify and get the code.

Credits to @alexandredantas

@rossabaker
Copy link
Member

This is something that should be cleaned up in #3221, or a follow-up to it. Some relevant discussion from http4s-dev Gitter today.

@duchoang
Copy link

I have tried new version here https://github.com/http4s/http4s/releases/tag/v0.21.13 and it's already fixed this issue and the workaround with Uri(path = ...) doesn't work anymore, example:

scala> val uri = Uri.uri("/some/url?param1=1%2B2%2F3")
val uri: org.http4s.Uri = /some/url?param1=1%2B2%2F3

scala> val uri2 = Uri(path = "http://foo.com/frunfles?redirect_uri=http://bar.com/quux")
val uri2: org.http4s.Uri = ./http://foo.com/frunfles?redirect_uri=http://bar.com/quux

This PR seems to fix this issue accidentally 😄
https://github.com/http4s/http4s/pull/3752/files

@programaker
Copy link

Fortunately, I'm not using the path workaround anymore. I've replaced it with Uri.fromString, which doesn't encode the given uri String, only validates it returning an Either.

This is an Ammonite script to check it (v0.21.14):

@ {
  import $ivy.`org.http4s::http4s-dsl:0.21.14`

  import java.net.URLEncoder
  import java.nio.charset.StandardCharsets.UTF_8

  import org.http4s._
  import org.http4s.implicits._

  def encodeUrl(s: String): String = URLEncoder.encode(s, UTF_8)

  val a = "https://foo.com/frunfles?redirect_uri=https://bar.com/quux"
  val b = s"https://foo.com/frunfles?redirect_uri=${encodeUrl("https://bar.com/quux")}"

  val uriA = Uri.fromString(a)
  val uriB = Uri.fromString(b)

  println(s">>> A = '$uriA'")
  println(s">>> B = '$uriB'")
  }
>>> A = 'Right(https://foo.com/frunfles?redirect_uri=https://bar.com/quux)'
>>> B = 'Right(https://foo.com/frunfles?redirect_uri=https%3A%2F%2Fbar.com%2Fquux)'

@rossabaker
Copy link
Member

Still seems problematic. Needs to be part of a grand URI rethink.

scala> uri"/some/url?param1=1%2B2%2F3"
val res2: org.http4s.Uri = /some/url?param1=1%2B2/3

@rossabaker rossabaker added bug Determined to be a bug in http4s modules:core labels Apr 6, 2021
@rossabaker rossabaker added this to the 1.0 milestone Apr 6, 2021
@rossabaker rossabaker added module:core uri raze it and salt the earth whence it grew and removed modules:core labels Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Determined to be a bug in http4s module:core uri raze it and salt the earth whence it grew
Projects
None yet
Development

No branches or pull requests

5 participants