`LinkBuilderSupport.toUri()` double-encodes request parameters

[jochenberger]

We're experiencing a change in behavior regarding path variable expansion.
Consider the following Java code and output:

    public static void main(String[] args) {
        Link l = Link.of("/foo/{data}");
        String data = Base64.getUrlEncoder()
                .encodeToString("Hello".getBytes(StandardCharsets.UTF_8));
        System.out.println("data: " + data);
        System.out.println(l.expand(data).getHref());
    }

Output:

data = SGVsbG8=
/foo/SGVsbG8%3D

In 1.4.0, the "=" is encoded as "%3D", in 1.3.x, it wasn't. This causes issues with Spring Security because a request to the resulting URL is rejected with org.springframework.security.web.firewall.RequestRejectedException: The request was rejected because the URL contained a potentially malicious String "%25".
This is probably related to db1cd5d (#1485).
Are we doing something wrong?

[odrotbohm]

It's likely to be #1583, which implements encoding according to the rules defined in RFC 6570. That lists the equals sign (=) as reserved character. The section for simple String expansions ({var}) the defines:

For each defined variable in the variable-list, perform variable expansion, as defined in Section 3.2.1, with the allowed characters being those in the unreserved set.

I read this as: as = is part of the reserved set, it needs to be encoded on expansion. @rwinch, can you elaborate on why the exception is triggered and the encoded = is considered malicious?

[jochenberger]

We're using the resulting URL to perform a query with MockMvc. Since Spring Security complains about %25 and not about %3D, maybe they're encoding the URL again?

[dpasek-senacor]

@jochenberger We are struggling with the same problem.

Yes, MockMvc performs an additional encoding on URIs. So, if you are directly taking an href from a HATEOAS link that contains an encoded char it will perform a double encoding.
Our example

Variable value is in the format clientx:123456
URI template is /acounts/{variable}

WebMvcLinkBuilder in Spring HATEOAS 1.3.x created an URI
/accounts/clientx:123456 which is a valid URI. Colons are allowed for path segments.
This URI could directly be used by MockMvc to perform a request.

WebMvcLinkBuilder in Spring HATEOAS 1.4.x creates now URI /accounts/clientx%3A123456
If we now use the URI with WebMVC it will be encoded a second time: /accounts/clientx%253A123456 which is now longer a valid URI on our server and might break security rules as on the server it will be decoded to /accounts/clientx%3A123456 again.

A workaround is to decode the links from HATEOAS for MockMVC, e.g.: mockMvc.perform(put(decode(putAccountLink.getHref(), UTF_8)) I do not like this, but it works.

We are still in the analysis if this new behavior breaks other clients on our side, e.g. calls done by RestTemplate with variable expansion. or our Angular/React single page applications.

I think the root cause can be found in the implementation of the encoding of the `TemplateVariables, see:
https://github.com/spring-projects/spring-hateoas/blob/main/src/main/java/org/springframework/hateoas/TemplateVariable.java#L512

public String encode(String value) {
    switch (this) {
        case DOT:
        case SEGMENT:
        case PATH_SEGMENT:
        case PATH_STYLE_PARAMETER:
        case REQUEST_PARAM:
        case REQUEST_PARAM_CONTINUED:
        case SIMPLE:
            return UriUtils.encode(value, StandardCharsets.UTF_8);
        case FRAGMENT:
        default:
            return UriUtils.encodePath(value, StandardCharsets.UTF_8);
    }
}

The different variable types are encoded either by UriUtils.encode(...) or UriUtils.encodePath(...) but the mapping is not right in my opinion.

• SEGMENT, PATH_SEGMENT, SIMPLE and PATH_VARIABLE should be encoded using UriUtils.encodePath(...)
• REQUEST_PARAM and REQUEST_PARAM_CONTINUED should be encoded using UriUtils.encodeRequestParam(...)
• FRAGMENT should be encoded using UriUtils.encodeFragment(...)
  and so on.

[odrotbohm]

That's useful feedback, thank you. One thing you have to be careful about is to note that UriUtils does not claim to follow the encoding rules for UriTemplates. The rules for the latter are defined here and the unit tests for our UriTemplate expansion have been based on exactly the examples given in that RFC. To get all of those green, the implementation has to use UriUtils the way it uses it right now. I've been quite surprised by that oddity, too. Feel free to tweak the switch statement to rather use the apparently more matching methods of UriUtils and see unit tests for the template encoding fail left and right. I assume changing the encoding rules in UriUtils has quite a ripple effect, that's unlikely to be bearable for Spring applications.

I'll take this back to the Spring team and see what they think.

[dpasek-senacor]

@odrotbohm Thx for your insights. The RFC for UriTemplates is really odd as it does not seems to reflect any context information regarding the URI component the variable is used in. Always only allowing the "unreserved characters" independent of the URI component seems to be quite restrictive to build URIs via templates.

Nevertheless the encoded URI are valid URIs and would work wenn used in a browser or other clients but in combination with Spring MVC test framework (aka MockMvc) we now have a problem as links extracted from HAL resources can no longer directly be used in MockMvc if a reserved character with would be valid for a path is used for a variable value.

Should I create a simple demonstration project / test case for this behaviour?

[odrotbohm]

Anything small that helps us see the problem is appreciated.

[dpasek-senacor]

Simple test project to clarify the unexpected / changed behavior:
spring-hateoas-mockmvc.zip

Using Spring Boot 2.5.x with Spring HATEOAS 1.3.x all test will be successful.

[dpasek-senacor]

I have extended the sample project to show case that it might also break Spring RestTemplate.
RestTemplate uses own URI template handler causing double encoding.

spring-hateoas-mockmvc.zip

[dpasek-senacor]

After some further tests I need some clarification how HAL/HATEOAS links schould be handled by a client as this problem also occurs with URLs containing request parameters that contain character like a space ' '.

• Imagine an endpoint for searching resources. This endpoint support a single query param for search terms: /find?term={search_term}
• The search term might include the space or other characters to be URI encoded.
• This endpoint creates a HAL response with a self link pointing to the search, e.g. /find?term=hello%20world
• The client now extract this URL from JSON document and executes the call via a spring RestTemplate using the overloaded method accepting plain URI strings. This will lead to a double encoding, e.g. the HTTP request will be /find?term=hello%2520world. On erver side URI will be decoded to hello%20world which is not the original search term.

So my question is:
Is it mandatory for a client of HAL endpoints to decode or parse all URIs extracted from HAL content before using?
Clients which provide an API of plain text strings will perform URI encoding for these strings leading to this double encoding.
Is this the way Spring / Spring HATEOAS should be used or not?
Should all operations in Spring API with take URL strings be as unencoded URI templates, i.e. we must decode any URI strings provided by HAL links.

I also checked the Traverson client implementation and they avoid this issue as they internally always directly transform the plain text URIs to parsed URIs before using the RestOperations for performing the client call.

[dpasek-senacor]

Some further insights: the documentation of Spring is not very consistent in regard of the various REST clients and the behaviour of URI-String- and URI-based methods.

The documentation of MockMvc is good and clarifies the different methods:
https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/test/web/servlet/request/MockMvcRequestBuilders.html#get-java.lang.String-java.lang.Object...-

urlTemplate - a URL template; the resulting URL will be encoded

https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/test/web/servlet/request/MockMvcRequestBuilders.html#get-java.net.URI-

uri - the URL

So no confusion here. It should be clear that you should use URI-based method if you have a complete URL and the String-based method should be used if expanding and encoding is still necessary.

The documentation of RestOperations is inconsistent across the method overloading:
https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/web/client/RestOperations.html
The terms "URI template" and so on are used not in a proper way, for example in the exchange(...) methods.

So it seems to me, that Spring HATEOAS does not have a bug, but the usage patterns in combination with the different Spring.-based clients might need better, more consistent documentation with examples.

[ThanksForAllTheFish]

I am not sure this is fully related, but I created a repo with a very very basic test and it seems there is indeed some issue in spring hateoas: https://github.com/ThanksForAllTheFish/doubleencoding/blob/main/src/test/kotlin/com/t4atf/doubleencoding/DoubleEncodingTest.kt

@Test
fun doubleEncoding() {
	Assertions.assertEquals(
		URI.create("/root?param=${UriUtils.encode("I+will:be+double+encoded", Charset.defaultCharset())}"),
		linkTo<MyController> { test("I+will:be+double+encoded") }.toUri()
	)
}

fails with

org.opentest4j.AssertionFailedError: 
Expected :/root?param=I%2Bwill%3Abe%2Bdouble%2Bencoded
Actual   :/root?param=I%252Bwill%253Abe%252Bdouble%252Bencoded

toUriComponentBuilder() is affected as well.

The issue is the the backing uri components has its encodeState set to RAW, which then causes a seconding encoding pass when this.components.toUri() is called

@odrotbohm @dpasek-senacor (please apologies if it was not ok to tag you)

[odrotbohm]

Thanks for the sample, but I'm not gonna debug a project written in Kotlin if the issue to be discussed does not involve Kotlin in the first place. Please avoid anything that distracts from the actual problem to investigate.

[ThanksForAllTheFish]

I converted the project to java, relevant test is https://github.com/ThanksForAllTheFish/doubleencoding/blob/java/src/test/java/com/t4atf/doubleencoding/DoubleEncodingTest.java

Again, doubleEncoding fails with

org.opentest4j.AssertionFailedError: 
Expected :/root?param=I%2Bwill%3Abe%2Bdouble%2Bencoded
Actual   :/root?param=I%252Bwill%253Abe%252Bdouble%252Bencoded