Benjamin Conlan opened SPR-17465 and commented

When using the reactive web WebClient's uribuilder building a Uri path value doesn't get encoded as expected:

(Excuse code here... Composed from phone)

webClient.get().uri(builder -> builder.path("/{test}").build("encode/this"))

Behaves differently to:

webClient.get().uri("/{test}", "encode/this")

I would expect the url path segment in the 1st case to also be encoded.

Affects: 5.0.8

Issue Links: - #22064 WebClient no longer encodes query parameters ("is duplicated by")

Referenced from: commits https://github.com/spring-projects/spring-framework/commit/24051619a5d67848ebee635170325d8842a2b445, https://github.com/spring-projects/spring-framework/commit/e4c84ec7576bc2986357b5bf12b646d663ef9730

Backported to: 5.0.11

Comment From: spring-projects-issues

Rossen Stoyanchev commented

Indeed there is an issue.

The explanation is made a little more complicated by the fact that in 5.1, the encoding mode for DefaultUriBuilderFactory changed as part of the work for #21577. Prior to 5.1 the encoding mode was URI_COMPONENTS where if you use path(..), as opposed to pathSegment(..), then "/" is allowed and therefore not encoded. So technically this example is not a regression, but using pathSegment in 5.1 does not work either because which is a regression.

After the change to encoding mode TEMPLATE_AND_VALUES in 5.1, URI variables should be strictly encoded, regardless of whether you use path or pathSegment, due to a bug when using the builder option. After the fix both of the above examples should encode as expected.

For more background on those options see the reference docs.

Comment From: spring-projects-issues

Rossen Stoyanchev commented

Benjamin Conlan I updated my previous comment to say 5.1 (instead of 5.0.8), as it should have been.

For 5.0.x the example you gave is expected behavior I'm afraid. You would have to use pathSegment("{test}") instead of path("/{test}"), or as of 5.0.11 (i.e. after the fix) switch the encoding mode from URI_COMPONENT to TEMPLATE_AND_VALUES.

For 5.1.x after the fix in 5.1.3 the example should just work since in 5.1.x the default encoding mode is TEMPLATE_AND_VALUES by default.

Comment From: marceloverdijk

I'm upgrading from 5.1.2 to 5.1.7 and facing this issue.

        String query = "{foo}";

        client.get().uri(uriBuilder -> uriBuilder.path("/graphql")
                .queryParam("query", query)
                .build())
                .accept(MediaType.APPLICATION_JSON_UTF8)
                .exchange()
                .expectStatus().isOk();

now returns java.lang.IllegalArgumentException: Not enough variable values available to expand 'foo'.

When using 5.1.2 we encoded the query string manually like:

        String query = "{foo}";
        String queryString = URLEncoder.encode(query, "UTF-8");

        client.get().uri(uriBuilder -> uriBuilder.path("/graphql")
                .queryParam("query", queryString)
                .build())
                .accept(MediaType.APPLICATION_JSON_UTF8)
                .exchange()
                .expectStatus().isOk();

but also this won't work anymore > 5.1.2.

@rstoyanchev what should be the approach from 5.1.3 onwards?

Comment From: rstoyanchev

@marceloverdijk, in 5.1.x TEMPLATE_AND_VALUES encoding applies by default, which is why pre-encoding any part of the template leads to double encoding. In 5.1.2 there was a bug that prevented this.

This encoding mode however applies full encoding to URI variables so you can do:

String query = "{foo}";

client.get().uri(uriBuilder -> uriBuilder.path("/graphql")
        .queryParam("query", "{query}")
        .build(query))
        .accept(MediaType.APPLICATION_JSON_UTF8)
        .exchange()
        .expectStatus().isOk();

Comment From: marceloverdijk

Thx @rstoyanchev that worked!

Comment From: leonard84

@rstoyanchev this works for fixed parameters, but how would you suggest handling dynamic query parameters. Having to create an array to hand over to build() seems cumbersome and most of all really error prone.

Here is a short example:

List<String> rels = ... // list of strings also containing special chars

webClient.get().uri(baseUri, uriBuilder -> {
         uriBuilder.queryParam("email", "{email}"); 
         rels.forEach(rel -> uriBuilder.queryParam(REL_PARAM, rel)); // how should we handle this?
         return uriBuilder.build(email);
 }

Comment From: rstoyanchev

UriUtils#encodeQueryParams perhaps, assuming the rest of the URI template is already encoded.