From a7f97a16699c5b1a428b99d40474f74e05d69dbb Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 18 Jul 2018 14:41:43 +0200 Subject: [PATCH] Avoid null signals when resolving handler arguments Prior to this commit, resolving an argument for a WebFlux controller that's missing from the request and not required by the handler would throw a NullPointerException in some cases. This involves the conversion of the parameter (a `String` parameter type might not trigger this behavior) and sending a `null` within a reactive stream, which is illegal per the RS spec. We now rely on a `Mono.justOrEmpty()` to handle those specific cases. Issue: SPR-17050 --- .../AbstractNamedValueArgumentResolver.java | 4 +-- ...questParamMethodArgumentResolverTests.java | 33 ++++++++++++------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java index 094535faec9..79565cce4f1 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java @@ -101,13 +101,13 @@ public abstract class AbstractNamedValueArgumentResolver extends HandlerMethodAr Model model = bindingContext.getModel(); return resolveName(resolvedName.toString(), nestedParameter, exchange) - .map(arg -> { + .flatMap(arg -> { if ("".equals(arg) && namedValueInfo.defaultValue != null) { arg = resolveStringValue(namedValueInfo.defaultValue); } arg = applyConversion(arg, namedValueInfo, parameter, bindingContext, exchange); handleResolvedValue(arg, namedValueInfo.name, parameter, model, exchange); - return arg; + return Mono.justOrEmpty(arg); }) .switchIfEmpty(getDefaultValue( namedValueInfo, parameter, bindingContext, model, exchange)); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java index f98c8be6e17..2a210b3fc0a 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -133,14 +133,14 @@ public class RequestParamMethodArgumentResolverTests { } @Test - public void resolveWithQueryString() throws Exception { + public void resolveWithQueryString() { MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class); MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path?name=foo")); assertEquals("foo", resolve(param, exchange)); } @Test - public void resolveStringArray() throws Exception { + public void resolveStringArray() { MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(String[].class); MockServerHttpRequest request = MockServerHttpRequest.get("/path?name=foo&name=bar").build(); Object result = resolve(param, MockServerWebExchange.from(request)); @@ -149,13 +149,21 @@ public class RequestParamMethodArgumentResolverTests { } @Test - public void resolveDefaultValue() throws Exception { + public void resolveDefaultValue() { MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class); assertEquals("bar", resolve(param, MockServerWebExchange.from(MockServerHttpRequest.get("/")))); } + @Test // SPR-17050 + public void resolveAndConvertNullValue() { + MethodParameter param = this.testMethod + .annot(requestParam().notRequired()) + .arg(Integer.class); + assertNull(resolve(param, MockServerWebExchange.from(MockServerHttpRequest.get("/?nullParam=")))); + } + @Test - public void missingRequestParam() throws Exception { + public void missingRequestParam() { MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(String[].class); @@ -168,7 +176,7 @@ public class RequestParamMethodArgumentResolverTests { } @Test - public void resolveSimpleTypeParam() throws Exception { + public void resolveSimpleTypeParam() { MockServerHttpRequest request = MockServerHttpRequest.get("/path?stringNotAnnot=plainValue").build(); ServerWebExchange exchange = MockServerWebExchange.from(request); MethodParameter param = this.testMethod.annotNotPresent(RequestParam.class).arg(String.class); @@ -177,13 +185,13 @@ public class RequestParamMethodArgumentResolverTests { } @Test // SPR-8561 - public void resolveSimpleTypeParamToNull() throws Exception { + public void resolveSimpleTypeParamToNull() { MethodParameter param = this.testMethod.annotNotPresent(RequestParam.class).arg(String.class); assertNull(resolve(param, MockServerWebExchange.from(MockServerHttpRequest.get("/")))); } @Test // SPR-10180 - public void resolveEmptyValueToDefault() throws Exception { + public void resolveEmptyValueToDefault() { ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path?name=")); MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class); Object result = resolve(param, exchange); @@ -191,21 +199,21 @@ public class RequestParamMethodArgumentResolverTests { } @Test - public void resolveEmptyValueWithoutDefault() throws Exception { + public void resolveEmptyValueWithoutDefault() { MethodParameter param = this.testMethod.annotNotPresent(RequestParam.class).arg(String.class); MockServerHttpRequest request = MockServerHttpRequest.get("/path?stringNotAnnot=").build(); assertEquals("", resolve(param, MockServerWebExchange.from(request))); } @Test - public void resolveEmptyValueRequiredWithoutDefault() throws Exception { + public void resolveEmptyValueRequiredWithoutDefault() { MethodParameter param = this.testMethod.annot(requestParam()).arg(String.class); MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path?name=")); assertEquals("", resolve(param, exchange)); } @Test - public void resolveOptionalParamValue() throws Exception { + public void resolveOptionalParamValue() { ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); MethodParameter param = this.testMethod.arg(forClassWithGenerics(Optional.class, Integer.class)); Object result = resolve(param, exchange); @@ -237,7 +245,8 @@ public class RequestParamMethodArgumentResolverTests { @RequestParam("name") String paramRequired, @RequestParam(name = "name", required = false) String paramNotRequired, @RequestParam("name") Optional paramOptional, - @RequestParam Mono paramMono) { + @RequestParam Mono paramMono, + @RequestParam(required = false) Integer nullParam) { } }