Skip to content

Commit a7f97a1

Browse files
committed
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
1 parent 5fcfe0f commit a7f97a1

File tree

2 files changed

+23
-14
lines changed

2 files changed

+23
-14
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,13 @@ public Mono<Object> resolveArgument(
101101
Model model = bindingContext.getModel();
102102

103103
return resolveName(resolvedName.toString(), nestedParameter, exchange)
104-
.map(arg -> {
104+
.flatMap(arg -> {
105105
if ("".equals(arg) && namedValueInfo.defaultValue != null) {
106106
arg = resolveStringValue(namedValueInfo.defaultValue);
107107
}
108108
arg = applyConversion(arg, namedValueInfo, parameter, bindingContext, exchange);
109109
handleResolvedValue(arg, namedValueInfo.name, parameter, model, exchange);
110-
return arg;
110+
return Mono.justOrEmpty(arg);
111111
})
112112
.switchIfEmpty(getDefaultValue(
113113
namedValueInfo, parameter, bindingContext, model, exchange));

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

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -133,14 +133,14 @@ public void doesNotSupportReactiveWrapper() {
133133
}
134134

135135
@Test
136-
public void resolveWithQueryString() throws Exception {
136+
public void resolveWithQueryString() {
137137
MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class);
138138
MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path?name=foo"));
139139
assertEquals("foo", resolve(param, exchange));
140140
}
141141

142142
@Test
143-
public void resolveStringArray() throws Exception {
143+
public void resolveStringArray() {
144144
MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(String[].class);
145145
MockServerHttpRequest request = MockServerHttpRequest.get("/path?name=foo&name=bar").build();
146146
Object result = resolve(param, MockServerWebExchange.from(request));
@@ -149,13 +149,21 @@ public void resolveStringArray() throws Exception {
149149
}
150150

151151
@Test
152-
public void resolveDefaultValue() throws Exception {
152+
public void resolveDefaultValue() {
153153
MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class);
154154
assertEquals("bar", resolve(param, MockServerWebExchange.from(MockServerHttpRequest.get("/"))));
155155
}
156156

157+
@Test // SPR-17050
158+
public void resolveAndConvertNullValue() {
159+
MethodParameter param = this.testMethod
160+
.annot(requestParam().notRequired())
161+
.arg(Integer.class);
162+
assertNull(resolve(param, MockServerWebExchange.from(MockServerHttpRequest.get("/?nullParam="))));
163+
}
164+
157165
@Test
158-
public void missingRequestParam() throws Exception {
166+
public void missingRequestParam() {
159167

160168
MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/"));
161169
MethodParameter param = this.testMethod.annotPresent(RequestParam.class).arg(String[].class);
@@ -168,7 +176,7 @@ public void missingRequestParam() throws Exception {
168176
}
169177

170178
@Test
171-
public void resolveSimpleTypeParam() throws Exception {
179+
public void resolveSimpleTypeParam() {
172180
MockServerHttpRequest request = MockServerHttpRequest.get("/path?stringNotAnnot=plainValue").build();
173181
ServerWebExchange exchange = MockServerWebExchange.from(request);
174182
MethodParameter param = this.testMethod.annotNotPresent(RequestParam.class).arg(String.class);
@@ -177,35 +185,35 @@ public void resolveSimpleTypeParam() throws Exception {
177185
}
178186

179187
@Test // SPR-8561
180-
public void resolveSimpleTypeParamToNull() throws Exception {
188+
public void resolveSimpleTypeParamToNull() {
181189
MethodParameter param = this.testMethod.annotNotPresent(RequestParam.class).arg(String.class);
182190
assertNull(resolve(param, MockServerWebExchange.from(MockServerHttpRequest.get("/"))));
183191
}
184192

185193
@Test // SPR-10180
186-
public void resolveEmptyValueToDefault() throws Exception {
194+
public void resolveEmptyValueToDefault() {
187195
ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path?name="));
188196
MethodParameter param = this.testMethod.annot(requestParam().notRequired("bar")).arg(String.class);
189197
Object result = resolve(param, exchange);
190198
assertEquals("bar", result);
191199
}
192200

193201
@Test
194-
public void resolveEmptyValueWithoutDefault() throws Exception {
202+
public void resolveEmptyValueWithoutDefault() {
195203
MethodParameter param = this.testMethod.annotNotPresent(RequestParam.class).arg(String.class);
196204
MockServerHttpRequest request = MockServerHttpRequest.get("/path?stringNotAnnot=").build();
197205
assertEquals("", resolve(param, MockServerWebExchange.from(request)));
198206
}
199207

200208
@Test
201-
public void resolveEmptyValueRequiredWithoutDefault() throws Exception {
209+
public void resolveEmptyValueRequiredWithoutDefault() {
202210
MethodParameter param = this.testMethod.annot(requestParam()).arg(String.class);
203211
MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path?name="));
204212
assertEquals("", resolve(param, exchange));
205213
}
206214

207215
@Test
208-
public void resolveOptionalParamValue() throws Exception {
216+
public void resolveOptionalParamValue() {
209217
ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/"));
210218
MethodParameter param = this.testMethod.arg(forClassWithGenerics(Optional.class, Integer.class));
211219
Object result = resolve(param, exchange);
@@ -237,7 +245,8 @@ public void handle(
237245
@RequestParam("name") String paramRequired,
238246
@RequestParam(name = "name", required = false) String paramNotRequired,
239247
@RequestParam("name") Optional<Integer> paramOptional,
240-
@RequestParam Mono<String> paramMono) {
248+
@RequestParam Mono<String> paramMono,
249+
@RequestParam(required = false) Integer nullParam) {
241250
}
242251

243252
}

0 commit comments

Comments
 (0)