Skip to content

Commit

Permalink
Fixes asString when query has 1 param with no value
Browse files Browse the repository at this point in the history 
When there was a key with no values in the query string,
there was a implicit conversion from string to list that
made the code compile but that messed up with the params.

That way, this query string `?hello` would be translated
into `?h&e&l&l&o`.

Signed-off-by: Albert Pastrana <albert.pastrana@gmail.com>
  • Loading branch information
@albertpastrana
albertpastrana committed May 8, 2018
1 parent 10aad22 commit 67cf0b4
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 31 deletions.
11 changes: 5 additions & 6 deletions url/src/main/scala/uscala/net/URL.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ case class URL(scheme: String,

import URL._

lazy val path = rawPath.map(URL.decode)
lazy val path: Option[String] = rawPath.map(URL.decode)

lazy val asURI: URI = new URI(asString)

Expand Down Expand Up @@ -112,20 +112,19 @@ final case class NonEmptyQuery(underlying: Map[String, List[String]]) extends Qu

override def asString: String =
"?" + underlying.flatMap {
case (key, Nil) => key
case (key, Nil) => List(key)
case (key, values) => values.map(v => s"$key=${URL.encode(v)}")
}.mkString("&")

}

object Query {

val empty = Empty
val empty: Empty.type = Empty

def apply(rawQuery: String): Try[Query] = Try {
Option(rawQuery).map { query =>
if (query.isEmpty) Map.empty[String, List[String]]
else query.split('&').map(_.split('=')).collect {
Option(rawQuery).collect { case query if query.nonEmpty =>
query.split('&').map(_.split('=')).collect {
case Array(key, value) => key -> URL.decode(value)
case Array(key) => key -> ""
case a @ Array(key, _*) => key -> a.tail.mkString("=")
Expand Down
28 changes: 23 additions & 5 deletions url/src/test/scala/uscala/net/QuerySpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ import org.specs2.specification.core.Fragment
class QuerySpec extends Specification with ScalaCheck with TestQueries {

"apply(String)" >> {
"when the query is null" >> {
"should return an empty Query" >> {
"should return an empty Query" >> {
"when the query is null" >> {
Query(null) must beASuccessfulTry.which(_.hasQuery must_=== false)
}
"when the query is empty" >> {
Query("") must beASuccessfulTry.which(_.hasQuery must_=== false)
}
}

Fragment.foreach(testQueries) { case (raw, params) =>
Expand All @@ -24,6 +27,18 @@ class QuerySpec extends Specification with ScalaCheck with TestQueries {
}
}

"asString" >> {
"should return empty string if it's an empty query" >> {
Query("").map(_.asString) must beASuccessfulTry("")
}
"should add the ? at the beginning of the query" >> {
Query("key=value").map(_.asString) must beASuccessfulTry("?key=value")
}
"should return only the key if there is no value" >> {
Query("key=").map(_.asString) must beASuccessfulTry("?key")
}
}

"Empty" >> {
"hasQuery should always return false" >> {
Empty.hasQuery must_== false
Expand All @@ -34,6 +49,9 @@ class QuerySpec extends Specification with ScalaCheck with TestQueries {
"getOrElse should always return the default value" >> {
Empty.getOrElse("a", List("b")) must_=== List("b")
}
"asString should return an empty string" >> {
Empty.asString must_=== ""
}
}

"NonEmptyQuery" >> {
Expand Down Expand Up @@ -62,15 +80,15 @@ class QuerySpec extends Specification with ScalaCheck with TestQueries {
}

trait TestQueries {
val testQueries = List(
"" -> Map.empty,
val testQueries: List[(String, Map[String, List[String]])] = List(
"q" -> Map("q" -> Nil),
"q=" -> Map("q" -> Nil),
"k:ey=a=a" -> Map("k:ey" -> List("a=a")),
"k:ey=a?a" -> Map("k:ey" -> List("a?a")),
"k=a&k=b&k=c" -> Map("k" -> List("a", "b", "c")),
"foo=bar&key=value" -> Map("foo" -> List("bar"), "key" -> List("value")),
"foo=bar&encoded=%2F-(4)+%26+a" -> Map("foo" -> List("bar"), "encoded" -> List("/-(4) & a"))
"foo=bar&encoded=%2F-(4)+%26+a" -> Map("foo" -> List("bar"), "encoded" -> List("/-(4) & a")),
"%3Cmy_tag_2f0e5c3f94df95106fbdd455f9abb89640/%3E=" -> Map("%3Cmy_tag_2f0e5c3f94df95106fbdd455f9abb89640/%3E" -> Nil)
)

}
40 changes: 20 additions & 20 deletions url/src/test/scala/uscala/net/URLSpec.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ import org.scalacheck.{Arbitrary, Gen}
import org.specs2.ScalaCheck
import org.specs2.mutable.Specification
import org.specs2.specification.core.Fragment

import URL._
import uscala.net.URL._

class URLSpec extends Specification with ScalaCheck with TestData {

Expand Down Expand Up @@ -69,40 +68,41 @@ class URLSpec extends Specification with ScalaCheck with TestData {
"path" >> {
"should decode the rawPath correctly" >> {
"when it has a space" >> {
URL("http://example.com/hello%20world/").map(_.path) must beASuccessfulTry(Some("/hello world/"))
URL("http://example.com/hello%20world/").map(_.path) must beASuccessfulTry(Option("/hello world/"))
}
"when it has an encoded `/` and `?` in it" >> {
URL("http://example.com/a%2Fb%3Fc").map(_.path) must beASuccessfulTry(Some("/a/b?c"))
URL("http://example.com/a%2Fb%3Fc").map(_.path) must beASuccessfulTry(Option("/a/b?c"))
}
}
}

def equivalentURI(u: StringUrl)(res: URI) = {
private def equivalentURI(u: StringUrl)(res: URI) = {
val expected = new URI(u)
res.getScheme must_== expected.getScheme aka "scheme"
res.getUserInfo must_== expected.getUserInfo
res.getHost must_== expected.getHost
res.getPort must_== expected.getPort
res.getRawPath must_== expected.getRawPath
res.getPath must_== expected.getPath
Option(res.getQuery).map(_.split('&').toSet) must_== Option(expected.getQuery).map(_.split('&').toSet)
queryToSet(res.getQuery) must_=== queryToSet(expected.getQuery)
res.getFragment must_== expected.getFragment
}

def equivalentJURL(u: StringUrl)(res: JURL) = {
def sameQuery(q1: Option[String], q2: Option[String]) =
q1.map(URL.decode).map(_.split('&').toSet) must_== q2.map(URL.decode).map(_.split('&').toSet)
private def equivalentJURL(u: StringUrl)(res: JURL) = {
val expected = new JURL(u)
res.getProtocol must_== expected.getProtocol
res.getUserInfo must_== expected.getUserInfo
res.getHost must_== expected.getHost
res.getPort must_== expected.getPort
res.getAuthority must_== expected.getAuthority
res.getPath must_== expected.getPath
sameQuery(Option(res.getQuery), Option(expected.getQuery))
res.getRef must_== expected.getRef
res.getProtocol must_=== expected.getProtocol
res.getUserInfo must_=== expected.getUserInfo
res.getHost must_=== expected.getHost
res.getPort must_=== expected.getPort
res.getAuthority must_=== expected.getAuthority
res.getPath must_=== expected.getPath
queryToSet(res.getQuery) must_=== queryToSet(expected.getQuery)
res.getRef must_=== expected.getRef
}

private def queryToSet(q: String) =
Option(q).map(URL.decode).map(_.split('&').toSet.filter(_.nonEmpty)).filter(_.nonEmpty)

"asString should provide an equivalent URI" >> prop { u: StringUrl =>
URL(u).map(u => new URI(u.asString)) must beASuccessfulTry.which(equivalentURI(u))
}
Expand Down Expand Up @@ -164,7 +164,7 @@ trait TestData {
//TODO include some examples from: https://www.talisman.org/~erlkonig/misc/lunatech%5ewhat-every-webdev-must-know-about-url-encoding/
val testUrls = List(
"http://host" -> URL(scheme = "http", host = "host", rawPath = Some("")),
"https://host?" -> URL(scheme = "https", host = "host", rawPath = Some(""), query = NonEmptyQuery(Map.empty)),
"https://host?" -> URL(scheme = "https", host = "host", rawPath = Some("")),
"ftp://example.com/" -> URL(scheme = "ftp", host = "example.com", rawPath = Some("/")),
"http://host/a%20path" -> URL(scheme = "http", host = "host", rawPath = Some("/a%20path")),
"http://user:password@example.com:2/p/a/t/h?foo=bar&key=value#foo"
Expand All @@ -176,11 +176,11 @@ trait TestData {
query = NonEmptyQuery(Map("foo" -> List("bar"), "encoded" -> List("/-(4) & a"))))
)

val testUris = testUrls.map { case (u, _) => new URI(u) }
val testUris: List[URI] = testUrls.map { case (u, _) => new URI(u) }

type StringUrl = String

def abStringUrlGen = Gen.oneOf(testUrls.map { case (u, _) => u })
def abStringUrlGen: Gen[StringUrl] = Gen.oneOf(testUrls.map { case (u, _) => u })

implicit def abStrings: Arbitrary[StringUrl] = Arbitrary(abStringUrlGen)

Expand Down

0 comments on commit 67cf0b4

Please sign in to comment.