jester
jester copied to clipboard
Request.params do not allow duplicate parameters
https://github.com/dom96/jester/blob/d8a03aa4c681bc8514bb7bbf4953d380d86f5bd6/jester/request.nim#L95
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.
What are people doing for this now?
would be nice to fix this
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 https://github.com/dom96/jester/blob/053d62c7f02ac49e3952da1acf85d50c35d5c7c7/jester/request.nim#L112-L117 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
.
@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.
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.
[..]
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.