Javalin: Context.formParam() does not decode the keys

Created on 5 Jun 2020  路  8Comments  路  Source: tipsy/javalin

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

BUG

All 8 comments

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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

JosefEvAlloc picture JosefEvAlloc  路  5Comments

mkpaz picture mkpaz  路  4Comments

tipsy picture tipsy  路  3Comments

TobiasWalle picture TobiasWalle  路  3Comments

wellingtoncosta picture wellingtoncosta  路  5Comments