Skip to content
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

DataBinder throws StringIndexOutOfBoundsException for map property without nested property path #34043

Open
n0rthdev opened this issue Dec 6, 2024 · 9 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@n0rthdev
Copy link

n0rthdev commented Dec 6, 2024

Since upgrading to Spring 6.2. we have the Issue that maps in form-data requests are no longer working.
After switching the DTO to using String keys, we are now getting an IndexOutOfRange in DataVinder.createMap
e48cbc5#diff-1f22c41307a3ddcec8f1bc7a237d3b77ac7564883f4c663402993affac8a5756R1071

the issue seems to be the +2 in String nestedPath = name.substring(0, endIdx + 2); this works if the name is something like: mapname[91].myparam but not if the map is just key and value then having name mapname[91]

The tests for this method do not cover my case so I am wondering if this my usecase might no longer be supported?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 6, 2024
@simonbasle
Copy link
Contributor

Hi @n0rthdev, can you provide a concrete example (ideally in the form of a simple unit test) of a map binding that used to work in 6.1 but doesn't in 6.2? I'm especially interested in the structure of the object you store as the map's value.

@simonbasle simonbasle added status: waiting-for-feedback We need additional information before we can continue in: core Issues in core modules (aop, beans, core, context, expression) labels Dec 16, 2024
@n0rthdev
Copy link
Author

Hi @simonbasle,
I will build a testcase. However, the issue we are facing is when the value isn't any custom object, but just a String. (might also be the case for any simple types) because then there is no nested path.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 16, 2024
@n0rthdev
Copy link
Author

n0rthdev commented Dec 16, 2024

@simonbasle

Inside of DataBinderConstructTests

	@Test
	void mapStringBinding() {
		MapValueResolver valueResolver = new MapValueResolver(Map.of(
				"stringMap[a]", "value1",
				"stringMap[b]", "value2",
				"stringMap['c']", "value3"));

		DataBinder binder = initDataBinder(MapStringClass.class);
		binder.construct(valueResolver);

		MapStringClass stringClass = getTarget(binder);
		Map<String, String> map = stringClass.stringMap();

		assertThat(map).hasSize(3);
		assertThat(map.get("a")).isEqualTo("value1");
		assertThat(map.get("b")).isEqualTo("value2");
		assertThat(map.get("c")).isEqualTo("value3");
	}
	
	private record MapStringClass(Map<String, String> stringMap) {
	}

The stacktrace my test is producing.

begin 0, end 13, length 12
java.lang.StringIndexOutOfBoundsException: begin 0, end 13, length 12
	at java.base/java.lang.String.checkBoundsBeginEnd(String.java:4606)
	at java.base/java.lang.String.substring(String.java:2709)
	at org.springframework.validation.DataBinder.createMap(DataBinder.java:1083)
	at org.springframework.validation.DataBinder.createObject(DataBinder.java:964)
	at org.springframework.validation.DataBinder.construct(DataBinder.java:909)
	at org.springframework.validation.DataBinderConstructTests.mapStringBinding(DataBinderConstructTests.java:151)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

@simonbasle
Copy link
Contributor

Yeah I understand how there could be a StringIndexOutOfBoundsException in 6.2, but this test also fails in 6.1 where the result Map contains null values.

@n0rthdev
Copy link
Author

Hmmmm. Our usecase was working in 6.1, However, we are not calling this directly so it might be that Spring 6.1 might have used different logic to parse our data structure. I can take a look if this is of value for you. Are you planning of implementing support for these kind of maps for the databinder or would you consider my testcase a use outside of the specification?

@simonbasle
Copy link
Contributor

simonbasle commented Dec 17, 2024

@n0rthdev yeah if you can provide more insight / an example closer to your actual DTO structure that would be valuable. You also mentioned a change in the DTO to use string keys?

Feel free to also investigate the issue further as well. I'm not sure yet if your particular use case is within what's supposed to be supported.

@simonbasle simonbasle removed the status: feedback-provided Feedback has been provided label Dec 17, 2024
@jochenchrist
Copy link

jochenchrist commented Dec 19, 2024

Same issue here, this prevents us from upgrading to Spring Boot 3.4.0.

Here is a minimal example that runs with Spring Boot 3.3.5, but fails with Spring Boot 3.4.0:

package com.example.spring_web_key_value_demo

import org.junit.jupiter.api.Test
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
import org.springframework.boot.test.context.SpringBootTest
import org.springframework.stereotype.Controller
import org.springframework.test.web.servlet.MockMvc
import org.springframework.test.web.servlet.post
import org.springframework.web.bind.annotation.ModelAttribute
import org.springframework.web.bind.annotation.PostMapping
import org.springframework.web.bind.annotation.ResponseBody

@SpringBootTest
@AutoConfigureMockMvc
class SpringWebKeyValueTests(
  @Autowired val mockMvc: MockMvc,
) {

  @Test
  fun postKeyValue() {
    mockMvc.post("/spring-web-key-value") {
      param("someMap[a]", "valueA")
    }.andExpect {
      status { isOk() }
      content { string("valueA") }
    }
  }

}


