From e6565c600a642af2ebafb459c738cda06c58f493 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Fri, 18 Aug 2023 11:46:00 +0200 Subject: [PATCH] Refine type conversion errors in ModelAttributeMethodProcessor This commit turns TypeMismatchException thrown in ModelAttributeMethodArgumentResolver#createAttribute into proper ServerWebInputException in order get HTTP response with 400 Bad Request status code instead of 500 Internal error. Closes gh-31045 --- .../ModelAttributeMethodArgumentResolver.java | 10 ++- ...lAttributeMethodArgumentResolverTests.java | 12 +++ ...ributeMethodArgumentResolverKotlinTests.kt | 85 +++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverKotlinTests.kt diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java index 25ae88e54f..c59d835f0d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolver.java @@ -26,6 +26,7 @@ import reactor.core.publisher.Mono; import reactor.core.publisher.Sinks; import org.springframework.beans.BeanUtils; +import org.springframework.beans.TypeMismatchException; import org.springframework.context.i18n.LocaleContext; import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.core.MethodParameter; @@ -45,6 +46,7 @@ import org.springframework.web.bind.support.WebExchangeDataBinder; import org.springframework.web.reactive.BindingContext; import org.springframework.web.reactive.result.method.HandlerMethodArgumentResolverSupport; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.ServerWebInputException; /** * Resolve {@code @ModelAttribute} annotated method arguments. @@ -63,6 +65,7 @@ import org.springframework.web.server.ServerWebExchange; * @author Rossen Stoyanchev * @author Juergen Hoeller * @author Sam Brannen + * @author Sebastien Deleuze * @since 5.0 */ public class ModelAttributeMethodArgumentResolver extends HandlerMethodArgumentResolverSupport { @@ -255,7 +258,12 @@ public class ModelAttributeMethodArgumentResolver extends HandlerMethodArgumentR args[i] = (methodParam.getParameterType() == Optional.class ? Optional.empty() : null); } else { - args[i] = binder.convertIfNecessary(value, paramTypes[i], methodParam); + try { + args[i] = binder.convertIfNecessary(value, paramTypes[i], methodParam); + } + catch (TypeMismatchException ex) { + throw new ServerWebInputException("Type mismatch.", methodParam, ex); + } } } return BeanUtils.instantiateClass(ctor, args); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java index d54b842a4f..c3c10578b5 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverTests.java @@ -38,6 +38,7 @@ import org.springframework.web.bind.support.ConfigurableWebBindingInitializer; import org.springframework.web.bind.support.WebExchangeBindException; import org.springframework.web.reactive.BindingContext; import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.ServerWebInputException; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; import org.springframework.web.testfixture.method.ResolvableMethod; import org.springframework.web.testfixture.server.MockServerWebExchange; @@ -50,6 +51,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Rossen Stoyanchev * @author Juergen Hoeller * @author Sam Brannen + * @author Sebastien Deleuze */ class ModelAttributeMethodArgumentResolverTests { @@ -388,6 +390,16 @@ class ModelAttributeMethodArgumentResolverTests { assertThat(model.get(bindingResultKey)).isInstanceOf(BindingResult.class); } + @Test + void bindDataClassError() { + MethodParameter parameter = this.testMethod.annotNotPresent(ModelAttribute.class).arg(DataClass.class); + Mono mono = createResolver().resolveArgument(parameter, this.bindContext, postForm("name=Robert&age=invalid&count=1")); + StepVerifier.create(mono) + .expectNextCount(0) + .expectError(ServerWebInputException.class) + .verify(); + } + // TODO: SPR-15871, SPR-15542 diff --git a/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverKotlinTests.kt b/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverKotlinTests.kt new file mode 100644 index 0000000000..4e98ac85e5 --- /dev/null +++ b/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/ModelAttributeMethodArgumentResolverKotlinTests.kt @@ -0,0 +1,85 @@ +/* + * 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.reactive.result.method.annotation + +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.springframework.core.MethodParameter +import org.springframework.core.ReactiveAdapterRegistry +import org.springframework.http.MediaType +import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean +import org.springframework.web.bind.annotation.ModelAttribute +import org.springframework.web.bind.support.ConfigurableWebBindingInitializer +import org.springframework.web.reactive.BindingContext +import org.springframework.web.server.ServerWebExchange +import org.springframework.web.server.ServerWebInputException +import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest +import org.springframework.web.testfixture.method.ResolvableMethod +import org.springframework.web.testfixture.server.MockServerWebExchange +import reactor.core.publisher.Mono +import reactor.test.StepVerifier + +/** + * Kotlin test fixture for [ModelAttributeMethodArgumentResolver]. + * + * @author Sebastien Deleuze + */ +class ModelAttributeMethodArgumentResolverKotlinTests { + + private val testMethod = ResolvableMethod.on(javaClass).named("handle").build() + + private lateinit var bindContext: BindingContext + + @BeforeEach + fun setup() { + val validator = LocalValidatorFactoryBean() + validator.afterPropertiesSet() + val initializer = ConfigurableWebBindingInitializer() + initializer.validator = validator + this.bindContext = BindingContext(initializer) + } + + @Test + fun bindDataClassError() { + val parameter: MethodParameter = this.testMethod.annotNotPresent(ModelAttribute::class.java).arg(DataClass::class.java) + val mono: Mono = + createResolver().resolveArgument(parameter, this.bindContext, postForm("name=Robert&age=invalid&count=1")) + StepVerifier.create(mono) + .expectNextCount(0) + .expectError(ServerWebInputException::class.java) + .verify() + } + + private fun createResolver(): ModelAttributeMethodArgumentResolver { + return ModelAttributeMethodArgumentResolver(ReactiveAdapterRegistry.getSharedInstance(), false) + } + + private fun postForm(formData: String): ServerWebExchange { + return MockServerWebExchange.from( + MockServerHttpRequest.post("/") + .contentType(MediaType.APPLICATION_FORM_URLENCODED) + .body(formData) + ) + } + + @Suppress("UNUSED_PARAMETER") + private fun handle(dataClassNotAnnotated: DataClass) { + } + + private class DataClass(val name: String, val age: Int, val count: Int) + +}