UriComponents.Type.QUERY_PARAM does not match spec [SPR-10172]

[spring-projects-issues]

Dave Syer opened SPR-10172 and commented

UriComponents.Type.QUERY_PARAM does not match spec, in particular it does not allow the relatively common usage of "+" to encode " " (SPACE). So you get a nasty error with ServletUriComponentsBuilder.fromRequest whenever the query is (legally) encoded with "+" for " ". An ugly workaround is to do this:

UriComponents uri = ServletUriComponentsBuilder.fromRequest(request).replaceQuery(request.getQueryString().replace("+",  "%20")).build(true);
String query = uri.getQuery().replace("%20", "+");
return ServletUriComponentsBuilder.fromUri(uri.toUri()).replaceQuery(query).build(true).toString();

---

Affects: 3.1.2

Issue Links:

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

5 votes, 7 watchers

[spring-projects-issues]

Dave Syer commented

Link to spec for grammar: http://tools.ietf.org/html/rfc3986#appendix-A (note that query string is allowed to contain "sub-delims" which itself contains "+").

[spring-projects-issues]

Rossen Stoyanchev commented

So you get a nasty error with ServletUriComponentsBuilder.fromRequest ...

I'm not sure what the error is or what your input URI string is but this is the behavior I get, which is correct:

MockHttpServletRequest request = new MockHttpServletRequest();
request.setRequestURI("/mvc-showcase/data");
request.setQueryString("v=foo+bar");
String result = ServletUriComponentsBuilder.fromRequest(request).build().encode().toUriString();
assertEquals("http://localhost/mvc-showcase/data?v=foo%2Bbar", result);

