Allow duplicate query keys

jester/request.nim

@@ -1,4 +1,5 @@
 import uri, cgi, tables, logging, strutils, re, options
+from sequtils import map
 
 import jester/private/utils
 
@@ -93,28 +94,62 @@ proc ip*(req: Request): string =
 
 proc params*(req: Request): Table[string, string] =
   ## Parameters from the pattern and the query string.
+  ##
+  ## Note that this doesn't allow for duplicated keys (it simply returns the last occuring value)
+  ## Use `paramValuesAsSeq` if you need multiple values for a key
   if req.patternParams.isSome():
     result = req.patternParams.get()
   else:
     result = initTable[string, string]()
 
-  when useHttpBeast:
-    let query = req.req.path.get("").parseUri().query
+  var queriesToDecode: seq[string] = @[]
+  queriesToDecode.add query(req)
+
+  let contentType = req.headers.getOrDefault("Content-Type")
+  if contentType.startswith("application/x-www-form-urlencoded"):
+    queriesToDecode.add req.body
+
+  for query in queriesToDecode:
+    try:
+      for key, val in cgi.decodeData(query):
+        result[key] = decodeUrl(val)
+    except CgiError:
+      logging.warn("Incorrect query. Got: $1" % [query])
+
+proc paramValuesAsSeq*(req: Request): Table[string, seq[string]] =
+  ## Parameters from the pattern and the query string.
+  ##
+  ## This allows for duplicated keys in the query (in contrast to `params`)
+  if req.patternParams.isSome():
+    let patternParams: Table[string, string] = req.patternParams.get()
+    var patternParamsSeq: seq[(string, string)] = @[]
+    for key, val in pairs(patternParams):
+      patternParamsSeq.add (key, val)
+
+    # We are not url-decoding the key/value for the patternParams (matches implementation in `params`
+    result = sequtils.map(patternParamsSeq,
+              proc(entry: (string, string)): (string, seq[string]) =
+                (entry[0], @[entry[1]])
+    ).toTable()
   else:
-    let query = req.req.url.query
+    result = initTable[string, seq[string]]()
 
-  try:
-    for key, val in cgi.decodeData(query):
-      result[key] = decodeUrl(val)
-  except CgiError:
-    logging.warn("Incorrect query. Got: $1" % [query])
+  var queriesToDecode: seq[string] = @[]
+  queriesToDecode.add query(req)
 
   let contentType = req.headers.getOrDefault("Content-Type")
   if contentType.startswith("application/x-www-form-urlencoded"):
+    queriesToDecode.add req.body
+
+  for query in queriesToDecode:
     try:
-      parseUrlQuery(req.body, result)
-    except:
-      logging.warn("Could not parse URL query.")
+      for key, value in cgi.decodeData(query):
+        if result.hasKey(key):
+          result[key].add value
+        else:
+          result[key] = @[value]
+    except CgiError:
+      logging.warn("Incorrect query. Got: $1" % [query])
 
 proc formData*(req: Request): MultiData =
   let contentType = req.headers.getOrDefault("Content-Type")

tests/issue247.nim

@@ -0,0 +1,41 @@
+from std/cgi import decodeUrl
+from std/strformat import fmt
+from std/strutils import join
+import jester
+
+settings:
+  port = Port(5454)
+  bindAddr = "127.0.0.1"
+
+proc formatParams(params: Table[string, string]): string =
+  result = ""
+  for key, value in params.pairs:
+    result.add fmt"{key}: {value}"
+
+proc formatSeqParams(params: Table[string, seq[string]]): string =
+  result = ""
+  for key, values in params.pairs:
+    let value = values.join ","
+    result.add fmt"{key}: {value}"
+
+routes:
+  get "/":
+    resp Http200
+  get "/params":
+    let params = params request
+    resp formatParams params
+  get "/params/@val%23ue":
+    let params = params request
+    resp formatParams params
+  post "/params/@val%23ue":
+    let params = params request
+    resp formatParams params
+  get "/multi":
+    let params = paramValuesAsSeq request
+    resp formatSeqParams(params)
+  get "/@val%23ue":
+    let params = paramValuesAsSeq request
+    resp formatSeqParams(params)
+  post "/@val%23ue":
+    let params = paramValuesAsSeq request
+    resp formatSeqParams(params)

tests/tester.nim

@@ -278,6 +278,101 @@ proc customRouterTest(useStdLib: bool) =
       check resp.headers["location"] == address & "/404"
       check (waitFor resp.body) == ""
 
+proc issue247(useStdLib: bool) =
+  waitFor startServer("issue247.nim", useStdLib)
+  var client = newAsyncHttpClient(maxRedirects = 0)
+
+  suite "issue247 useStdLib=" & $useStdLib:
+    test "duplicate keys in query":
+      let resp = waitFor client.get(address & "/multi?a=1&a=2")
+      check (waitFor resp.body) == "a: 1,2"
+
+    test "no duplicate keys in query":
+      let resp = waitFor client.get(address & "/multi?a=1")
+      check (waitFor resp.body) == "a: 1"
+
+    test "assure that empty values are handled":
+      let resp = waitFor client.get(address & "/multi?a=1&a=")
+      check (waitFor resp.body) == "a: 1,"
+
+    test "assure that fragment is not parsed":
+      let resp = waitFor client.get(address & "/multi?a=1&#a=2")
+      check (waitFor resp.body) == "a: 1"
+
+    test "ensure that values are url decoded per default":
+      let resp = waitFor client.get(address & "/multi?a=1&a=1%232")
+      check (waitFor resp.body) == "a: 1,1#2"
+
+    test "ensure that keys are url decoded per default":
+      let resp = waitFor client.get(address & "/multi?a%23b=1&a%23b=1%232")
+      check (waitFor resp.body) == "a#b: 1,1#2"
+
+    test "test different keys":
+      let resp = waitFor client.get(address & "/multi?a=1&b=2")
+      check (waitFor resp.body) == "b: 2a: 1"
+
+    test "ensure that path params aren't escaped":
+      let resp = waitFor client.get(address & "/hello%23world")
+      check (waitFor resp.body) == "val%23ue: hello%23world"
+
+    test "test path params and query":
+      let resp = waitFor client.get(address & "/hello%23world?a%23+b=1%23+b")
+      check (waitFor resp.body) == "a# b: 1# bval%23ue: hello%23world"
+
+    test "test percent encoded path param and query param (same key)":
+      let resp = waitFor client.get(address & "/hello%23world?val%23ue=1%23+b")
+      check (waitFor resp.body) == "val%23ue: hello%23worldval#ue: 1# b"
+
+    test "test path param, query param and x-www-form-urlencoded":
+      client.headers = newHttpHeaders({"Content-Type": "application/x-www-form-urlencoded"})
+      let resp = waitFor client.post(address & "/hello%23world?val%23ue=1%23+b", "val%23ue=1%23+b&b=2")
+      check (waitFor resp.body) == "val%23ue: hello%23worldb: 2val#ue: 1# b,1# b"
+
+    test "params duplicate keys in query":
+      let resp = waitFor client.get(address & "/params?a=1&a=2")
+      check (waitFor resp.body) == "a: 2"
+
+    test "params no duplicate keys in query":
+      let resp = waitFor client.get(address & "/params?a=1")
+      check (waitFor resp.body) == "a: 1"
+
+    test "params assure that empty values are handled":
+      let resp = waitFor client.get(address & "/params?a=1&a=")
+      check (waitFor resp.body) == "a: "
+
+    test "params assure that fragment is not parsed":
+      let resp = waitFor client.get(address & "/params?a=1&#a=2")
+      check (waitFor resp.body) == "a: 1"
+
+    test "params ensure that values are url decoded per default":
+      let resp = waitFor client.get(address & "/params?a=1&a=1%232")
+      check (waitFor resp.body) == "a: 1#2"
+
+    test "params ensure that keys are url decoded per default":
+      let resp = waitFor client.get(address & "/params?a%23b=1&a%23b=1%232")
+      check (waitFor resp.body) == "a#b: 1#2"
+
+    test "params test different keys":
+      let resp = waitFor client.get(address & "/params?a=1&b=2")
+      check (waitFor resp.body) == "b: 2a: 1"
+
+    test "params ensure that path params aren't escaped":
+      let resp = waitFor client.get(address & "/params/hello%23world")
+      check (waitFor resp.body) == "val%23ue: hello%23world"
+
+    test "params test path params and query":
+      let resp = waitFor client.get(address & "/params/hello%23world?a%23+b=1%23+b")
+      check (waitFor resp.body) == "a# b: 1# bval%23ue: hello%23world"
+
+    test "params test percent encoded path param and query param (same key)":
+      let resp = waitFor client.get(address & "/params/hello%23world?val%23ue=1%23+b")
+      check (waitFor resp.body) == "val#ue: 1# bval%23ue: hello%23world"
+
+    test "params test path param, query param and x-www-form-urlencoded":
+      client.headers = newHttpHeaders({"Content-Type": "application/x-www-form-urlencoded"})
+      let resp = waitFor client.post(address & "/params/hello%23world?val%23ue=1%23+b", "val%23ue=1%23+b&b=2")
+      check (waitFor resp.body) == "b: 2val#ue: 1# bval%23ue: hello%23world"
+
 when isMainModule:
   try:
     allTest(useStdLib=false) # Test HttpBeast.
@@ -286,6 +381,8 @@ when isMainModule:
     issue150(useStdLib=true)
     customRouterTest(useStdLib=false)
     customRouterTest(useStdLib=true)
+    issue247(useStdLib=false)
+    issue247(useStdLib=true)
 
     # Verify that Nim in Action Tweeter still compiles.
     test "Nim in Action - Tweeter":