Actual behavior (the bug)
When you submit a form using POST method, the input names are encoded by the browser. For example,
<input name="user[type]" value="admin">
will be submitted in POST body as:
user%5Btype%5D=admin
I would expect it to be decoded by javalin, so that I can get it by calling:
Context.formParam("user[type]")
But this doesn't work. The formParams map keeps the keys in the encoded form, so i have to use it this way:
Context.formParam("user%5Btype%5D")
This is really awkward, and definitely looks like a bug. Was it done for purpose this way? Or am i missing some configuration settings?
Expected behavior
Context.formParam("user[type]") should return the value "admin", not null
To Reproduce
Just create a form with method=POST and a input named "user[type]" or similar, and submit it, and try accessing the param value via Context.formParam
Additional context
Add any other context about the bug here
We do decode the form-param values, but not the keys. I don't know what the spec says about this, but it seems like something we should allow. If you want to make a PR, you can find the code in ContextUtil#splitKeyValueStringAndGroupByKey.
@grzegorzbor I'm planning a release tomorrow, do you want this included?
Ough. What a pity i didn't see your message, i would have made a fix for the release!
I verified that specification clearly says both values and keys should be percent-encoded and decoded. I can make a PR, but there is no rush now :(
I'd say this is a bug, warranting a patch release. So if you make a PR @tipsy is quick to release such things ^^
@pkkummermo is correct. There was another bug with the OpenAPI plugin, so I will be releasing a patch soon.
Ok i will take a look then and provide the PR
Fixing it was trivial, PR is ready.
But this is a backward-compatibility breaking change. Someone can already have a code where he passes encoded strings as keys for body params. This change is going to break it - the code will return null now, because such keys will not longer be matched. I don't think a patch release should introduce such breaking change. What do you think?
I would argue that it's fine in this case, this is an actual bug that's being fixed.