UriTemplate doesn't encode query parameters [SPR-8403]

[spring-projects-issues]

Dave Syer opened SPR-8403 and commented

Maybe people tend not to use UriTemplate with query strings? UriUtils actually has some code for encoding query parameters and for encoding the whole query string, but UriTemplate only uses the latter. The query parameter encoding method is used by the JSP tag UrlTag, so from that I assume that there is a good technical reason for doing the encoding. Example from the JSP case (where UriTemplate will leave the input unchanged):

foo?bar=a=b&c -> foo?bar=a%3Db&c

I think it would be easy to implement (by splitting the query string on '&' and feeding the parts through the parameter encoding method) if we can agree on the principle.

---

Affects: 3.0.5

Issue Links:

• UriUtils does double encode [SPR-8507] #13153 UriUtils does double encode
• RestTemplate cannot support OAuth [SPR-6143] #10811 RestTemplate cannot support OAuth
• UriTemplate takes wrong approach at encoding substituted template variables [SPR-8662] #13304 UriTemplate takes wrong approach at encoding substituted template variables

[spring-projects-issues]

Rossen Stoyanchev commented

Assigning to Arjen for comment.

[spring-projects-issues]

Arjen Poutsma commented

The underlying problem that it is impossible determine where a specific URL component (scheme, userinfo, host, port path, query, fragment) of a given String starts and where one ends. We can get close by using using regexes (as we currently do in UriUtils), but it's not completely fool proof. For instance, consider the following String

http://www.example/foo?bar/baz?qux

in this case, '/foo?bar/baz' should be considered the path, and 'qux' the query part. So the following code:

System.out.println(UriUtils.encodeUriComponents("http", "", null, "www.example.com", null, "/foo?bar/baz", "qux", null));

will result in

http://www.example.com/foo%3Fbar/baz?qux

Given all this background info, I am not sure what we can do to improve the situation.

[spring-projects-issues]

Dave Syer commented

Hmm, so the general case is unavoidably hard. I would still say we can do a better job of easier parts of the problem (like the example I gave). It seems reasonable to adopt some rules of thumb (possibly with overridable behaviour or configurable) in the template, like "the first colon is the scheme separator" and "the first/last question mark is the query string separator".

[spring-projects-issues]

Dave Syer commented

For the sake of clarification: the basic use case is RestTemplate.execute(). It takes a URI template and a map of variables to insert, but it doesn't allow you to append query parameters, only to insert them in the template through placeholders. The goal, the, then is to encode the values before they are inserted in the template, but only if they are query parameters. I'll throw that open to discussion before trying to conclude that it is implementable.

[spring-projects-issues]

Rossen Stoyanchev commented

I don't think the issue is with recognizing the query string part. RFC 3986 says the query component is indicated by the first question mark ("?") character. UriUtils handles it correctly -- given http://www.example/foo?bar/baz?qux the query string is 'bar/baz?qux'. The challenge I think is with parsing the query string since parameter names and values can contain '&' and '='.

UrlTag is able to handle this because it separates the path from the query string. Its URI template contains the path portion only. Param tags within the url tag are used to expand the URI template and only then any unused params are appended as query string parameters.

We can't use this approach in the RestTemplate. The UrlTag accepts (explicit) param sub-elements while the RestTemplate accepts a Map that can contain any amount of data, not necessarily something to be included in the URL. It would also be a breaking change as currently unused URI template variables are ignored.

For starters we can document more prominently in the RestTemplate and in the UriTemplate that '=' and '&' in query parameter names and values can not be encoded.

The only other idea I see is some syntax inside a URI template variable. For example instead of this:

/foo?{bar}

Have this:

/foo?{queryParam:bar}

The UriTemplate can then look for a variable called "bar" and treat it as a query string parameter resulting in:

bar=a%3Db&c

[spring-projects-issues]

Rossen Stoyanchev commented

I've changed the issue type to improvement.

[spring-projects-issues]

Dave Syer commented

I think that's a great idea and possibly the best way forward without needing to change the interfaces or UriTemplate or RestTemlpate. I'm not sure if a single keyword is enough to cover all the use cases. E.g in your example above the placeholder "bar" had a value "bar=a=b&c" and the UriTemplate has to treat the first equals sign as a name-value separator, so maybe that should be "queryKeyValue". The existing behaviour would be "queryString". And the most common use case would probably be

/foo?bar={queryValue:bar}

with { bar=a=b&c } and the same encoded result as above. There might be a need for keywords covering the other parts of the URI spec as well (not just the query part), since they all have different rules for encoding and allowed characters.

[spring-projects-issues]

Rossen Stoyanchev commented

It seems the URI template spec defines expression expansion mechanisms that includes query params:
http://tools.ietf.org/html/draft-gregorio-uritemplate-06#section-3.2.8
http://tools.ietf.org/html/draft-gregorio-uritemplate-06#section-3.2.9

[spring-projects-issues]

Rossen Stoyanchev commented

UriTemplate is now backed by UriComponentsBuilder, which breaks down the URI template as best as it can allowing it to expand and encode each part accordingly.

The following now works (but didn't before):

UriTemplate template = new UriTemplate("foo?bar={bar}&c");
URI result = template.expand("a=b");
assertEquals(new URI("foo?bar=a%3Db&c"), result);

However this won't work the same way:

UriTemplate template = new UriTemplate("foo?bar=a=b&c");
URI result = template.expand("a=b");
assertEquals(new URI("foo?bar=a=b&c"), result);  // actual: "foo?bar=a&b&c"

The regular expression uses "=" to find param name-value pairs. We could switch to a different convention but ultimately the above string is ambiguous.

Essentially one should provide a URI template that can be parsed without ambiguity like in the 1st example. Previously that was impossible since the template was parsed after it was expanded. Now the URI template is parsed first and then the individual parts expanded and encoded separately.

Dave does that work for you?