From 98b0b4e268696667e4f812d4c135693a71f591f5 Mon Sep 17 00:00:00 2001 From: Petri Lehtinen Date: Tue, 6 Nov 2018 19:45:37 +0200 Subject: [PATCH] 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 | 2 ++ src/HTTPure/Path.purs | 6 +++++- src/HTTPure/Query.purs | 20 ++++++++++++-------- src/HTTPure/Request.purs | 3 ++- src/HTTPure/Utils.js | 11 +++++++++++ src/HTTPure/Utils.purs | 30 ++++++++++++++++++++++++++++++ test/Test/HTTPure/PathSpec.purs | 7 +++++++ test/Test/HTTPure/QuerySpec.purs | 18 ++++++++++++++---- test/Test/HTTPure/RequestSpec.purs | 10 +++++++--- 9 files changed, 90 insertions(+), 17 deletions(-) create mode 100644 src/HTTPure/Utils.js create mode 100644 src/HTTPure/Utils.purs diff --git a/History.md b/History.md index 8751b99..feca812 100644 --- a/History.md +++ b/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 ================== diff --git a/src/HTTPure/Path.purs b/src/HTTPure/Path.purs index 338b482..ceaea31 100644 --- a/src/HTTPure/Path.purs +++ b/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 diff --git a/src/HTTPure/Query.purs b/src/HTTPure/Query.purs index 80c62ca..cf16eb7 100644 --- a/src/HTTPure/Query.purs +++ b/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 --- | the string `"true"`. Note that this type has an implementation of `Lookup` --- | for `String` keys defined by `lookupObject` in [Lookup.purs](./Lookup.purs) --- | because `lookupObject` is defined for any `Object` of `Monoids`. So you can --- | do something like `query !! "foo"` to get the value of the query parameter --- | "foo". +-- | (`/some/path?query` or `/some/path?query=`), the value in the `Object` for +-- | that parameter will be the an empty string. Note that this type has an +-- | implementation of `Lookup` for `String` keys defined by `lookupObject` in +-- | [Lookup.purs](./Lookup.purs) because `lookupObject` is defined for any +-- | `Object` of `Monoids`. So you can do something like `query !! "foo"` to get +-- | 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 diff --git a/src/HTTPure/Request.purs b/src/HTTPure/Request.purs index 61c19b2..b3c008e 100644 --- a/src/HTTPure/Request.purs +++ b/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. diff --git a/src/HTTPure/Utils.js b/src/HTTPure/Utils.js new file mode 100644 index 0000000..cf4769d --- /dev/null +++ b/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; + } +}; diff --git a/src/HTTPure/Utils.purs b/src/HTTPure/Utils.purs new file mode 100644 index 0000000..5485d7e --- /dev/null +++ b/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 diff --git a/test/Test/HTTPure/PathSpec.purs b/test/Test/HTTPure/PathSpec.purs index 7e92931..77b3a99 100644 --- a/test/Test/HTTPure/PathSpec.purs +++ b/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 diff --git a/test/Test/HTTPure/QuerySpec.purs b/test/Test/HTTPure/QuerySpec.purs index 830f71a..197f18a 100644 --- a/test/Test/HTTPure/QuerySpec.purs +++ b/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 diff --git a/test/Test/HTTPure/RequestSpec.purs b/test/Test/HTTPure/RequestSpec.purs index 9d19d8f..6d2b4a0 100644 --- a/test/Test/HTTPure/RequestSpec.purs +++ b/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