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

Encoding of URI Variables on RestTemplate [SPR-16202] #20750

Closed
spring-issuemaster opened this Issue Nov 15, 2017 · 19 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Nov 15, 2017

Nicolas Miranda opened SPR-16202 and commented

On Version 5.0.1 encoding works different than version 4.x.

Given the following code in version 5 the parameter is received by any endpoint with an space "a b" while in version 4 it was received with "a+b". I tried to encode the + sign with %2B but is received as %2B. I don't know if its a bug on version 4 or version 5 but it works different.

// Some comments here
	RestTemplate restTemplate = new RestTemplate();
    	String url = "http://localhost:8080/test?param1=a+b&param2=d";
    	HttpHeaders headers = new HttpHeaders();
    	HttpEntity<String> req = new HttpEntity<String>(new String(""), headers);
    	String restResp = restTemplate.postForObject(url, req, String.class);
    	
    	System.out.println(restResp);

Affects: 5.0.1

Issue Links:

  • #22006 Not encoding '+' in URLs anymore breaks backwards compatibility with apps running on spring 4 ("is duplicated by")
  • #19394 UriComponentBuilder doesn't work with encoded HTTP URL having '+'.
  • #20968 [docs] Explain URI template encoding
  • #21577 Support stricter encoding of URI variables in UriComponents ("is superseded by")

0 votes, 7 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Nov 16, 2017

Brian Clozel commented

I've tried the following:

template.getForObject("http://localhost:8080/?param=a+b", String.class);

With a break point in SimpleClientHttpRequestFactory, I can see that the plus sign is still there.
Capturing the traffic with Wireshard yields the same result.

With a breakpoint inside the following application, I can see that the param has been decoded when injected with @RequestParam. But getting the raw query string shows the expected value.

@Controller
public class HelloController {

	@GetMapping("/")
	@ResponseBody
	public String hello(@RequestParam String param, HttpServletRequest request) {
		// query = "param=a+b"
                String query = request.getQueryString();
		// param = "a b"
		return param;
	}
}

Could you create a sample application that demonstrate the issue?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Nov 17, 2017

Nicolas Miranda commented

Did you try with a POST Request? This is the app I using for test.

@Controller
@RequestMapping("/") 
public class TopController {
	
	@RequestMapping(method = RequestMethod.GET)
	@ResponseBody
	public String top() {

		RestTemplate restTemplate = new RestTemplate();
	    	String url = "http://localhost:8080/test?param1=a+b&param2=d";
	    	HttpHeaders headers = new HttpHeaders();
	    	HttpEntity<String> req = new HttpEntity<String>(new String(""), headers);
	    	String restResp = restTemplate.postForObject(url, req, String.class);
    	
		return restResp;
	}

	@RequestMapping(value="/test", method = RequestMethod.POST)
	@ResponseBody
	public String test(@RequestParam(name="param1", required=true) String param1,
			@RequestParam(name="param2", required=true) String param2,
HttpServletRequest request) {

                System.out.println(request.getQueryString());
		System.out.println(param1);
		System.out.println(param2);
		return param1.concat(param2);
	}

}

Result in the console is (with Spring 5):

a b
d
"a bd"

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Nov 17, 2017

Nicolas Miranda commented

getForObject() gives me the same result. I added a System.out.println(request.getQueryString());

Results for Spring 4:
param1=a%2Bb&param2=d
a+b
d
"a+bd"

Results for Spring 5:
param1=a+b&param2=d
a b
d
"a bd"

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Nov 17, 2017

Nicolas Miranda commented

I debugged with the two versions of Spring and the URI is changed inside this call:

SPRING 5.0.1.RELEASE

