UriComponentsBuilder inconsistent encode/decode query params behavior [SPR-16002]

[spring-projects-issues]

Vsevolod Kalinin opened SPR-16002 and commented

After working with that for quite some time I have a strong feeling that something is wrong with UriComponentsBuilder query params encoding/decoding. Will try to explain it step by step now + my vision to the problem (still with hope of doing something wrong/missing something :)). Will use fromHttpUrl(..) in examples, but the behavior is the same for all from*(..) methods of all *UriComponentsBuilder classes (and it hurt even more when using something like fromHttpRequest(..)).

Use cases

So, consider I want to take current request URL, modify it (add/remove query params, etc.) and eventually use it somewhere else. Again there are fromHttpUrl(..) calls below for simplicity; IRL they are fromHttpRequest(..), etc.

1. h3. Double-encoded query params from original URL

UriComponentsBuilder ucb = UriComponentsBuilder.fromHttpUrl("http://host.com?name=Tom%26Jerry");
ucb.queryParam("album", "S&M");
ucb.toUriString(); // http://host.com?name=Tom%2526Jerry&album=S%26M

It returns http://host.com?name=Tom%2526Jerry&album=S%26M which is wrong (see double-encoded initial name param).
I was only able to solve the above with something like:

UriComponentsBuilder ucb = UriComponentsBuilder.fromHttpUrl("http://host.com?name=Tom%26Jerry");
ucb.queryParam("album", java.net.URLEncoder.encode("S&M"));
ucb.build(true).toUriString();

Now it returns the correct http://host.com?name=Tom%26Jerry&album=S%26M, but the whole thing got a bit ugly.

1. h3. application/x-www-form-urlencoded whitespaces
   Now the request has application/x-www-form-urlencoded whitespace (converted to +) and taught by the bitter experience above we use ucb.build(true).toUriString():

UriComponentsBuilder ucb = UriComponentsBuilder.fromHttpUrl("http://host.com?name=Tom+Jerry");
ucb.queryParam("album", "S&M");
ucb.build(true).toUriString();

But it's java.lang.IllegalArgumentException now - UriComponentsBuilder doesn't like "+".

Reverting to the initial approach doesn't work either:

UriComponentsBuilder ucb = UriComponentsBuilder.fromHttpUrl("http://host.com?name=Tom+Jerry");
ucb.queryParam("album", "S&M");
ucb.toUriString();

results in http://host.com?name=Tom%2BJerry&album=S%26M - note "+" being encoded now.

The only option is to do an ugly workaround mentioned in #14805.

Probable solution

IMHO it would have been much more convenient if UriComponentsBuilder decoded query params while parsing (presumably with application/x-www-form-urlencoded support; at least when creating from a request where MIME type should be available). This way both cases above are solved + a test

final String requestHttpUrl = <any valid request URL>;
final String processedHttpUrl = UriComponentsBuilder.fromHttpUrl(requestHttpUrl).toUriString();
assertEquals(processedHttpUrl, requestHttpUrl);

will pass for any valid URL which is what I'd expect. Although backward compatibility concern won't make it happen easily...

Looking forward for help with this one :D

---

Affects: 4.3.11

Issue Links:

• UriComponentBuilder doesn't work with encoded HTTP URL having '+'. [SPR-14828] #19394 UriComponentBuilder doesn't work with encoded HTTP URL having '+'. ("duplicates")

[spring-projects-issues]

Brian Clozel commented

Hi FlasH,

Sorry for the late reply. Part of this has been fixed in #19394 ("+" is a valid character in a query parameter now). Also, we've documented a bit more the whole concept of URI encoding, since this is a complex matter.

Now for the second part of this issue, the UriComponentsBuilder#fromHttpUrl API states that you should not provide there reserved characters. This is why you're providing an already encoded URL in the first place, and asking it not to encode it with build(true). But at the same time you're also providing non-encoded elements with chain calls. You should not mix both approaches for the same URL.

UriComponentsBuilder ucb = UriComponentsBuilder.fromHttpUrl("http://host.com?name={name}");
UriComponents uri = ucb.queryParam("album", "S&M").buildAndExpand("Tom&Jerry");
assertEquals("http://host.com?name=Tom%26Jerry&album=S%26M", uri.encode().toString());

ucb = UriComponentsBuilder.fromHttpUrl("http://host.com?name=Tom+Jerry");
uri = ucb.queryParam("album", "S&M").build();
assertEquals("http://host.com?name=Tom+Jerry&album=S%26M", uri.encode().toString());

This code snippet shows alternate approaches that aren't workarounds, verified to be working with Spring Framework 5.0+.

I'm closing this issue for now, as I believe both aspects are now resolved. Thanks for this report!

[spring-projects-issues]

Vsevolod Kalinin commented

Brian,
Thanks for the response, np, was not urgent. Just an effort to make a world a bit better :)

In short I totally agree with your code snippet, but in our case it was a bit more complex: UriComponentsBuilder was constructed from a real URL (i.e. HttpServletRequest, etc.) or even worse - an URL String was passed over and over to be extended.
Anyway looking back now it sounds like a bad architectural decision and therefore not a valid use case.

[spring-projects-issues]

Brian Clozel commented

Thanks for your feedback !