Double URL Decoding of Query Parameters

[Traderjoe95]

Version

5.0.4

Context

When specifying query parameters that need to be URL-encoded, the extractQuery function of RequestUtils attempts URL decoding of the parameter value. This is incorrect, as parameters are already URL-decoded when retrieving them them from the HttpServerRequest (in vertx-core/io.vertx.core.http.impl.HttpUtils::params).

This behavior is likely to go undetected, as URL decoding will only lead to unexpected results if the decoded parameter value include + characters or fail if the parameter value contains %. We ran into the latter case.

Steps to reproduce

1. Define an endpoint with a query parameter
2. Send a request where the parameter value includes the % character (encoded %25)
3. Observe

io.vertx.openapi.validation.ValidatorException: Can't decode URL value: ((name!=Invalid) AND (name!=Error) and (name like 'thi%') and ( name like '_hin_' ) ) or name=thing
	at io.vertx.openapi.validation.RequestUtils.decodeUrl(RequestUtils.java:175) ~[vertx-openapi-5.0.4.jar:5.0.4]
	at io.vertx.openapi.validation.RequestUtils.lambda$joinFormValues$6(RequestUtils.java:155) ~[vertx-openapi-5.0.4.jar:5.0.4]
	[...]
	at io.vertx.openapi.validation.RequestUtils.joinFormValues(RequestUtils.java:155) ~[vertx-openapi-5.0.4.jar:5.0.4]
	at io.vertx.openapi.validation.RequestUtils.extractQuery(RequestUtils.java:131) ~[vertx-openapi-5.0.4.jar:5.0.4]
	at io.vertx.openapi.validation.RequestUtils.extract(RequestUtils.java:87) ~[vertx-openapi-5.0.4.jar:5.0.4]
	at io.vertx.openapi.validation.RequestUtils.extract(RequestUtils.java:55) ~[vertx-openapi-5.0.4.jar:5.0.4]
	at io.vertx.ext.web.openapi.router.RouterBuilder.lambda$create$0(RouterBuilder.java:61) ~[vertx-web-openapi-router-5.0.4.jar:5.0.4]
	at io.vertx.ext.web.openapi.router.impl.RouterBuilderImpl.lambda$createRouter$3(RouterBuilderImpl.java:131) ~[vertx-web-openapi-router-5.0.4.jar:5.0.4]
	[...]
Caused by: java.lang.IllegalArgumentException: URLDecoder: Illegal hex characters in escape (%) pattern - Error at index 0 in: "')"
	at java.base/java.net.URLDecoder.decode(URLDecoder.java:243) ~[?:?]
	at io.vertx.openapi.validation.RequestUtils.decodeUrl(RequestUtils.java:173) ~[vertx-openapi-5.0.4.jar:5.0.4]
	... 68 more

Do you have a reproducer?

No response

[Traderjoe95]

Ah, I should have looked into the latest snapshot first. Thanks for the fix @pk-work

[Traderjoe95]

@pk-work is there any specific reason why these fixes did not make it into the latest releases?

[pk-work]

The bug fix is not part of the latest release, as the latest release only increased the patch version. And even if the current behavior is flawed, this change is still a significant alteration in functionality. I would not expect such a change in behavior with an increase in the patch version.

Therefore, the fix is part of 5.1, the next minor release.

I hope this make sense.

[Traderjoe95]

Hm, I would argue that this is more of a bugfix. I wouldn't consider correcting the behavior a breaking change, especially when the fixed behavior works the same in most situations and changes from "broken" to "working as expected" in all other cases. I'm referring specifically to the query parameter handling here, for the other cases I think I can follow your argument.

Anyway, I hope that 5.1 will not take too long, as this is another show stopper in our Vert.x 5 migration. I guess I'll keep this issue open until 5.1 is released, or would you suggest otherwise?

[pk-work]

For me, it's fine if the ticket remains open. I mean, it's already merged and will definitely be part of the 5.1 release, so I don't see much benefit in it, but it doesn't hurt either. :)