Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc: UriComponentsBuilder does not encode query parameters [SPR-14256] #18828

Closed
spring-issuemaster opened this issue May 6, 2016 · 5 comments
Closed

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented May 6, 2016

Christopher Smith opened SPR-14256 and commented

UriComponentsBuilder does not encode query parameters, expecting them to be encoded by the client already. The documentation does not indicate one way or another whether pre-encoding is necessary, and it would be helpful to note that it is.


Affects: 4.3 RC1

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 22, 2016

Rossen Stoyanchev commented

Query parameters it does encode, so I'm not quite sure what scenario you're looking at. The following works and is documented in the Javadoc:

UriComponents uriComponents = UriComponentsBuilder.fromUriString("/path?q={var}")
		.buildAndExpand("a=b")
		.encode();

assertEquals("q=a%3Db", uriComponents.getQuery());
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 23, 2017

Tristan Lins commented

Here are two issues I have with the UriComponentsBuilder:

The `&` will be encoded twice:

```java 
@Grab(group='org.springframework', module='spring-web', version='4.3.10.RELEASE')
import org.codehaus.groovy.runtime.InvokerHelper
import org.springframework.web.util.UriComponentsBuilder

URI source = new URI("http://example.com?foo=b%26r") // ?foo=b&r
URI target = UriComponentsBuilder.fromUri(source).build().toUri()

assert Objects.equals(source, target): "${source} != ${target}"

Ends with AssertionError: http://example.com?foo=b%26r != http://example.com?foo=b%2526r


The & will not be encoded either:

@Grab(group='org.springframework', module='spring-web', version='4.3.10.RELEASE')
import org.codehaus.groovy.runtime.InvokerHelper
import org.springframework.web.util.UriComponentsBuilder

URI source = new URI("http://example.com?foo=b%26r") // ?foo=b&r
URI target = UriComponentsBuilder.newInstance()
                 .scheme("http")
                 .host("example.com")
                 .queryParam("foo", "b&r")
                 .build().toUri()

assert Objects.equals(source, target): "${source} != ${target}"

Ends with AssertionError: http://example.com?foo=b%26r != http://example.com?foo=b&r

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Sep 1, 2017

Rossen Stoyanchev commented

In the second example, you're missing the instruction to encode:

URI target = UriComponentsBuilder.newInstance()
		.scheme("http")
		.host("example.com")
		.queryParam("foo", "b&r")
		.build()
		.encode()
		.toUri();

The reason the first encodes is because the constructor of URI encodes "%". In that case the source URI is already encoded and you could specify that through a flag on the build method:

URI target = UriComponentsBuilder.fromUri(source).build(true).toUri();

That said arguably when fromUri(URI) is used, the "encoded" flag should be assumed. That's an improvement we can make but please open a separate ticket since it's much more specific, i.e. something like "Double encoding issue when using UriComponents#fromUri(URI)".

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Sep 1, 2017

Rossen Stoyanchev commented

Resolving as "Works as designed" based on original description.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Sep 4, 2017

Tristan Lins commented

Thanks for explanation, I give it a try :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.