From 569df6eeccf374d66b316616058c343908eb95af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Tue, 14 Feb 2023 14:07:41 +0100 Subject: [PATCH] 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 --- .../bind/MethodArgumentNotValidException.java | 34 +++++++- .../ModelAttributeMethodProcessor.java | 20 ++++- ...odelAttributeMethodProcessorKotlinTests.kt | 87 +++++++++++++++++++ 3 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 spring-web/src/test/kotlin/org/springframework/web/method/annotation/ModelAttributeMethodProcessorKotlinTests.kt diff --git a/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java b/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java index 93454f588f37..46290aa206b5 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java +++ b/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.web.bind; +import java.lang.reflect.Executable; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -42,13 +43,18 @@ * * @author Rossen Stoyanchev * @author Juergen Hoeller + * @author Sebastien Deleuze * @since 3.1 */ @SuppressWarnings("serial") public class MethodArgumentNotValidException extends BindException implements ErrorResponse { + @Nullable private final MethodParameter parameter; + @Nullable + private final Executable executable; + private final ProblemDetail body; @@ -60,6 +66,19 @@ public class MethodArgumentNotValidException extends BindException implements Er public MethodArgumentNotValidException(MethodParameter parameter, BindingResult bindingResult) { super(bindingResult); this.parameter = parameter; + this.executable = null; + this.body = ProblemDetail.forStatusAndDetail(getStatusCode(), "Invalid request content."); + } + + /** + * Constructor for {@link MethodArgumentNotValidException}. + * @param executable the executable that failed validation + * @param bindingResult the results of the validation + */ + public MethodArgumentNotValidException(Executable executable, BindingResult bindingResult) { + super(bindingResult); + this.parameter = null; + this.executable = executable; this.body = ProblemDetail.forStatusAndDetail(getStatusCode(), "Invalid request content."); } @@ -83,9 +102,16 @@ public final MethodParameter getParameter() { @Override public String getMessage() { - StringBuilder sb = new StringBuilder("Validation failed for argument [") - .append(this.parameter.getParameterIndex()).append("] in ") - .append(this.parameter.getExecutable().toGenericString()); + StringBuilder sb = new StringBuilder("Validation failed "); + if (this.parameter != null) { + sb.append("for argument [") + .append(this.parameter.getParameterIndex()).append("] in ") + .append(this.parameter.getExecutable().toGenericString()); + } + else { + sb.append("in ") + .append(this.executable.toGenericString()); + } BindingResult bindingResult = getBindingResult(); if (bindingResult.getErrorCount() > 1) { sb.append(" with ").append(bindingResult.getErrorCount()).append(" errors"); diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java index 2e3d0efd8b75..9f8de78c336f 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,6 +36,7 @@ import org.springframework.beans.BeanInstantiationException; import org.springframework.beans.BeanUtils; import org.springframework.beans.TypeMismatchException; +import org.springframework.core.KotlinDetector; import org.springframework.core.MethodParameter; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; @@ -47,6 +48,7 @@ import org.springframework.validation.BindException; import org.springframework.validation.BindingResult; import org.springframework.validation.Errors; +import org.springframework.validation.ObjectError; import org.springframework.validation.SmartValidator; import org.springframework.validation.Validator; import org.springframework.validation.annotation.ValidationAnnotationUtils; @@ -329,7 +331,21 @@ public Object getTarget() { throw new MethodArgumentNotValidException(parameter, result); } - return BeanUtils.instantiateClass(ctor, args); + try { + return BeanUtils.instantiateClass(ctor, args); + } + catch (BeanInstantiationException ex) { + Throwable cause = ex.getCause(); + if (KotlinDetector.isKotlinType(ctor.getDeclaringClass()) && cause instanceof NullPointerException) { + BindingResult result = binder.getBindingResult(); + ObjectError error = new ObjectError(ctor.getName(), cause.getMessage()); + result.addError(error); + throw new MethodArgumentNotValidException(ctor, result); + } + else { + throw ex; + } + } } /** diff --git a/spring-web/src/test/kotlin/org/springframework/web/method/annotation/ModelAttributeMethodProcessorKotlinTests.kt b/spring-web/src/test/kotlin/org/springframework/web/method/annotation/ModelAttributeMethodProcessorKotlinTests.kt new file mode 100644 index 000000000000..c080e370da37 --- /dev/null +++ b/spring-web/src/test/kotlin/org/springframework/web/method/annotation/ModelAttributeMethodProcessorKotlinTests.kt @@ -0,0 +1,87 @@ +/* + * Copyright 2002-2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.method.annotation + +import org.assertj.core.api.Assertions +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.mockito.ArgumentMatchers.* +import org.mockito.BDDMockito +import org.mockito.Mockito +import org.springframework.core.MethodParameter +import org.springframework.core.annotation.SynthesizingMethodParameter +import org.springframework.web.bind.MethodArgumentNotValidException +import org.springframework.web.bind.support.WebDataBinderFactory +import org.springframework.web.bind.support.WebRequestDataBinder +import org.springframework.web.context.request.ServletWebRequest +import org.springframework.web.method.support.ModelAndViewContainer +import org.springframework.web.testfixture.servlet.MockHttpServletRequest +import kotlin.annotation.AnnotationTarget.* + +/** + * Kotlin test fixture for [ModelAttributeMethodProcessor]. + * + * @author Sebastien Deleuze + */ +class ModelAttributeMethodProcessorKotlinTests { + + private lateinit var container: ModelAndViewContainer + + private lateinit var processor: ModelAttributeMethodProcessor + + private lateinit var param: MethodParameter + + + @BeforeEach + fun setup() { + container = ModelAndViewContainer() + processor = ModelAttributeMethodProcessor(false) + var method = ModelAttributeHandler::class.java.getDeclaredMethod("test",Param::class.java) + param = SynthesizingMethodParameter(method, 0) + } + + @Test + fun resolveArgumentWithValue() { + val mockRequest = MockHttpServletRequest().apply { addParameter("a", "b") } + val requestWithParam = ServletWebRequest(mockRequest) + val factory = Mockito.mock() + BDDMockito.given(factory.createBinder(any(), any(), eq("param"))) + .willAnswer { WebRequestDataBinder(it.getArgument(1)) } + Assertions.assertThat(processor.resolveArgument(this.param, container, requestWithParam, factory)).isEqualTo(Param("b")) + } + + @Test + fun throwMethodArgumentNotValidExceptionWithNull() { + val mockRequest = MockHttpServletRequest().apply { addParameter("a", null) } + val requestWithParam = ServletWebRequest(mockRequest) + val factory = Mockito.mock() + BDDMockito.given(factory.createBinder(any(), any(), eq("param"))) + .willAnswer { WebRequestDataBinder(it.getArgument(1)) } + Assertions.assertThatThrownBy { + processor.resolveArgument(this.param, container, requestWithParam, factory) + }.isInstanceOf(MethodArgumentNotValidException::class.java) + .hasMessageContaining("parameter a") + } + + private data class Param(val a: String) + + @Suppress("UNUSED_PARAMETER") + private class ModelAttributeHandler { + fun test(param: Param) { } + } + +}