From ff97eafa4f39411a68a1632f7218158287546457 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 10 Nov 2017 10:57:26 -0500 Subject: [PATCH] Fix NPE in ErrorsArgumentResolver Issue: SPR-16187 --- .../ErrorsMethodArgumentResolver.java | 30 ++++++++++--------- .../ErrorsMethodArgumentResolverTests.java | 30 +++++++++++-------- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolver.java index 356da46e85a..fe534a4c6cd 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolver.java @@ -48,7 +48,7 @@ public class ErrorsMethodArgumentResolver extends HandlerMethodArgumentResolverS @Override public boolean supportsParameter(MethodParameter parameter) { - return checkParameterTypeNoReactiveWrapper(parameter, Errors.class::isAssignableFrom); + return checkParameterType(parameter, Errors.class::isAssignableFrom); } @@ -56,25 +56,21 @@ public class ErrorsMethodArgumentResolver extends HandlerMethodArgumentResolverS public Mono resolveArgument( MethodParameter parameter, BindingContext context, ServerWebExchange exchange) { - String name = getModelAttributeName(parameter); - Object errors = context.getModel().asMap().get(BindingResult.MODEL_KEY_PREFIX + name); + Object errors = getErrors(parameter, context); - Mono errorsMono; if (Mono.class.isAssignableFrom(errors.getClass())) { - errorsMono = (Mono) errors; + return ((Mono) errors).cast(Object.class); } else if (Errors.class.isAssignableFrom(errors.getClass())) { - errorsMono = Mono.just(errors); + return Mono.just(errors); } else { throw new IllegalStateException( "Unexpected Errors/BindingResult type: " + errors.getClass().getName()); } - - return errorsMono.cast(Object.class); } - private String getModelAttributeName(MethodParameter parameter) { + private Object getErrors(MethodParameter parameter, BindingContext context) { Assert.isTrue(parameter.getParameterIndex() > 0, "Errors argument must be immediately after a model attribute argument"); @@ -88,11 +84,17 @@ public class ErrorsMethodArgumentResolver extends HandlerMethodArgumentResolverS "Either declare the @ModelAttribute without an async wrapper type or " + "handle a WebExchangeBindException error signal through the async type."); - ModelAttribute ann = parameter.getParameterAnnotation(ModelAttribute.class); - if (ann != null && StringUtils.hasText(ann.value())) { - return ann.value(); - } - return Conventions.getVariableNameForParameter(attributeParam); + ModelAttribute annot = parameter.getParameterAnnotation(ModelAttribute.class); + String name = (annot != null && StringUtils.hasText(annot.value()) ? + annot.value() : Conventions.getVariableNameForParameter(attributeParam)); + + Object errors = context.getModel().asMap().get(BindingResult.MODEL_KEY_PREFIX + name); + + Assert.notNull(errors, "An Errors/BindingResult argument is expected to be declared " + + "immediately after the @ModelAttribute argument to which it applies: " + + parameter.getMethod()); + + return errors; } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolverTests.java index 5493079c880..0e5050a0229 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ErrorsMethodArgumentResolverTests.java @@ -18,7 +18,6 @@ package org.springframework.web.reactive.result.method.annotation; import java.time.Duration; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -40,7 +39,6 @@ import org.springframework.web.reactive.BindingContext; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; /** * Unit tests for {@link ErrorsMethodArgumentResolver}. @@ -69,21 +67,16 @@ public class ErrorsMethodArgumentResolverTests { parameter = this.testMethod.arg(BindingResult.class); assertTrue(this.resolver.supportsParameter(parameter)); - } - @Test - public void doesNotSupport() throws Exception { + parameter = this.testMethod.arg(ResolvableType.forClassWithGenerics(Mono.class, Errors.class)); + assertTrue(this.resolver.supportsParameter(parameter)); - MethodParameter parameter = this.testMethod.arg(String.class); + parameter = this.testMethod.arg(String.class); assertFalse(this.resolver.supportsParameter(parameter)); - - this.expectedException.expectMessage("ErrorsMethodArgumentResolver doesn't support reactive type wrapper"); - parameter = this.testMethod.arg(ResolvableType.forClassWithGenerics(Mono.class, Errors.class)); - this.resolver.supportsParameter(parameter); } @Test - public void resolveErrors() throws Exception { + public void resolve() throws Exception { BindingResult bindingResult = createBindingResult(new Foo(), "foo"); this.bindingContext.getModel().asMap().put(BindingResult.MODEL_KEY_PREFIX + "foo", bindingResult); @@ -101,7 +94,7 @@ public class ErrorsMethodArgumentResolverTests { } @Test - public void resolveErrorsWithMono() throws Exception { + public void resolveWithMono() throws Exception { BindingResult bindingResult = createBindingResult(new Foo(), "foo"); MonoProcessor monoProcessor = MonoProcessor.create(); @@ -116,7 +109,7 @@ public class ErrorsMethodArgumentResolverTests { } @Test - public void resolveErrorsAfterMonoModelAttribute() throws Exception { + public void resolveWithMonoOnBindingResultAndModelAttribute() throws Exception { this.expectedException.expectMessage("An @ModelAttribute and an Errors/BindingResult) arguments " + "cannot both be declared with an async type wrapper."); @@ -126,6 +119,17 @@ public class ErrorsMethodArgumentResolverTests { .block(Duration.ofMillis(5000)); } + @Test // SPR-16187 + public void resolveWithBindingResultNotFound() throws Exception { + + this.expectedException.expectMessage("An Errors/BindingResult argument is expected " + + "to be declared immediately after the @ModelAttribute argument"); + + MethodParameter parameter = this.testMethod.arg(Errors.class); + this.resolver.resolveArgument(parameter, this.bindingContext, this.exchange) + .block(Duration.ofMillis(5000)); + } + @SuppressWarnings("unused") private static class Foo {