Add support for duplicate keys in parameters by adding paramValuesAsSeq

.github/workflows/main.yml

@@ -13,21 +13,22 @@ jobs:
       matrix:
         nimversion:
         - 1.4.8
-        - git:6b97889f44d06f66
+        - 1.6.14
         os:
         - ubuntu-latest
         - macOS-latest
         - windows-latest
     steps:
-    - uses: actions/checkout@v1
+    - uses: actions/checkout@v4
       with:
         submodules: true
-    - uses: iffy/install-nim@v3.2.0
+    - uses: iffy/install-nim@v4
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       with:
-        nimversion: ${{ matrix.nimversion }}
+        version: ${{ matrix.nimversion }}
     - name: Test
       run: |
+        nimble --version
+        nimble refresh -y
         nimble test
-        nimble refresh

jester.nimble

@@ -16,5 +16,9 @@ when not defined(windows):
   requires "httpbeast >= 0.4.0"
 
 task test, "Runs the test suite.":
+  when NimMajor < 2:
+    exec "git submodule update --init"
+  exec "nimble refresh -y"
+  exec "nimble install -y"
   exec "nimble install -y asynctools@#0e6bdc3ed5bae8c7cc9"
   exec "nim c -r tests/tester"

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

@@ -154,7 +154,7 @@ proc allTest(useStdLib: bool) =
     let resp = waitFor client.get(address & "/foo/issue157")
     let headers = resp.headers
     check headers["Content-Type"] == "text/css"
-  
+
   test "resp doesn't overwrite headers":
     let resp = waitFor client.get(address & "/foo/manyheaders")
     let headers = resp.headers
@@ -271,13 +271,108 @@ proc customRouterTest(useStdLib: bool) =
       let body = (waitFor resp.body)
       checkpoint body
       check body.startsWith("Something bad happened: Foobar")
-    
+
     test "redirect in error":
       let resp = waitFor client.get(address & "/definitely404route")
       check resp.code == Http303
       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,10 +381,14 @@ when isMainModule:
     issue150(useStdLib=true)
     customRouterTest(useStdLib=false)
     customRouterTest(useStdLib=true)
-
-    # Verify that Nim in Action Tweeter still compiles.
-    test "Nim in Action - Tweeter":
-      let path = "tests/nim-in-action-code/Chapter7/Tweeter/src/tweeter.nim"
-      check execCmd("nim c --path:. " & path) == QuitSuccess
+    issue247(useStdLib=false)
+    issue247(useStdLib=true)
+
+    # nim-in-action Chapter7 is not compatible with nim >= 2.0
+    when NimMajor < 2:
+      # Verify that Nim in Action Tweeter still compiles.
+      test "Nim in Action - Tweeter":
+        let path = "tests/nim-in-action-code/Chapter7/Tweeter/src/tweeter.nim"
+        check execCmd("nim c --path:. " & path) == QuitSuccess
   finally:
     doAssert execCmd("kill -15 " & $serverProcess.processID()) == QuitSuccess