Request.params do not allow duplicate parameters

[mildred]

jester/jester/request.nim

Line 95 in d8a03aa

result[key] = val

Sometimes, in query strings, you need to pass lists. This is generally done by duplicating the same key over like: name=a&name=b. juster folds these into a single parameter in the above code. Instead of using []= to add new parameters, add allows key duplicates.

[sdmcallister]

What are people doing for this now?

[dom96]

would be nice to fix this

[torbencarstens]

Hey @dom96, I'd be interested to implement this since I want to use it myself (to use <select multiple> with GET).

Before creating a PR I just wanted to get some input from you regarding the implementation.

tables.add

I'd avoid this since it has been deprecated since nim 1.4.

joining values on a character

Since 0.6.0 added the auto decoding of query values simply joining the different values on some reserved character (e.g. #) is not really viable anymore.

-> This would fail as soon as the user has a percent-endoding for the "join character" in their query value (e.g. a=1&a=b%23c would be returned in the following way 1#b#c).

changing the return type

This would completely break all existing programs relying on params.

custom ParamValue type

Changing the return type to Table[str, ParamValue] where ParamValue would be something like (note that this is basically a copy of the JsonNode implementation)

ParamValue implementation

type
  ParamValueKind* = enum
    ParamString
    ParamList

  ParamValueObj* = object
    case kind*: ParamValueKind
    of ParamString:
      str*: string
    of ParamList:
      list*: seq[string]

  ParamValue* = ref ParamValueObj

which could be constructed like this

let stringParam = ParamValue(kind: ParamString, str: "2")
let listParam = ParamValue(kind: ParamList, list: @["1", "2"])

Additionally some helper functions to retrieve a specific type could be added as well (e.g. getListParam).

Table[str, seq[str]]

Another option would be to simply use Table[str, seq[str]], since query parameters occuring multiple times is the odd case this wouldn't be a nice default.

Additional function

Adding a new function (e.g. paramsListValues) might be the option with the least user impact.

Implemented by either option in changing the return type.

Downside of this option is that it would basically mean a 1:1 implementation of params with the exception of the result type (and it's creation).

[...]

---

One general thing about the params function, is there any special reason for this

jester/jester/request.nim

Lines 112 to 117 in 053d62c

	let contentType = req.headers.getOrDefault("Content-Type")
	if contentType.startswith("application/x-www-form-urlencoded"):
	try:
	parseUrlQuery(req.body, result)
	except:
	logging.warn("Could not parse URL query.")

part?

parseUrlQuery is deprecated and points towards cgi/decodeData which you're already using in params.

The main difference seems to be that key is also url decoded in parseUrlQuery.

[ThomasTJdev]

@torbencarstens,
How about a helper proc which parses req.url? That would require the user to call it manually to avoid the overhead of parsing twice.

[torbencarstens]

Would you mind elaborating a bit on this, I guess you agree with implementing the Additional function part of my comment above?

This would implicitely avoid parsing twice unless the user calls both params and {newFunction} which would be ... interesting.

I wouldn't use the req.url though since the query proc is already there and the return value can be used for the cgi.decodeData proc argument. (although we could use cgi.decodeQuery directly as well since decodeData(string) is just decodeQuery in disguise).

Users calling both query and params/{newFunction} is probably rather rare as well (and rather inexpensive at that if it happens).

This solution would basically look like this (I chose Table[string, seq[string]] here for breviety since the ParamValue implementation would be a bit longer)

proc paramsValuesAsList*(req: Request): Table[string, seq[string]] =
  ## Parameters from the pattern and the query string.
  ##
  ## In contrast to `params` this supports query keys occuring multiple times (e.g. `?a=1&a=2`).
  # deliberately ignoring `patternParams` here for brevity since it needs a mapping from `string` -> `seq[string]`
  result = initTable[string, seq[string]]()

  let query = query(req)

  try:
    for key, val in cgi.decodeQuery(query):
      let decodedValue = decodeUrl(val)

      # this assumes that the key url decoding is irrelevant (only implemented for `x-www-form-urlencoded` queries as of now)
      if result.hasKey(key):
        result[key].add decodedValue
      else:
        result[key] = @[decodedValue]
  except CgiError:
    logging.warn("Incorrect query. Got: $1" % [query])

  result

(this assumes that the parseUrlQuery function is currently unnecessary)

---

As a side note regarding the What are people doing for this now?, if you want to work around this on the "client" side you can basically take the function from above, put it in your own program and pass the request variable to it.

[ThomasTJdev]

[..]

My suggestion was for a way to do it without changing jester-code, but just hooking in on available fields = the .url

For your suggested solutions, I'm in favor of the Additional function. Otherwise you should change it globally to the Table[string, seq[string]] and with the param template @ just return the first value as a string, so it resembles the current approach.