Skip to content

Commit 569df6e

Browse files
sdeleuzesdeleuze
committed
Refine ModelAttributeMethodProcessor Kotlin exception handling
This commit refines ModelAttributeMethodProcessor Kotlin exception handling in order to throw a proper MethodArgumentNotValidException instead of a NullPointerException when Kotlin null-safety constraints are not fulfilled, translating to an HTTP error with 400 status code (Bad Request) instead of 500 (Internal Server Error). Closes gh-23846
1 parent 979118c commit 569df6e

File tree

3 files changed

+135
-6
lines changed

3 files changed

+135
-6
lines changed

‎spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.web.bind;
1818

19+
import java.lang.reflect.Executable;
1920
import java.util.ArrayList;
2021
import java.util.LinkedHashMap;
2122
import java.util.List;
@@ -42,13 +43,18 @@
4243
*
4344
* @author Rossen Stoyanchev
4445
* @author Juergen Hoeller
46+
* @author Sebastien Deleuze
4547
* @since 3.1
4648
*/
4749
@SuppressWarnings("serial")
4850
public class MethodArgumentNotValidException extends BindException implements ErrorResponse {
4951

52+
@Nullable
5053
private final MethodParameter parameter;
5154

55+
@Nullable
56+
private final Executable executable;
57+
5258
private final ProblemDetail body;
5359

5460

@@ -60,6 +66,19 @@ public class MethodArgumentNotValidException extends BindException implements Er
6066
public MethodArgumentNotValidException(MethodParameter parameter, BindingResult bindingResult) {
6167
super(bindingResult);
6268
this.parameter = parameter;
69+
this.executable = null;
70+
this.body = ProblemDetail.forStatusAndDetail(getStatusCode(), "Invalid request content.");
71+
}
72+
73+
/**
74+
* Constructor for {@link MethodArgumentNotValidException}.
75+
* @param executable the executable that failed validation
76+
* @param bindingResult the results of the validation
77+
*/
78+
public MethodArgumentNotValidException(Executable executable, BindingResult bindingResult) {
79+
super(bindingResult);
80+
this.parameter = null;
81+
this.executable = executable;
6382
this.body = ProblemDetail.forStatusAndDetail(getStatusCode(), "Invalid request content.");
6483
}
6584

@@ -83,9 +102,16 @@ public final MethodParameter getParameter() {
83102

84103
@Override
85104
public String getMessage() {
86-
StringBuilder sb = new StringBuilder("Validation failed for argument [")
87-
.append(this.parameter.getParameterIndex()).append("] in ")
88-
.append(this.parameter.getExecutable().toGenericString());
105+
StringBuilder sb = new StringBuilder("Validation failed ");
106+
if (this.parameter != null) {
107+
sb.append("for argument [")
108+
.append(this.parameter.getParameterIndex()).append("] in ")
109+
.append(this.parameter.getExecutable().toGenericString());
110+
}
111+
else {
112+
sb.append("in ")
113+
.append(this.executable.toGenericString());
114+
}
89115
BindingResult bindingResult = getBindingResult();
90116
if (bindingResult.getErrorCount() > 1) {
91117
sb.append(" with ").append(bindingResult.getErrorCount()).append(" errors");

‎spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -36,6 +36,7 @@
3636
import org.springframework.beans.BeanInstantiationException;
3737
import org.springframework.beans.BeanUtils;
3838
import org.springframework.beans.TypeMismatchException;
39+
import org.springframework.core.KotlinDetector;
3940
import org.springframework.core.MethodParameter;
4041
import org.springframework.http.HttpHeaders;
4142
import org.springframework.http.HttpMethod;
@@ -47,6 +48,7 @@
4748
import org.springframework.validation.BindException;
4849
import org.springframework.validation.BindingResult;
4950
import org.springframework.validation.Errors;
51+
import org.springframework.validation.ObjectError;
5052
import org.springframework.validation.SmartValidator;
5153
import org.springframework.validation.Validator;
5254
import org.springframework.validation.annotation.ValidationAnnotationUtils;
@@ -329,7 +331,21 @@ public Object getTarget() {
329331
throw new MethodArgumentNotValidException(parameter, result);
330332
}
331333

332-
return BeanUtils.instantiateClass(ctor, args);
334+
try {
335+
return BeanUtils.instantiateClass(ctor, args);
336+
}
337+
catch (BeanInstantiationException ex) {
338+
Throwable cause = ex.getCause();
339+
if (KotlinDetector.isKotlinType(ctor.getDeclaringClass()) && cause instanceof NullPointerException) {
340+
BindingResult result = binder.getBindingResult();
341+
ObjectError error = new ObjectError(ctor.getName(), cause.getMessage());
342+
result.addError(error);
343+
throw new MethodArgumentNotValidException(ctor, result);
344+
}
345+
else {
346+
throw ex;
347+
}
348+
}
333349
}
334350

335351
/**

‎spring-web/src/test/kotlin/org/springframework/web/method/annotation/ModelAttributeMethodProcessorKotlinTests.kt

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright 2002-2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.web.method.annotation
18+
19+
import org.assertj.core.api.Assertions
20+
import org.junit.jupiter.api.BeforeEach
21+
import org.junit.jupiter.api.Test
22+
import org.mockito.ArgumentMatchers.*
23+
import org.mockito.BDDMockito
24+
import org.mockito.Mockito
25+
import org.springframework.core.MethodParameter
26+
import org.springframework.core.annotation.SynthesizingMethodParameter
27+
import org.springframework.web.bind.MethodArgumentNotValidException
28+
import org.springframework.web.bind.support.WebDataBinderFactory
29+
import org.springframework.web.bind.support.WebRequestDataBinder
30+
import org.springframework.web.context.request.ServletWebRequest
31+
import org.springframework.web.method.support.ModelAndViewContainer
32+
import org.springframework.web.testfixture.servlet.MockHttpServletRequest
33+
import kotlin.annotation.AnnotationTarget.*
34+
35+
/**
36+
* Kotlin test fixture for [ModelAttributeMethodProcessor].
37+
*
38+
* @author Sebastien Deleuze
39+
*/
40+
class ModelAttributeMethodProcessorKotlinTests {
41+
42+
private lateinit var container: ModelAndViewContainer
43+
44+
private lateinit var processor: ModelAttributeMethodProcessor
45+
46+
private lateinit var param: MethodParameter
47+
48+
49+
@BeforeEach
50+
fun setup() {
51+
container = ModelAndViewContainer()
52+
processor = ModelAttributeMethodProcessor(false)
53+
var method = ModelAttributeHandler::class.java.getDeclaredMethod("test",Param::class.java)
54+
param = SynthesizingMethodParameter(method, 0)
55+
}
56+
57+
@Test
58+
fun resolveArgumentWithValue() {
59+
val mockRequest = MockHttpServletRequest().apply { addParameter("a", "b") }
60+
val requestWithParam = ServletWebRequest(mockRequest)
61+
val factory = Mockito.mock<WebDataBinderFactory>()
62+
BDDMockito.given(factory.createBinder(any(), any(), eq("param")))
63+
.willAnswer { WebRequestDataBinder(it.getArgument(1)) }
64+
Assertions.assertThat(processor.resolveArgument(this.param, container, requestWithParam, factory)).isEqualTo(Param("b"))
65+
}
66+
67+
@Test
68+
fun throwMethodArgumentNotValidExceptionWithNull() {
69+
val mockRequest = MockHttpServletRequest().apply { addParameter("a", null) }
70+
val requestWithParam = ServletWebRequest(mockRequest)
71+
val factory = Mockito.mock<WebDataBinderFactory>()
72+
BDDMockito.given(factory.createBinder(any(), any(), eq("param")))
73+
.willAnswer { WebRequestDataBinder(it.getArgument(1)) }
74+
Assertions.assertThatThrownBy {
75+
processor.resolveArgument(this.param, container, requestWithParam, factory)
76+
}.isInstanceOf(MethodArgumentNotValidException::class.java)
77+
.hasMessageContaining("parameter a")
78+
}
79+
80+
private data class Param(val a: String)
81+
82+
@Suppress("UNUSED_PARAMETER")
83+
private class ModelAttributeHandler {
84+
fun test(param: Param) { }
85+
}
86+
87+
}

0 commit comments

Comments
 (0)