UriBuilder query parameter encoding behaves differently depending on required RequestParameter

[pax95]

I'm upgrading from sb 1.5.9.RELEASE to 2.5.1 i steps and noticed a different behaviour in how the UriBuilder handles encoding of query parameters depending on if you set the RequestParam required to true or false.
The queryparamters ZonedDateTime has to be encoded (+sign), but is not if the RequestParam(required=false).
I would expect the parameter to be encoded no matter if it's required or not.
Running the example code will result in the first 'from' parameter to be encoded, but the 'to' not.
I think the issue lies in the DefaultUriBuilder or related. Spring Hateoas uses this to render links.

Following the link will result in ->
Resolved [org.springframework.web.method.annotation.MethodArgumentTypeMismatchException: Failed to convert value of type 'java.lang.String' to required type 'java.time.ZonedDateTime'; nested exception is org.springframework.core.convert.ConversionFailedException: Failed to convert from type [java.lang.String] to type [@org.springframework.web.bind.annotation.RequestParam @org.springframework.format.annotation.DateTimeFormat java.time.ZonedDateTime] for value '2021-06-18T14:59:05.802044 02:00[Europe/Copenhagen]'; nested exception is java.lang.IllegalArgumentException: Parse attempt failed for value [2021-06-18T14:59:05.802044 02:00[Europe/Copenhagen]]]
Because + is treated as space.

Sample application ->

import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.linkTo;
import static org.springframework.hateoas.server.mvc.WebMvcLinkBuilder.methodOn;

import java.time.ZonedDateTime;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.format.annotation.DateTimeFormat;
import org.springframework.format.annotation.DateTimeFormat.ISO;
import org.springframework.hateoas.RepresentationModel;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@SpringBootApplication
public class DemoApplication {

    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }

    @RestController
    public class DemoController {
        @GetMapping(value = "/search/findByDate", produces = MediaType.APPLICATION_JSON_VALUE)
        public ResponseEntity<Void> findByDate(@RequestParam(name = "from", required = true) @DateTimeFormat(iso = ISO.DATE_TIME) ZonedDateTime from,
                                               @RequestParam(name = "to", required = false) @DateTimeFormat(iso = ISO.DATE_TIME) ZonedDateTime to) {
            return ResponseEntity.ok().build();
        }

        @GetMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE)
        public ResponseEntity<RootResource> getRoot() {
            return ResponseEntity.ok(new RootResource());
        }
    }

    public class RootResource extends RepresentationModel<RootResource> {
        public RootResource() {
            add(linkTo(methodOn(DemoController.class).findByDate(null, null)).withRel("hsf").expand(ZonedDateTime.now(), ZonedDateTime.now().plusMonths(1)));
        }

    }

}

Setting both parameters to required = true renders the link correctly.

[bclozel]

Here's an additional comment from the gitter conversation that triggered this issue:

As i see it there is an issue in UriTemplate.expand where the UriBuilder is constructed. It needs the resolved queryParameters to be able to encode them correctly.
Somehow non mandatory query parameters are not part of the list and are therefor not encoded.

[pax95]

Seems to me it is actually a Spring hateoas issue since only required parameters are filtered https://github.com/spring-projects/spring-hateoas/blob/main/src/main/java/org/springframework/hateoas/UriTemplate.java#L262
If you have a mix of required and not required parameters encoding is not applied consistently.

[bclozel]

You're right, this looks like a duplicate of #1485.

I'm closing this issue for now, feel free to reopen it if Spring HATEOAS reporters find that the root cause belongs to Spring Framework. Thanks!

[odrotbohm]

Just a quick analysis. It looks like in Spring Framework, there's a conceptual difference between expanding a URI template and adding parameters via UriComponentsBuilder.queryParam(…). The latter doesn't URI encode the values given and is used for optional request parameters, whereas required ones are applied by expanding the template. Taking this back to the team, too.

[odrotbohm]

This should be fixed with the fix for #1485. Would you mind giving the 1.4 snapshots a try?

[pax95]

@odrotbohm sure will try next week when I'm back at work

[pax95]

@odrotbohm Tested on 1.4 snapshot and now the encoding is applied as expected. Thanks for looking into this.