RestTemplate:670 - URI expanded = getUriTemplateHandler().expand(url, uriVariables);
Value after call: (http://localhost:7081/admin/test?param1=a+b&param2=d)

SPRING 4.3.7.RELEASE

RestTemplate:612 - URI expanded = getUriTemplateHandler().expand(url, uriVariables);
Value after call: (http://localhost:7081/admin/test?param1=a%2Bb&param2=d)

I tried too in Spring 5 to change the encodingmode with any result.

DefaultUriBuilderFactory uriBuilder = new DefaultUriBuilderFactory();
	uriBuilder.setEncodingMode(EncodingMode.NONE); // Or VALUES_ONLY
	restTemplate.setUriTemplateHandler(uriBuilder);
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Nov 17, 2017

Tapio Koskinen commented

This issue is probably related to #19394.

I don't know if plus signs should be automatically encoded or not, but there should be an easy way to encode them if they actually represent plus signs and not spaces.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Nov 17, 2017

Brian Clozel commented

Hi Nicolas Miranda,

Thanks for the additional information. I managed to track down this behaviour to #19394. "+" signs are allowed in query parameters and were wrongfully encoded previously - it seems you were relying on that.

A quick way to achieve what you're trying to do is to take full control over the encoding and provide your own URI:

URI uri = URI.create("http://localhost:8080/foo/?param=a%2Bb");
String responseString = restTemplate.postForObject(uri, req, String.class);

Indeed, manually encoding the query param before the expansion will result in double encoding:

UriTemplateHandler factory = restTemplate.getUriTemplateHandler();
URI uri = factory.uriString("http://localhost:8080/foo").queryParam("param", "{param}").build("a%2Bb");
// http://localhost:8080/foo/?param=a+b

URI otherUri = factory.uriString("http://localhost:8080/foo").queryParam("param", "{param}").build("a%2Bb");
// http://localhost:8080/foo/?param=a%252Bb

Latest comments in #19394 suggest that you're not alone in this case.
Rossen Stoyanchev, Arjen Poutsma, what do you think?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Nov 17, 2017

Rossen Stoyanchev commented

It is possible to apply "full" (or strict) encoding, essentially encoding any character that might have reserved meaning in a URI. That can be applied to URI variable values and this is what the VALUE_ONLY encoding mode is. For example this works:

DefaultUriBuilderFactory builderFactory = new DefaultUriBuilderFactory();
builderFactory.setEncodingMode(EncodingMode.VALUES_ONLY);
URI uri = builderFactory.uriString("http://localhost:8080/test?param1={value}&param2=d").build("a+b");
assertEquals("http://localhost:8080/test?param1=a%2Bb&param2=d", uri.toString());

The same with the RestTemplate:

DefaultUriBuilderFactory builderFactory = new DefaultUriBuilderFactory();
builderFactory.setEncodingMode(EncodingMode.VALUES_ONLY);
restTemplate.setUriTemplateHandler(builderFactory);

HttpEntity<String> request = new HttpEntity<String>("");
String restResp = restTemplate.postForObject("http://localhost:8080/test?param1={value}&param2=d", request, String.class, "a+b");

The last comment under #19394 is really more like a long back and forth of comments. As I said there i am open to other "modes" of encoding, which we can codify through the EncodingMode option on DefaultUriBuilderFactory. I just need an idea or definition of how the encoding mode should work.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Nov 20, 2017

Nicolas Miranda commented

Thanks. I was trying before using DefaultUriBuilderFactory with that encoding but it only works with expanded URI variables. Now it works.

It can work, but I think that for easy migration for Spring 4 to Spring 5 it would be good if RestTemplate have some way to initialize using old behavior mode or lot of people will have the same problem or undesired bugs if it's not well documented somewhere.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Nov 27, 2017

Rossen Stoyanchev commented

Nicolas Miranda, there is no change of behavior between 4 and 5. The only thing we've done is to deprecate DefaultUriTemplateHandler in favor of DefaultUriBuilderFactory, and the strict encoding mode of the latter behaves exactly the same way as the EncodingMode.VALUES_ONLY of the latter. In both 4 and 5 you have to explicitly configure the uriTemplateHandler property of the RestTemplate. Am I missing anything?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jan 25, 2018

Manfred Hantschel commented

We also detected a change of behavior between Spring 4 and 5. We are using the UriComponentsBuilder to add Base64 encoded parameters, that may contain "+". After the upgrade to Spring 5 some of our tests failed.

This test works with Spring 4, but fails with Spring 5 (it uses the Apache HttpClient for decoding):

    @Test
    public void test()
    {
        String value = "A+B=C";
        
        URI uri = UriComponentsBuilder.newInstance().queryParam("test", value).build().encode().toUri();
        
        List<org.apache.http.NameValuePair> parameters =
            org.apache.http.client.utils.URLEncodedUtils.parse(uri, Charset.forName("UTF-8"));

        // Spring 4: A+B=C
        // Spring 5: A B=C
        
        assert value.equals(parameters.get(0).getValue());
    }

In fact, by sending a PLUS unencoded will be decoded by most libraries as SPACE, while sending a PLUS encoded, does not harm them.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jan 25, 2018

Rossen Stoyanchev commented

Well, harm depends on your intent, i.e. you may want to encode the "+" or not as in #19394. The reason we made the change in 5.0 based on #19394 is because it's consistent with how UriComponents does encoding, which is to encode only characters that are reserved for a given URI component.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jan 26, 2018

Manfred Hantschel commented

The problem with the + is, that it was once used for a space, see https://tools.ietf.org/html/rfc1866#section-8.2.1. In the current URL spec it's a reserved word, see https://tools.ietf.org/html/rfc3986#section-2.2. There are a lot of libraries, that still decode + as space. The preferred way around this problem ist to encode the + as %2B. With the current implementation it seems to be impossible to add a parameter with a +, if the other side decodes it as space (if there is a way, please let me know). In #19394 the parameters are pre-encoded, but the implemenation didn't not allow "+" as space and double-encoded %2B. The problem is valid, but it seems, that the fix wasn't the best choice and provoked this issue.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jan 26, 2018

Rossen Stoyanchev commented

HierarchicalUriComponents reflects the syntax for RFC 3986. You can see "+" is correctly defined as a sub-delimiter. In section 3.4 you'll see the definition for query > pchar > sub-delimiters. So "+" has reserved meaning in a URI, yes, but it is allowed in a query, and and we only encode characters that are not allowed.

I can understand this mode of encoding can be confusing, if what you want is to to encode all characters with reserved meaning. It is possible to switch to a different mode of encoding as I explained in this comment.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jan 26, 2018

Rossen Stoyanchev commented

I've created #20968.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jan 29, 2018

Daniel Furtlehner commented

It is good and i fully understand that the HierarchicalUriComponents reflect the standard for 100%.

But the problem I have now is the following (Please correct me when I make wrong assumptions):

  1. As UriComponentsBuilder is documentet in 4.3.x and 5.x (https://docs.spring.io/spring/docs/4.3.14.RELEASE/spring-framework-reference/htmlsingle/#mvc-uri-building) i assume that it is intended to be used by everyone.
  2. The encoding of the query string changed between Version 4 and 5 (the + is not encoded anymore, because the standard says it is not required to encode it). So this is a breaking change between the two versions. (Did not find any informations about this in the release notes. Maybe i did not look close enough)
  3. There are implementations out there, that treat a + in the query string as a space character when decoding. I checked the code for tomcat-embed 8.5.2 and in org.apache.tomcat.util.http.Parameters on line 325 it says we should decode a + when we find one (This also happens in the query string). And later on it calls the org.apache.tomcat.util.buf.UDecoder and on line 106 it replaces a + with a space. This is hardcoded. I can't find a way to change the behaviour of tomcats request parsing.

With this information in mind I cannot find a way to use the UriComponentsBuilder to send a request to the server, that preserves the + in the query string after it was decoded by tomcats parsing logic. This was possible with version 4 of spring.

Maybe you have an idea how one can send a + sign as query parameter to the server. Your last comment says one can use DefaultUriBuilderFactory. But i can't see how this is a full replacement for using UriComponentsBuilder directly.

Thanks for you time and patience :)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jan 31, 2018

Rossen Stoyanchev commented

UriComponentsBuilder is a basic class, a slightly extended version of java.net.URI, but still pretty basic, and following the same model for encoding as described in the URI constructor under "Escaped octets, quotation, encoding, and decoding". The two should be consistent with each other, and as of 5.0 that includes the treatment of "+".

I don't see much room for providing multiple approaches to encoding in UriComponentsBuilder itself without complicating the API. It's easier to apply those as an additional layer, while using UriComponentsBuilder as a foundation.

Above I suggested a solution through DefaultUriBuilderFactory which is plugged into the RestTemplate and the WebClient but the same should also be usable directly. I'd be curious to hear if there are limitations you anticipate from using DefaultUriBuilderFactory, or did you mean it simply, from an upgrade point of view?

That said if what you want to use UriComponentsBuilder directly the closest I can suggest is:

String value = "A+B=C";
value = UriUtils.encode(value, StandardCharsets.UTF_8);
URI uri = UriComponentsBuilder.newInstance().queryParam("test", value).build(true).toUri();

Or if using URI variables, you can apply UriUtils#encodeUriVariables to the values prior to expanding.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jan 31, 2018

Daniel Furtlehner commented

Thank you for your answer.

I try to explain our use case to you.
We maintain a application that acts as a IdentityProvider for about 200 connected applications. So users authenticate to our Portal and then they can access all the applications via Single Sign On. Therefor we have to build URIs to redirect the User to.

// At first we start with a base uri
String url = "https://something.com?param=Some+thing"; // This url is different for each application. Sometimes with parameters. Sometimes without.
UriComponentsBuilder builder = UriComponentsBuilder.fromUriString(url);

// Next we add some additional parameters
builder.queryParam("paramName", "ParamValueThatContains+Sign");

// Now build the complete URL
UriComponents components = builder.build(); // Can't use builder.build(true) here. Because there might be something inside that is not encoded yet. Everything is dynamic.
String url = components.encode(encodingType).toUriString(); // Version 4 escaped the + in the query parameter.

// Send redirect
response.sendRedirect(url);

With version 5 the server gets the URL https://something.com?param=Some+thing&paramName=ParamValueThatContains+Sign and decodes it. At least Apache Tomcat in Version 8.5 decodes the parameter "paramName" to "ParamValueThatContains Sign". And this is not what we tried to send to the other application.

UriComponentsBuilder was the perfect fit for our UseCase so far.
But now I checked DefaultUriBuilderFactory again and noticed, that it returns a UriBuilder that has almost the same API than the UriComponentsBuilder. I think this, in combination with the right encoding mode, can be a replacement for the UriComponentsBuilder for our usecase.

Did no recognized, that the factory can be used like that when reading your comment at first.

Sorry for the trouble and thank you for your time :)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Mar 7, 2018

Rossen Stoyanchev commented

Daniel Furtlehner, just to close out the discussion.

As part of #20968 I've overhauled the "URI Links" chapter, with present day coverage of what's available, including a sub-section on URI encoding. I think we've adequate options for control over URI encoding when working through the RestTemplate and WebClient, or at a lower level, through the DefaultUriBuilderFactory.

What I think we still have an opportunity for is extra flexibility when working directly with UriComponents for full encoding of all reserved chars in URI variables. I think it's feasible but I haven't yet found an API option that wouldn't make things more confusing.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator

spring-issuemaster commented Jul 13, 2018

Rossen Stoyanchev commented

Manfred Hantschel, Daniel Furtlehner, Nicolas Miranda, please follow #21577 which should address this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment