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

whitespace in param #75

Closed
sshastry opened this issue Nov 20, 2014 · 3 comments
Closed

whitespace in param #75

sshastry opened this issue Nov 20, 2014 · 3 comments
Assignees
Labels
bug
Milestone

Comments

@sshastry
Copy link

@sshastry sshastry commented Nov 20, 2014

hey! having some trouble with the router in http4s v0.4.0.

object X extends ParamMatcher("x")
object Y extends ParamMatcher("y")

class App(hostname: String = "localhost", port: Int = 9999)(implicit ec: ExecutionContext = ExecutionContext.global) extends Logging {
  def run = BlazeBuilder.bindHttp(port, hostname).mountService(service, "/").run.awaitShutdown()
  def logreq(r: Request): Unit = log.info(s"${r.remoteAddr.getOrElse("null")} -> ${r.method}: ${r.uri.path} ${r.uri.query}")

  lazy val service = HttpService {
    case req @ GET -> Root / "blah" :? X(x0) +& Y(y0) =>
      logreq(req)
      log.info(s"x0=${x0}, y0=${y0}")
      Ok("x = " + x0 + "\n" + "y = " + y0)
    case req =>
      logreq(req)
      NotFound("404: not found")
  }
}

(here's the full file: https://gist.github.com/sshastry/874fa6ce2431e3725e9f)

http://localhost:8080/blah?x=abc&y=ijk works and binds "abc" to x0 and "ijk" to y0.

but http://localhost:8080/blah?x=a+bc&y=ijk falls through to the NotFound route, contrary to my expectation that it would bind "a bc" to x0. likewise for http://localhost:8080/blah?x=a%20bc&y=ijk.

this is what the log shows:

14:47:43.370 [pool-6-thread-1] INFO  org.app.App - 127.0.0.1 -> GET: /blah Some(x=a bc&y=ijk)

something simple I'm overlooking? thanks!

bryce-anderson added a commit that referenced this issue Nov 20, 2014
fixes #75
The QueryParser expects a url encoded string, but the UriParser
predecodes it. This fixes the problem at the cost of needing to
manually decode the query String if all you really want is the String.
bryce-anderson added a commit that referenced this issue Nov 21, 2014
Also added a few more tests to the UriSpec
issue #75.
@rossabaker rossabaker added the bug label Nov 21, 2014
@rossabaker rossabaker added this to the 0.4.1 milestone Nov 21, 2014
@bryce-anderson
Copy link
Member

@bryce-anderson bryce-anderson commented Nov 21, 2014

@sshastry this should be fixed, but can you confirm? if it works for you we will do a 0.4.1 release with this bug fix to get a working artifact out there.

@sshastry
Copy link
Author

@sshastry sshastry commented Nov 21, 2014

confirmed working.

@bryce-anderson
Copy link
Member

@bryce-anderson bryce-anderson commented Nov 21, 2014

Perfect, thank you.

bryce-anderson added a commit that referenced this issue Nov 21, 2014
Also added a few more tests to the UriSpec
issue #75.

Conflicts:
	core/src/main/scala/org/http4s/Uri.scala
	core/src/main/scala/org/http4s/parser/Rfc3986Parser.scala
	core/src/test/scala/org/http4s/UriSpec.scala
rossabaker pushed a commit that referenced this issue Mar 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.