@Controller
class SpringWebKeyValueController {
  @PostMapping("/spring-web-key-value")
  @ResponseBody
  fun postKeyValue(@ModelAttribute("payload") payload: PayloadWithMap): String {
    println(payload)
    return payload.someMap.get("a")!!
  }
}

data class PayloadWithMap(
  val someMap: Map<String, String?> = mutableMapOf(),
)

Exception:

jakarta.servlet.ServletException: Request processing failed: java.lang.StringIndexOutOfBoundsException: Range [0, 11) out of bounds for length 10

	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1022)
	at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:914)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:590)
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:885)
	at org.springframework.test.web.servlet.TestDispatcherServlet.service(TestDispatcherServlet.java:72)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:658)
	at org.springframework.mock.web.MockFilterChain$ServletFilterProxy.doFilter(MockFilterChain.java:165)
	at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:132)
	at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	at org.springframework.test.web.servlet.setup.MockMvcFilterDecorator.doFilter(MockMvcFilterDecorator.java:162)
	at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:132)
	at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	at org.springframework.test.web.servlet.setup.MockMvcFilterDecorator.doFilter(MockMvcFilterDecorator.java:162)
	at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:132)
	at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
	at org.springframework.test.web.servlet.setup.MockMvcFilterDecorator.doFilter(MockMvcFilterDecorator.java:162)
	at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:132)
	at org.springframework.test.web.servlet.MockMvc.perform(MockMvc.java:201)
	at org.springframework.test.web.servlet.MockHttpServletRequestDsl.perform$spring_test(MockHttpServletRequestDsl.kt:222)
	at org.springframework.test.web.servlet.MockMvcExtensionsKt.post(MockMvcExtensions.kt:56)
	at com.example.spring_web_key_value_demo.SpringWebKeyValueTests.postKeyValue(SpringWebKeyValueTests.kt:24)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: java.lang.StringIndexOutOfBoundsException: Range [0, 11) out of bounds for length 10
	at java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:55)
	at java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:52)
	at java.base/jdk.internal.util.Preconditions$4.apply(Preconditions.java:213)
	at java.base/jdk.internal.util.Preconditions$4.apply(Preconditions.java:210)
	at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:98)
	at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckFromToIndex(Preconditions.java:112)
	at java.base/jdk.internal.util.Preconditions.checkFromToIndex(Preconditions.java:349)
	at java.base/java.lang.String.checkBoundsBeginEnd(String.java:4865)
	at java.base/java.lang.String.substring(String.java:2834)
	at org.springframework.validation.DataBinder.createMap(DataBinder.java:1083)
	at org.springframework.validation.DataBinder.createObject(DataBinder.java:964)
	at org.springframework.validation.DataBinder.construct(DataBinder.java:909)
	at org.springframework.web.bind.ServletRequestDataBinder.construct(ServletRequestDataBinder.java:116)
	at org.springframework.web.servlet.mvc.method.annotation.ServletModelAttributeMethodProcessor.constructAttribute(ServletModelAttributeMethodProcessor.java:157)
	at org.springframework.web.method.annotation.ModelAttributeMethodProcessor.resolveArgument(ModelAttributeMethodProcessor.java:148)
	at org.springframework.web.method.support.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:122)
	at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:224)
	at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:178)
	at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:118)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:986)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:891)
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1088)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:978)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1014)

@sbrannen sbrannen assigned sbrannen and unassigned sbrannen Dec 19, 2024
@sbrannen sbrannen added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 19, 2024
@sbrannen sbrannen added this to the 6.2.2 milestone Dec 19, 2024
@sbrannen
Copy link
Member

Thanks for the reproducer, @jochenchrist.

I converted that from Kotlin to Java as follows.

import java.util.Map;

import example.SpringWebKeyValueTests.SpringWebKeyValueController;
import org.junit.jupiter.api.Test;

import org.springframework.test.context.junit.jupiter.web.SpringJUnitWebConfig;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.web.bind.annotation.ModelAttribute;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.context.WebApplicationContext;

import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup;

@SpringJUnitWebConfig(SpringWebKeyValueController.class)
class SpringWebKeyValueTests {

	@Test
	void postKeyValue(WebApplicationContext wac) throws Exception {
		MockMvc mockMvc = webAppContextSetup(wac).build();
		mockMvc.perform(post("/spring-web-key-value").param("someMap[a]", "valueA"))
			.andExpect(status().isOk())
			.andExpect(content().string("valueA"));
	}

	record PayloadWithMap(Map<String, String> someMap) {}

	@RestController
	static class SpringWebKeyValueController {

		@PostMapping("/spring-web-key-value")
		String postKeyValue(@ModelAttribute("payload") PayloadWithMap payload) {
			return payload.someMap.get("a");
		}
	}

}

In DataBinder.createMap(...), if I replace:

String nestedPath = name.substring(0, endIdx + 2);

... with:

String nestedPath = ((name.length() > endIdx + 1) ? name.substring(0, endIdx + 2) : "");

... that allows SpringWebKeyValueTests to pass.

I have not verified if this is the ideal solution, but the team will look into it.

@sbrannen
Copy link
Member

This is closely related to:

And the two should be looked at in conjunction.

@sbrannen sbrannen changed the title DataBinder Support for Key Value Map DataBinder throws StringIndexOutOfBoundsException for map property without nested property path Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

5 participants