Constructor binding fails for simple types List/Map/Array and for nested container combinations

[VladM-N1]

Hello,

It's first time I'm reporting bug here.

I've found bunch of bugs in recent spring version (6.2.2) related to query parameters. I can see that few of them were already fixed (#34121)

Here is 3 more that I've found:

• param[]=value -> java.lang.NumberFormatException: For input string: ""
  

• param[0]=123 -> java.lang.IllegalStateException: No primary or single unique constructor found for class java.lang.Integer
  

• param[key]=123 -> java.lang.IllegalStateException: No primary or single unique constructor found for class java.lang.Integer
  

Thanks for contributing to project and best regards

Code to reproduce:
GH project: https://github.com/VladM-N1/spring-622-bugs/blob/main/src/test/java/org/example/queryparamsbugs/BrokenQueryParamsTest.java

As code:

package org.example.queryparamsbugs;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.web.reactive.server.WebTestClient;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;
import reactor.core.publisher.Mono;

import java.util.List;
import java.util.Map;

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
class BrokenQueryParamsTest {

    @Autowired
    private WebTestClient webTestClient;

    @Test
    void unindexedStringArray() {
        final var expected = new ArrayDto(
            List.of("asd"),
            null
        );
       webTestClient.get()
           .uri("/array?stringList[]=asd")
           .exchange()
           .expectStatus()
           .is2xxSuccessful()
           .expectBody(ArrayDto.class)
           .isEqualTo(expected);
    }

    @Test
    void indexedIntArray() {
        final var expected = new ArrayDto(
            List.of("asd"),
            List.of(123)
        );
        webTestClient.get()
            .uri("/array?stringList[0]=asd&intList[0]=123")
            .exchange()
            .expectStatus()
            .is2xxSuccessful()
            .expectBody(ArrayDto.class)
            .isEqualTo(expected);
    }

    @Test
    void mapWithIntValues() {
        final var expected = new MapDto(
            Map.of(
                "key1", 123
            )
        );
        webTestClient.get()
            .uri("/map?intMap[key1]=123")
            .exchange()
            .expectStatus()
            .is2xxSuccessful()
            .expectBody(MapDto.class)
            .isEqualTo(expected);
    }
}

@RestController
class TestController {

    @GetMapping("/array")
    public Mono<ArrayDto> array(ArrayDto queryParams) {
        return Mono.just(queryParams);
    }

    @GetMapping("/map")
    public Mono<MapDto> array(MapDto queryParams) {
        return Mono.just(queryParams);
    }
}

record ArrayDto(
    List<String> stringList,
    List<Integer> intList
) {}

record MapDto(
    Map<String, Integer> intMap
) {}

[andrew-pirkl]

I found a similar issue with a set nested in a map:

@RestController
class HelloController {
    @GetMapping("/test")
    fun index(filterExample: FilterExample): String {
        return filterExample.toString()
    }
}

data class FilterExample(@RequestParam(required = false) var testMap: Map<String,Set<String>>? = null)

error is Caused by: java.lang.IllegalStateException: No primary or single unique constructor found for interface java.util.Set

an example project is here: https://github.com/apirkl/demoDataBinding

[remeio]

I add a test case:

@Test  // gh-34205
void createBinderViaConstructorWithChooseConstructor() {
	request.addParameter("Some-Int-Array[0]", "1");

	ServletRequestDataBinder binder = new ExtendedServletRequestDataBinder(null);
	binder.setTargetType(ResolvableType.forClass(DataBean.class));
	binder.setNameResolver(new BindParamNameResolver());
	assertThatThrownBy(() -> binder.construct(request))
			.isInstanceOf(IllegalStateException.class)
			.hasMessage("No primary or single unique constructor found for class java.lang.Integer");
}
private record DataBean(String name, int age, @BindParam("Some-Int-Array") Integer[] someIntArray) {
}

For X[] someXArray, when X can't find primary or single unique constructor, It will throws IllegalStateException.

[sbrannen]

@VladM-N1 and @apirkl, are you claiming that these use cases were supported in previous versions of Spring Framework?

If so, with which version(s) of the Spring Framework did those use cases work?

---

@apirkl, have you tested with Spring Framework 6.2.2?

[VladM-N1]

Hi @sbrannen
Yes it was working fine, not sure about version, but It stopped working for me after update to spring-boot 3.4.1, 3.3.x seems to be working fine
From my testing, 6.1.1 seems to not have this issues

Also one of the cases with databinding covered here, but only for ServletDataBinding
https://github.com/spring-projects/spring-framework/blob/main/spring-test/src/test/java/org/springframework/test/web/servlet/samples/spr/ServletRequestDataBinderIntegrationTests.java

edit:
Also I've updated example repo using kotlin (I've found issue originally in kotlin project, part of the tests started to fail):
https://github.com/VladM-N1/spring-622-bugs/tree/kotlin-dto
You can change version in build.gradle.kts to check

plugins {
  kotlin("jvm") version "1.9.25"
  kotlin("plugin.spring") version "1.9.25"
  id("org.springframework.boot") version "3.3.0" // works
//  id("org.springframework.boot") version "3.4.1" // does not works
  id("io.spring.dependency-management") version "1.1.7"
}

