Browse Source

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
pull/1653/merge
Brian Clozel 7 years ago
parent
commit
a7f97a1669
  1. 4
      spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java
  2. 33
      spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java

4
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java

@ -101,13 +101,13 @@ public abstract class AbstractNamedValueArgumentResolver extends HandlerMethodAr @@ -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));

33
spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverTests.java

@ -1,5 +1,5 @@ @@ -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 { @@ -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 { @@ -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 { @@ -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 { @@ -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 { @@ -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 { @@ -237,7 +245,8 @@ public class RequestParamMethodArgumentResolverTests {
@RequestParam("name") String paramRequired,
@RequestParam(name = "name", required = false) String paramNotRequired,
@RequestParam("name") Optional<Integer> paramOptional,
@RequestParam Mono<String> paramMono) {
@RequestParam Mono<String> paramMono,
@RequestParam(required = false) Integer nullParam) {
}
}

Loading…
Cancel
Save