Query param fixes (#128)

* Represent emtpy query parameters as empty strings instead of "true"

* Encode and decode query params

Fixes #126

* Pass invalid query parameters through instead of ignoring them

* Decode the plus sign `+` as a space ` ` in query string

* Decode percent encoding in path segments

* Update History.md

History.md

@@ -4,6 +4,8 @@ unreleased
 - Support binary response body (thanks **@akheron**)
 - Add support for chunked responses
 - `ServerM` now contains a callback that when called will shut down the server
+- Map empty query parameters to empty strings instead of `"true"`
+- Decode percent encoding in path segments and query parameters automatically
 
 0.7.0 / 2018-07-08
 ==================

src/HTTPure/Path.purs

@@ -10,6 +10,9 @@ import Data.Maybe as Maybe
 import Data.String as String
 import Node.HTTP as HTTP
 
+import HTTPure.Utils as Utils
+
+
 -- | The `Path` type is just sugar for an `Array` of `String` segments that are
 -- | sent in a request and indicates the path of the resource being requested.
 -- | Note that this type has an implementation of `Lookup` for `Int` keys
@@ -20,7 +23,8 @@ type Path = Array String
 
 -- | Given an HTTP `Request` object, extract the `Path`.
 read :: HTTP.Request -> Path
-read = HTTP.requestURL >>> split "?" >>> first >>> split "/" >>> nonempty
+read =
+  HTTP.requestURL >>> split "?" >>> first >>> split "/" >>> nonempty >>> map Utils.urlDecode
   where
     nonempty = Array.filter ((/=) "")
     split = String.Pattern >>> String.split

src/HTTPure/Query.purs

@@ -6,20 +6,23 @@ module HTTPure.Query
 import Prelude
 
 import Data.Array as Array
+import Data.Bifunctor as Bifunctor
 import Data.Maybe as Maybe
 import Data.String as String
 import Data.Tuple as Tuple
 import Foreign.Object as Object
 import Node.HTTP as HTTP
 
+import HTTPure.Utils as Utils
+
 -- | The `Query` type is a `Object` of `Strings`, with one entry per query
 -- | parameter in the request. For any query parameters that don't have values
--- | (`/some/path?query`), the value in the `Object` for that parameter will be
+-- | (`/some/path?query` or `/some/path?query=`), the value in the `Object` for
--- | the string `"true"`. Note that this type has an implementation of `Lookup`
+-- | that parameter will be the an empty string. Note that this type has an
--- | for `String` keys defined by `lookupObject` in [Lookup.purs](./Lookup.purs)
+-- | implementation of `Lookup` for `String` keys defined by `lookupObject` in
--- | because `lookupObject` is defined for any `Object` of `Monoids`. So you can
+-- | [Lookup.purs](./Lookup.purs) because `lookupObject` is defined for any
--- | do something like `query !! "foo"` to get the value of the query parameter
+-- | `Object` of `Monoids`. So you can do something like `query !! "foo"` to get
--- | "foo".
+-- | the value of the query parameter "foo".
 type Query = Object.Object String
 
 -- | The `Map` of query segments in the given HTTP `Request`.
@@ -32,7 +35,8 @@ read =
     split = String.Pattern >>> String.split
     first = Array.head >>> Maybe.fromMaybe ""
     last = Array.tail >>> Maybe.fromMaybe [] >>> String.joinWith ""
-    toTuple item = Tuple.Tuple (first itemParts) $ value $ last itemParts
+    decode = Utils.replacePlus >>> Utils.urlDecode
+    decodeKeyValue = Bifunctor.bimap decode decode
+    toTuple item = decodeKeyValue $ Tuple.Tuple (first itemParts) (last itemParts)
       where
-        value val = if val == "" then "true" else val
         itemParts = split "=" item

src/HTTPure/Request.purs

@@ -16,6 +16,7 @@ import HTTPure.Headers as Headers
 import HTTPure.Method as Method
 import HTTPure.Path as Path
 import HTTPure.Query as Query
+import HTTPure.Utils (encodeURIComponent)
 
 -- | The `Request` type is a `Record` type that includes fields for accessing
 -- | the different parts of the HTTP request.
@@ -37,7 +38,7 @@ fullPath request = "/" <> path <> questionMark <> queryParams
     questionMark = if Object.isEmpty request.query then "" else "?"
     queryParams = String.joinWith "&" queryParamsArr
     queryParamsArr = Object.toArrayWithKey stringifyQueryParam request.query
-    stringifyQueryParam key value = key <> "=" <> value
+    stringifyQueryParam key value = encodeURIComponent key <> "=" <> encodeURIComponent value
 
 -- | Given an HTTP `Request` object, this method will convert it to an HTTPure
 -- | `Request` object.

src/HTTPure/Utils.js

@@ -0,0 +1,11 @@
+"use strict";
+
+exports.encodeURIComponent = encodeURIComponent
+
+exports.decodeURIComponentImpl = function(s) {
+  try {
+    return decodeURIComponent(s);
+  } catch(error) {
+    return null;
+  }
+};

src/HTTPure/Utils.purs

@@ -0,0 +1,30 @@
+module HTTPure.Utils
+  ( encodeURIComponent
+  , decodeURIComponent
+  , replacePlus
+  , urlDecode
+  ) where
+
+import Prelude
+
+import Data.Maybe as Maybe
+import Data.Nullable as Nullable
+import Data.String as String
+
+
+foreign import encodeURIComponent :: String -> String
+
+foreign import decodeURIComponentImpl :: String -> Nullable.Nullable String
+
+decodeURIComponent :: String -> Maybe.Maybe String
+decodeURIComponent = Nullable.toMaybe <<< decodeURIComponentImpl
+
+
+replacePlus :: String -> String
+replacePlus =
+  String.replace (String.Pattern "+") (String.Replacement "%20")
+
+
+urlDecode :: String -> String
+urlDecode s =
+  Maybe.fromMaybe s $ decodeURIComponent s

test/Test/HTTPure/PathSpec.purs

@@ -27,6 +27,13 @@ readSpec = Spec.describe "read" do
     Spec.it "strips the empty segments" do
       request <- TestHelpers.mockRequest "GET" "//test//path///?query" "" []
       Path.read request ?= [ "test", "path" ]
+  Spec.describe "with percent encoded segments" do
+    Spec.it "decodes percent encoding" do
+      request <- TestHelpers.mockRequest "GET" "/test%20path/%2Fthis" "" []
+      Path.read request ?= [ "test path", "/this" ]
+    Spec.it "does not decode a plus sign" do
+      request <- TestHelpers.mockRequest "GET" "/test+path/this" "" []
+      Path.read request ?= [ "test+path", "this" ]
 
 pathSpec :: TestHelpers.Test
 pathSpec = Spec.describe "Path" do

test/Test/HTTPure/QuerySpec.purs

@@ -34,20 +34,30 @@ readSpec = Spec.describe "read" do
       req <- TestHelpers.mockRequest "" "/test?a=b&a=c" "" []
       Query.read req ?= Object.singleton "a" "c"
   Spec.describe "with empty params" do
-    Spec.it "uses 'true' as the value" do
+    Spec.it "uses '' as the value" do
       req <- TestHelpers.mockRequest "" "/test?a" "" []
-      Query.read req ?= Object.singleton "a" "true"
+      Query.read req ?= Object.singleton "a" ""
   Spec.describe "with complex params" do
     Spec.it "is the correct Map" do
       req <- TestHelpers.mockRequest "" "/test?&&a&b=c&b=d&&&e=f&g=&" "" []
       Query.read req ?= expectedComplexResult
+  Spec.describe "with urlencoded params" do
+    Spec.it "decodes valid keys and values" do
+      req <- TestHelpers.mockRequest "" "/test?foo%20bar=%3Fx%3Dtest" "" []
+      Query.read req ?= Object.singleton "foo bar" "?x=test"
+    Spec.it "passes invalid keys and values through" do
+      req <- TestHelpers.mockRequest "" "/test?%%=%C3" "" []
+      Query.read req ?= Object.singleton "%%" "%C3"
+    Spec.it "converts + to a space" do
+      req <- TestHelpers.mockRequest "" "/test?foo=bar+baz" "" []
+      Query.read req ?= Object.singleton "foo" "bar baz"
   where
       expectedComplexResult =
         Object.fromFoldable
-          [ Tuple.Tuple "a" "true"
+          [ Tuple.Tuple "a" ""
           , Tuple.Tuple "b" "d"
           , Tuple.Tuple "e" "f"
-          , Tuple.Tuple "g" "true"
+          , Tuple.Tuple "g" ""
           ]
 
 querySpec :: TestHelpers.Test

test/Test/HTTPure/RequestSpec.purs

@@ -51,9 +51,13 @@ fullPathSpec = Spec.describe "fullPath" do
       mock <- mockRequest "?a=b&c=d"
       Request.fullPath mock ?= "/?a=b&c=d"
   Spec.describe "with only empty query parameters" do
-    Spec.it "is has the default value of 'true' for the empty parameters" do
+    Spec.it "is has the default value of '' for the empty parameters" do
       mock <- mockRequest "?a"
-      Request.fullPath mock ?= "/?a=true"
+      Request.fullPath mock ?= "/?a="
+  Spec.describe "with query parameters that have special characters" do
+    Spec.it "percent encodes query params" do
+      mock <- mockRequest "?a=%3Fx%3Dtest"
+      Request.fullPath mock ?= "/?a=%3Fx%3Dtest"
   Spec.describe "with empty query parameters" do
     Spec.it "strips out the empty arameters" do
       mock <- mockRequest "?a=b&&&"
@@ -61,7 +65,7 @@ fullPathSpec = Spec.describe "fullPath" do
   Spec.describe "with a mix of segments and query parameters" do
     Spec.it "is correct" do
       mock <- mockRequest "/foo///bar/?&a=b&&c"
-      Request.fullPath mock ?= "/foo/bar?a=b&c=true"
+      Request.fullPath mock ?= "/foo/bar?a=b&c="
   where
     mockHTTPRequest path = TestHelpers.mockRequest "POST" path "body" []
     mockRequest path = mockHTTPRequest path >>= Request.fromHTTPRequest