"+" is reserved as a sub-delimeter character and must be encoded % encoded if it appears in a query name or value. "+" is used to encode spaces with HTML form encoding (here is [one previous discussion](https://jira.springsource.org/browse/SPR-5516?focusedCommentId=47792&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-47792 on the subject, although in the context of URI paths).

[spring-projects-issues]

Dave Syer commented

Hmm. This is what I get from code that looks identical:

java.lang.IllegalArgumentException: Invalid character '+' for QUERY_PARAM in "bar+spam"
	at org.springframework.web.util.UriComponents.verifyUriComponent(UriComponents.java:373)
	at org.springframework.web.util.UriComponents.verify(UriComponents.java:341)
	at org.springframework.web.util.UriComponents.<init>(UriComponents.java:105)
	at org.springframework.web.util.UriComponentsBuilder.build(UriComponentsBuilder.java:222)
	at org.springframework.security.oauth2.client.filter.OAuth2ClientContextFilter.calculateCurrentUri(OAuth2ClientContextFilter.java:126)
	at org.springframework.security.oauth2.client.filter.TestOAuth2ClientContextFilter.testCurrentUriWithIllegalSpaces(TestOAuth2ClientContextFilter.java:34)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
...

Did this change in Spring 3.2?

[spring-projects-issues]

Rossen Stoyanchev commented

In 3.2 we introduced sub-classes for hierarchical vs opaque URIs but not substantive changes. That said you're seem to be using 3.1.2. Is it possible that you're calling UriComponentsBuilder.build(boolean) with the encoded input arg set to 'true'? In which case we assume the URI is encoded and UriComponents tries to verify it and comes across "+" which should have been encoded.

[spring-projects-issues]

Dave Syer commented

Yes, I was calling build(true), but "+" is an encoding of SPACE, and the rest of the URI is encoded too, so I don't see what choice I have. The implementation of Type.QUERY_PARAM (in 3.1.2) clearly says that "+" is illegal in a query param and it clearly isn't, so surely that's a problem?

[spring-projects-issues]

Rossen Stoyanchev commented

Yes, I was calling build(true), but "+" is an encoding of SPACE

Not in the URI spec RFC 3986 ! There it is defined a special character (sub-delim). The "+" is a replacement for space in form encoding (see application/x-www-form-urlencoded). This is what Arjen explained in the discussion I referenced above.

[spring-projects-issues]

Rossen Stoyanchev commented

What's the source of the URI and what encoded it?

[spring-projects-issues]

Alexander Kharitonov commented

First servlet:
String url = "http://some.com/?var="+URLEncoder.encode("this is error text", "UTF-8");
response.sendRedirect(url);

url now: http://some.com/?var=this+is+error+text

Second servlet, mapped to "/":
String currentUriWithoutCode = ServletUriComponentsBuilder.fromRequest(request).replaceQueryParam("code").build(true).toUriString();

line above throws an exception:
java.lang.IllegalArgumentException: Invalid character '+' for QUERY_PARAM in "this+is+error+text"
at org.springframework.web.util.UriComponents.verifyUriComponent(UriComponents.java:373)
at org.springframework.web.util.UriComponents.verify(UriComponents.java:341)
at org.springframework.web.util.UriComponents.<init>(UriComponents.java:105)
at org.springframework.web.util.UriComponentsBuilder.build(UriComponentsBuilder.java:222)

[spring-projects-issues]

Rossen Stoyanchev commented

URLEncoder does not do URI encoding despite its name. From the Javadoc:

Utility class for HTML form encoding. 
This class contains static methods for converting a String to the application/x-www-form-urlencoded MIME format.
For more information about HTML form encoding, consult the HTML specification.

[spring-projects-issues]

Dave Syer commented

That may be strictly true, but we can't help it that "+" encoded spaces in query parameters are extremely common in the world - just google for plus encoded spaces and you will see how many servers do it absolutely routinely, and only some of those are even written in Java. Thus we often don't have any control over how incoming query parameters are generated, and an HttpServletRequest often contains form-encoded parameter values. I see no great harm in that and Spring Web is just splitting hairs and getting in the way of writing a robust client application.

Can we at least add an option to treat "+" as an encoded space in the builder so we don't have to workaround this (it's even uglier than I said before because, as Alexander has pointed out, there is a missing null check in the code I posted).

[spring-projects-issues]

Rossen Stoyanchev commented

Yes, we can try to do something for cases where you have to deal with such URIs. At the same time I don't think we should encourage creating them. Hence my question about the source of the URI and its encoding. Having a concrete example will certainly help. I did want to make sure we first agree on what is right according to the spec.

[spring-projects-issues]

Rossen Stoyanchev commented

I've marked this pending closure. If you use build UriComponents with encoded=true, we consider the URI encoded.

[spring-projects-issues]

Felix Barnsteiner commented

Is there a chance this will be resolved? When I submit a from with Chrome, spaces are encoded with +. So if I for example google for spring mvc, the url will be https://www.google.de/#q=spring+mvc. In my application, I obviously don't have control over how browsers encode spaces. So when trying to get the full request url with ServletUriComponentsBuilder.fromRequest(request).build(true).toUriString() I get this nasty exception. Please fix.

This unit test reproduces the problem:

@Test
public void testQueryParamSpaceEncodesAsPlusSign() throws Exception {
	final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo");
	request.setQueryString("foo=bar+baz");

	assertThat(fromRequest(request).build(true).toUriString(), is("/foo?foo=bar+baz"));
}

[spring-projects-issues]

Ruslan Stelmachenko commented

A agree with Felix.

All popular browsers I tried encode space as "+" when I just enter "spring mvc" into google.com's search box.

Chrome 65.0: https://www.google.com/search?q=spring+mvc&oq=spring+mvc
Firefox 59.0: https://www.google.com/search?q=spring+mvc&oq=spring+mvc
IE 11.0: https://www.google.com/search?q=spring+mvc&oq=spring+mvc

(I removed additional query params to make it clear)

All three is not "%20" but "+" encoded.

So, if all the world do this, maybe it is time to deprecate RFC...
But at least we can make the Framework to follow the real world standard instead of formally be RFC-strict?

[spring-projects-issues]

Rossen Stoyanchev commented

This was fixed in 5.0 with #19394, which makes this a duplicate.