[andrew-pirkl]

@sbrannen Thanks for taking a look! Yep if you check the example project it's forcing the spring version in gradle.properties and I confirmed it is using 6.2.2. Also yeah I'd expect it to be supported, the issue comes from the way #32426 was implemented.

It was working correctly using id("org.springframework.boot") version "3.3.7" so that would have been 6.1.16. If you downgrade the version in the example project then the test included in it will pass.

The issue is the DataBinder in createMap is trying to use createObject when the type does not have a default constructor

spring-framework/spring-context/src/main/java/org/springframework/validation/DataBinder.java

Line 1063 in 673e2b0

map.put(key, (V) createObject(elementType, nestedPath, valueResolver));

[rstoyanchev]

In 6.1 DataBinder began to support constructor binding (#26721). It did exist before that, but it was not encapsulated in DataBinder and it was more limited, e.g. only top level constructor args with type conversion.

In 6.2 DataBinder was further enhanced to support map/list/array constructor args (#32426). However, that only covers m map/list/array Object value that requires constructor invocation. It does not yet support map/list/array of simple types as the tests above point out, and I'm going to schedule this for a fix.

What I don't see is how this could have worked in 6.1.x where we simply didn't have such support. @VladM-N1, I tried your sample with 3.3.0 but it doesn't work, which is expected because the parameter names alone (e.g. stringList, intList) cannot be used to get the request values, so I get:

Response body
Expected :ArrayDto[stringList=[asd], intList=null]
Actual   :ArrayDto[stringList=null, intList=null]

I am marking it as a bug for now, but will change to regression if it was indeed possible before.

[VladM-N1]

Hi @rstoyanchev
Thank you for looking at it
My java example seems to have something missing (perhaps default values during object init), please refer to kotlin one: https://github.com/VladM-N1/spring-622-bugs/tree/kotlin-dto
It works well with spring-boot 3.3.0, it even works with spring-boot 2.7.18 (SpringCore version 5.3.31) I believe this feature exists for a very long time
Or as Kotlin Code:

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
internal class BrokenQueryParamsTest {
  @Autowired
  private val webTestClient: WebTestClient? = null

  @Test
  fun unindexedStringArray() {
    val expected = ArrayDto(
      listOf("asd"),
    )
    webTestClient!!.get()
      .uri("/array?stringList[]=asd")
      .exchange()
      .expectStatus()
      .is2xxSuccessful()
      .expectBody(ArrayDto::class.java)
      .isEqualTo(expected)
  }

  @Test
  fun indexedIntArray() {
    val expected = ArrayDto(
      listOf("asd"),
      listOf(123)
    )
    webTestClient!!.get()
      .uri("/array?stringList[0]=asd&intList[0]=123")
      .exchange()
      .expectStatus()
      .is2xxSuccessful()
      .expectBody(ArrayDto::class.java)
      .isEqualTo(expected)
  }

  @Test
  fun mapWithIntValues() {
    val expected = MapDto(
      mapOf(
        "key1" to 123
      )
    )
    webTestClient!!.get()
      .uri("/map?intMap[key1]=123")
      .exchange()
      .expectStatus()
      .is2xxSuccessful()
      .expectBody(MapDto::class.java)
      .isEqualTo(expected)
  }
}

@RestController
internal class TestController {
  @GetMapping("/array")
  fun array(queryParams: ArrayDto): Mono<ArrayDto> {
    return Mono.just(queryParams)
  }

  @GetMapping("/map")
  fun array(queryParams: MapDto): Mono<MapDto> {
    return Mono.just(queryParams)
  }
}

data class ArrayDto(
  var stringList: List<String> = mutableListOf(),
  var intList: List<Int> = mutableListOf(),
)

data class MapDto(
  var intMap: Map<String, Int> = mutableMapOf()
)

[rstoyanchev]

The Kotlin branch uses a data class which is constructed with empty input initially (since we did not support constructor binding with map/list/array). However, the setter binding applied after that succeeds, and the tests pass.

The Java branch uses a record and it can only be initialized through the constructor. As expected, I get failures in the Java branch with both Boot 3.3.0 and 2.7.18.

So this is a regression when using Kotlin data classes, but not for Java records.

[VladM-N1]

Hi @rstoyanchev thanks for verifying this issue.

[VladM-N1]

Hi @rstoyanchev
Thanks for fixing. I had a short look into commit and will it work for queries like list[]=value?
From my POV it will create empty lists, since this part will create empty indexes list

if (StringUtils.hasLength(rawIndex)) {
					int index = Integer.parseInt(rawIndex);
					indexes = (indexes != null ? indexes : new TreeSet<>());
					indexes.add(index);
}

[rstoyanchev]

@VladM-N1, we now have support for List/Map/Array of simple types, as the issue states, in addition to complex objects with nested paths. However, I shouldn't have closed the issue just yet. I meant to look into the [] syntax as well as things like Map<String, List<Integer>>, i.e. nested lists within maps, etc.

[VladM-N1]

Hi @rstoyanchev
I've tested new version, it's working great
Thanks for looking at it