Skip to content

Commit

Permalink
dave - fixed bug with encoding of path parameter values into URLs
Browse files Browse the repository at this point in the history
  • Loading branch information
daviddenton committed Aug 2, 2015
1 parent ca4a388 commit 9cdb49b
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 8 deletions.
5 changes: 3 additions & 2 deletions RELEASE.md
Expand Up @@ -6,10 +6,11 @@ The main API is fairly stable now, but expect some amount of breaking changes ar
- Strictness checks around accepted content types, resulting in Unsupported Media Type (415) in case of mismatch.

#####v8.2.0
- Upgraded version of Finagle that we build against to v6.27.0
- Upgraded version of Finagle that we build against to v6.27.0.
- Bugfix for Path parameter values not being encoded correctly in clients.

#####v8.1.0
- Added native XML support as a body and a parameter type
- Added native XML support as a body and a parameter type.

#####v8.0.1
- Bugfix for cannot bind custom body to a request.
Expand Down
8 changes: 3 additions & 5 deletions src/main/scala/io/fintrospect/parameters/Path.scala
@@ -1,6 +1,6 @@
package io.fintrospect.parameters

import java.net.URI
import io.fintrospect.util.PathSegmentEncoderDecoder.{decode, encode}

import scala.util.Try

Expand Down Expand Up @@ -38,11 +38,9 @@ object Path extends Parameters[PathParameter, PathBindable] {

override def toString() = s"{$name}"

override def unapply(str: String) = Option(str).flatMap(s => {
Try(spec.deserialize(new URI("http://localhost/" + s).getPath.substring(1))).toOption
})
override def unapply(str: String) = Option(str).flatMap(s => Try(spec.deserialize(decode(s))).toOption)

override def -->(value: T) = Seq(new PathBinding(this, spec.serialize(value)))
override def -->(value: T) = Seq(new PathBinding(this, encode(spec.serialize(value))))

override def iterator = Option(this).iterator
}
Expand Down
11 changes: 11 additions & 0 deletions src/main/scala/io/fintrospect/util/PathSegmentEncoderDecoder.scala
@@ -0,0 +1,11 @@
package io.fintrospect.util

import java.net.URI

object PathSegmentEncoderDecoder {
def encode(in: String): String = {
in.split("/").map(new URI("http", "localhost", _).getRawFragment).mkString("%2F")
}

def decode(in: String): String = new URI("http://localhost/" + in).getPath.substring(1)
}
5 changes: 5 additions & 0 deletions src/test/scala/io/fintrospect/parameters/PathTest.scala
@@ -1,6 +1,7 @@
package io.fintrospect.parameters


import org.jboss.netty.handler.codec.http.HttpMethod
import org.scalatest.{FunSpec, ShouldMatchers}

class PathTest extends FunSpec with ShouldMatchers {
Expand Down Expand Up @@ -31,6 +32,10 @@ class PathTest extends FunSpec with ShouldMatchers {
it("does not url decode reserved characters") {
Path.string("urlEncoded").unapply(":@-._~!$&'()*+,;=") shouldEqual Option(":@-._~!$&'()*+,;=")
}

it("handles special characters when binding values") {
(Path.string("urlEncoded") --> "a path/+piece").head.apply(RequestBuild()).build(HttpMethod.GET).getUri shouldEqual "a%20path%2F+piece"
}
}

}
@@ -0,0 +1,22 @@
package io.fintrospect.util

import io.fintrospect.util.PathSegmentEncoderDecoder._
import org.scalatest.{FunSpec, ShouldMatchers}

class PathSegmentEncoderDecoder$Test extends FunSpec with ShouldMatchers{

describe("encode/decode") {
it("roundtrips") {
val inputString = " :@-._~!$&'()*+,;="
decode(encode(inputString)) shouldBe inputString
}

it("does not url encode reserved characters") {
encode(":@-._~!$&'()*+,;=") shouldEqual ":@-._~!$&'()*+,;="
}

it("handles spaces and forward slashes gracefully") {
encode("a path/+piece") shouldEqual "a%20path%2F+piece"
}
}
}
1 change: 0 additions & 1 deletion src/test/scala/presentation/_6/FakeRemoteLibrary.scala
Expand Up @@ -14,7 +14,6 @@ import presentation.Books
class FakeRemoteLibrary(books: Books) {
def search(titlePart: String) = new Service[HttpRequest, HttpResponse] {
override def apply(request: HttpRequest): Future[HttpResponse] = {
println("i go this", titlePart)
val results = books.titles().filter(_.toLowerCase.contains(titlePart.toLowerCase))
Future.value(PlainTextResponseBuilder.Ok(results.mkString(",")))
}
Expand Down

0 comments on commit 9cdb49b

Please sign in to comment.