UriComponentsBuilder '{' '}' may not be encoded although invalid characters

[jonenst]

Using the UriComponentsBuilder, the { and } characters can end up in the result if you are not careful (they are the only ones from the invalid printable ascii chars which do this, most probably because they are used for templates, like in {city}).

jshell> UriComponentsBuilder.fromUriString(" \"<>\\^`|][%{}").encode().build().toUriString()
$2 ==> "%20%22%3C%3E%5C%5E%60%7C%5D%5B%25{}"
// {} not percent encoded at the end

jshell> UriComponentsBuilder.fromUriString(" \"<>\\^`|][%}{").encode().build().toUriString()
$3 ==> "%20%22%3C%3E%5C%5E%60%7C%5D%5B%25%7D%7B"
// }{ correctly percent encoded at the end

Using toUri() instead of toUriString() at least does check and throws an exception in the bad case.

 jshell> UriComponentsBuilder.fromUriString("}{").encode().build().toUri()
$4 ==> %7D%7B

jshell> UriComponentsBuilder.fromUriString("{}").encode().build().toUri()
|  Exception java.lang.IllegalStateException: Could not create URI object: Illegal character in path at index 0: {}

Using toUri() and removing .encode() actually makes it encode:

jshell> UriComponentsBuilder.fromUriString(" \"<>\\^`|][%{}").encode().build().toUriString()
$2 ==> "%20%22%3C%3E%5C%5E%60%7C%5D%5B%25{}"
// As seen before, with .encode() and .toUriString(): {} not encoded

jshell> UriComponentsBuilder.fromUriString(" \"<>\\^`|][{}").build().toUri();
$8 ==> %20%22%3C%3E%5C%5E%60%7C%5D%5B%7B%7D
// without .encode() and with .toUri(): {} encoded !?

With buildAndExand(), things are a bit safer, but still there are cases where it lets unencoded chars through.

 jshell> UriComponentsBuilder.fromUriString("{a}").buildAndExpand().toUriString()
|  Exception java.lang.IllegalArgumentException: Not enough variable values available to expand 'a'
// a bit safer, expand detects the missing argument

jshell> UriComponentsBuilder.fromUriString("{}").buildAndExpand().toUriString()
$29 ==> "{}"
// empty brackets are neither encoded nor detected as errors.

[sbrannen]

I've edited your comment to improve the formatting. You might want to check out this Mastering Markdown guide for future reference.

[jonenst]

Thanks. Sorry for the bad formatting.

I found another place where '{' slips through (and in addition to '}' slipping through, the wrong name must be passed as the key in the uriVariable map to avoid getting an exception):

jshell>  UriComponentsBuilder.fromUriString("/{foo{}}").encode().build().expand("bar")
$132 ==> /bar}

jshell>  UriComponentsBuilder.fromUriString("/{foo{}}").encode().build().expand(Map.of("foo{","bar"))
$133 ==> /bar}
# the key in the map should be foo{} !

jshell>  UriComponentsBuilder.fromUriString("/{foo{}}").encode().build().expand(Map.of("foo{}","bar"))
|  Exception java.lang.IllegalArgumentException: Map has no value for 'foo{'
# throws :(

This one is because UriTemplateEncoder chooses the variable name to be "foo{}" by correctly tracking the nesting level and so correctly doesn't encode this variable name, but the expandUriComponent uses Pattern.compile("\{([^/]+?)}") to find names to replace and so stops at the first '}'. Normal regexps can't track arbitrary high nesting levels, so there's no easy fix for this one while still allowing names of variables to contain correctly nested brackets. Note that as soon as a variable name is incorrectly nested, it becomes considered as a literal (with it's surround '{' and '}'):

jshell>  UriComponentsBuilder.fromUriString("/{foo{}}").encode().build().expand("bar")
$140 ==> /bar}
# correctly nested

jshell>  UriComponentsBuilder.fromUriString("/{foo{}").encode().build().expand("bar")
$141 ==> /%7Bfoo%7B%7D
# incorrectly nested, treated as a literal, bar is not used

jshell>  UriComponentsBuilder.fromUriString("/{foo}}").encode().build().expand("bar")
$142 ==> /bar%7D
# incorrectly nested, closes too soon, the remaining '}' is treated as a literal

There are more weird manifestations of the discrepancy the UriTemplateEncoder code and the expandUriComponent code:

jshell>  UriComponentsBuilder.fromUriString("/{foo{}{}}").encode().build().expand("bar")
|  Exception java.lang.IllegalArgumentException: Not enough variable values available to expand '}'
# this should have worked, a single variable "foo{}{}" gets its value from the positional argument.
# But it throws about a bogus second variable.
jshell>  UriComponentsBuilder.fromUriString("/{foo{}{}}").encode().build().expand("bar", "xxx")
$144 ==> /barxxx
# correctly nested, so  UriTemplateEncoder treats it as one unencoded variable
# but the regexp in expandUriComponent matches 2 variables. Chaos ensues.

[poutsma]

We are aware of the fact that it's impossible to correctly parse a string into a URL using a regex. But I would argue a even stronger case: it's impossible to correctly parse URL strings at all, for a number of reasons:

The typical developers idea of what a URL is differs significantly from the relevant specifications. We have no intention on trying to enforce these specs all of our users, and instead try to be more pragmatic instead. For instance, this leads to us supporting url template placeholders where strictly they are not allowed.

Secondly, there is URL encoding. It is impossible to distinguish between encoded and unencoded components, and even if you could, there are too many encoding choices available to guess correctly 100% of the time. Again, we could have decided to only support the specifications, but instead we try to be more lenient where we can.

Overall, the goal of the UriComponentsBuilder is to be useful for Spring Framework itself, and its users. The goal is not to strictly conform to URL RFCs, though we try to do so where we can. The parse methods on the UriComponentsBuilder are essentially best guesses, with absolutely no guarantee of parsing the given string correctly, because that is impossible anyway—whether using a regex or not.

Given the above, I am not at all surprised that the builder has difficulty parsing strings like \"<>\\^|][%{}or/{foo{}{}}`. In fact, I am surprised the former is even accepted at all, because it's not a valid URI. Both simply have too much ambiguity, as we do allow placeholders. If we would resolve this and not allow placeholders in those places, we would break the vast majority of the Spring apps out there. So that is not an option.

However, there is a way to resolve this. As the name of the class suggests, the UriComponentsBuilder can also be used as a builder, where you can supply individual components (scheme, host, path, etc) one at a time. Used in this way, there is less ambiguity to solve, so the builder generally does a better job.

[rstoyanchev]

I think I see where the issue is and it's related to the encode() at the UriComponentsBuilder level. That encodes the template while it still contains URI variables, while URI variables themselves are encoded more strongly later (i.e. all URI reserved characters as opposed to just legal ones) when expanded. This means that if you don't expand, you can end up submitting a URI with what was expected to be placeholders.