[#324] Fixed bug in Router.queryString when handling empty strings fr…

…om QueryStringBindable
playframework · Sep 25, 2012 · 63a19ac · 63a19ac
1 parent c9b93ec
commit 63a19ac
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 1 deletion.
diff --git a/framework/src/play/src/main/scala/play/core/router/Router.scala b/framework/src/play/src/main/scala/play/core/router/Router.scala
@@ -134,7 +134,7 @@ object Router {
   }
 
   def queryString(items: List[Option[String]]) = {
-    Option(items.filter(_.isDefined).map(_.get)).filterNot(_.isEmpty).map("?" + _.mkString("&")).map(_.reverse.dropWhile(_ == '&').dropWhile(_ == '?').reverse).getOrElse("")
+    Option(items.filter(_.isDefined).map(_.get).filterNot(_.isEmpty)).filterNot(_.isEmpty).map("?" + _.mkString("&")).getOrElse("")
   }
 
   // HandlerInvoker

diff --git a/framework/src/play/src/test/scala/play/core/router/RouterSpec.scala b/framework/src/play/src/test/scala/play/core/router/RouterSpec.scala
@@ -0,0 +1,24 @@
+package play.core.router
+
+import org.specs2.mutable.Specification
+import play.core.Router
+
+object RouterSpec extends Specification {
+  "Router queryString builder" should {
+    "build a query string" in {
+      Router.queryString(List(Some("a"), Some("b"))) must_== "?a&b"
+    }
+    "ignore none values" in {
+      Router.queryString(List(Some("a"), None, Some("b"))) must_== "?a&b"
+      Router.queryString(List(None, Some("a"), None)) must_== "?a"
+    }
+    "ignore empty values" in {
+      Router.queryString(List(Some("a"), Some(""), Some("b"))) must_== "?a&b"
+      Router.queryString(List(Some(""), Some("a"), Some(""))) must_== "?a"
+    }
+    "produce nothing if no values" in {
+      Router.queryString(List(None, Some(""))) must_== ""
+      Router.queryString(List()) must_== ""
+    }
+  }
+}