Skip to content

Commit

Permalink
Avoid null signals when resolving handler arguments
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bclozel committed Jul 18, 2018
1 parent 5fcfe0f commit a7f97a1
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ public Mono<Object> resolveArgument(
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));
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -133,14 +133,14 @@ public void doesNotSupportReactiveWrapper() {
}

@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));
Expand All @@ -149,13 +149,21 @@ public void resolveStringArray() throws Exception {
}

@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);
Expand All @@ -168,7 +176,7 @@ public void missingRequestParam() throws Exception {
}

@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);
Expand All @@ -177,35 +185,35 @@ public void resolveSimpleTypeParam() throws Exception {
}

@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);
assertEquals("bar", result);
}

@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);
Expand Down Expand Up @@ -237,7 +245,8 @@ public void handle(
@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) {
}

}

0 comments on commit a7f97a1

Please sign in to comment.