Skip to content

Commit 00e80d7

Browse files
OlgaMaciaszekOlgaMaciaszek
committed
Reject not optional null arguments.
1 parent a55207e commit 00e80d7

File tree

14 files changed

+414
-40
lines changed

14 files changed

+414
-40
lines changed

‎framework-docs/modules/ROOT/pages/integration/rest-clients.adoc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,9 @@ method parameters:
977977
`MultiValueMap<String, ?>` with multiple cookies, a `Collection<?>` of values, or an
978978
individual value. Type conversion is supported for non-String values.
979979

980+
The parameters can't be null unless they are set as not required through the annotation,
981+
annotated with `@Nullable` or they are `Optional`.
982+
980983
|===
981984

982985

‎framework-docs/modules/ROOT/pages/rsocket.adoc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1115,7 +1115,8 @@ method parameters:
11151115
| `@Payload`
11161116
| Set the input payload(s) for the request. This can be a concrete value, or any producer
11171117
of values that can be adapted to a Reactive Streams `Publisher` via
1118-
`ReactiveAdapterRegistry`
1118+
`ReactiveAdapterRegistry`. The payload can't be null unless it's set as not required
1119+
through the annotation, annotated with `@Nullable` or it is `Optional`.
11191120

11201121
| `Object`, if followed by `MimeType`
11211122
| The value for a metadata entry in the input payload. This can be any `Object` as long

‎spring-messaging/src/main/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolver.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 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.
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.messaging.rsocket.service;
1818

19+
import java.util.Optional;
20+
1921
import org.springframework.core.MethodParameter;
2022
import org.springframework.core.ParameterizedTypeReference;
2123
import org.springframework.core.ReactiveAdapter;
@@ -29,6 +31,7 @@
2931
* annotated arguments.
3032
*
3133
* @author Rossen Stoyanchev
34+
* @author Olga Maciaszek-Sharma
3235
* @since 6.0
3336
*/
3437
public class PayloadArgumentResolver implements RSocketServiceArgumentResolver {
@@ -54,8 +57,15 @@ public boolean resolve(
5457
return false;
5558
}
5659

60+
parameter = parameter.nestedIfOptional();
61+
62+
if (argument instanceof Optional<?> optionalValue) {
63+
argument = optionalValue.orElse(null);
64+
}
65+
5766
if (argument != null) {
58-
ReactiveAdapter reactiveAdapter = this.reactiveAdapterRegistry.getAdapter(parameter.getParameterType());
67+
ReactiveAdapter reactiveAdapter = this.reactiveAdapterRegistry
68+
.getAdapter(parameter.getNestedParameterType());
5969
if (reactiveAdapter == null) {
6070
requestValues.setPayloadValue(argument);
6171
}
@@ -70,7 +80,10 @@ public boolean resolve(
7080
reactiveAdapter.toPublisher(argument),
7181
ParameterizedTypeReference.forType(nestedParameter.getNestedGenericParameterType()));
7282
}
83+
return true;
7384
}
85+
boolean required = (annot == null || annot.required()) && !parameter.isOptional();
86+
Assert.isTrue(!required, () -> "Missing payload");
7487

7588
return true;
7689
}

‎spring-messaging/src/test/java/org/springframework/messaging/rsocket/service/PayloadArgumentResolverTests.java

Lines changed: 97 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.messaging.rsocket.service;
1818

19+
import java.util.Optional;
20+
1921
import io.reactivex.rxjava3.core.Completable;
2022
import io.reactivex.rxjava3.core.Single;
2123
import org.junit.jupiter.api.Test;
@@ -25,6 +27,7 @@
2527
import org.springframework.core.MethodParameter;
2628
import org.springframework.core.ParameterizedTypeReference;
2729
import org.springframework.core.ReactiveAdapterRegistry;
30+
import org.springframework.lang.Nullable;
2831
import org.springframework.messaging.handler.annotation.Payload;
2932

3033
import static org.assertj.core.api.Assertions.assertThat;
@@ -34,7 +37,9 @@
3437
* Tests for {@link PayloadArgumentResolver}.
3538
*
3639
* @author Rossen Stoyanchev
40+
* @author Olga Maciaszek-Sharma
3741
*/
42+
@SuppressWarnings("DataFlowIssue")
3843
class PayloadArgumentResolverTests extends RSocketServiceArgumentResolverTestSupport {
3944

4045
@Override
@@ -47,20 +52,15 @@ void stringPayload() {
4752
String payload = "payloadValue";
4853
boolean resolved = execute(payload, initMethodParameter(Service.class, "execute", 0));
4954

50-
assertThat(resolved).isTrue();
51-
assertThat(getRequestValues().getPayloadValue()).isEqualTo(payload);
52-
assertThat(getRequestValues().getPayload()).isNull();
55+
assertPayload(resolved, payload);
5356
}
5457

5558
@Test
5659
void monoPayload() {
5760
Mono<String> payloadMono = Mono.just("payloadValue");
5861
boolean resolved = execute(payloadMono, initMethodParameter(Service.class, "executeMono", 0));
5962

60-
assertThat(resolved).isTrue();
61-
assertThat(getRequestValues().getPayloadValue()).isNull();
62-
assertThat(getRequestValues().getPayload()).isSameAs(payloadMono);
63-
assertThat(getRequestValues().getPayloadElementType()).isEqualTo(new ParameterizedTypeReference<String>() {});
63+
assertPayloadMono(resolved, payloadMono);
6464
}
6565

6666
@Test
@@ -92,31 +92,116 @@ void completable() {
9292
}
9393

9494
@Test
95-
void notRequestBody() {
95+
void notPayload() {
9696
MethodParameter parameter = initMethodParameter(Service.class, "executeNotAnnotated", 0);
9797
boolean resolved = execute("value", parameter);
9898

9999
assertThat(resolved).isFalse();
100100
}
101101

102102
@Test
103-
void ignoreNull() {
104-
boolean resolved = execute(null, initMethodParameter(Service.class, "execute", 0));
103+
void nullPayload() {
104+
assertThatIllegalArgumentException()
105+
.isThrownBy(() -> execute(null, initMethodParameter(Service.class, "execute", 0)))
106+
.withMessage("Missing payload");
107+
108+
assertThatIllegalArgumentException()
109+
.isThrownBy(() -> execute(null, initMethodParameter(Service.class, "executeMono", 0)))
110+
.withMessage("Missing payload");
111+
}
112+
113+
@Test
114+
void nullPayloadWithNullable() {
115+
boolean resolved = execute(null, initMethodParameter(Service.class, "executeNullable", 0));
116+
assertNullValues(resolved);
117+
118+
boolean resolvedMono = execute(null, initMethodParameter(Service.class, "executeNullableMono", 0));
119+
assertNullValues(resolvedMono);
120+
}
121+
122+
@Test
123+
void nullPayloadWithNotRequired() {
124+
boolean resolved = execute(null, initMethodParameter(Service.class, "executeNotRequired", 0));
125+
assertNullValues(resolved);
126+
127+
boolean resolvedMono = execute(null, initMethodParameter(Service.class, "executeNotRequiredMono", 0));
128+
assertNullValues(resolvedMono);
129+
}
130+
131+
@Test
132+
void nullPayloadWithOptional() {
133+
boolean resolved = execute(null, initMethodParameter(Service.class, "executeOptional", 0));
134+
assertNullValues(resolved);
135+
136+
boolean resolvedMono = execute(null, initMethodParameter(Service.class, "executeOptionalMono", 0));
137+
assertNullValues(resolvedMono);
138+
}
139+
140+
@Test
141+
void emptyPayloadWithOptional() {
142+
boolean resolved = execute(Optional.empty(), initMethodParameter(Service.class, "executeOptional", 0));
143+
assertNullValues(resolved);
144+
145+
boolean resolvedMono = execute(Optional.empty(), initMethodParameter(Service.class, "executeOptionalMono", 0));
146+
assertNullValues(resolvedMono);
147+
}
148+
149+
@Test
150+
void optionalStringPayload() {
151+
String payload = "payloadValue";
152+
boolean resolved = execute(Optional.of(payload), initMethodParameter(Service.class, "executeOptional", 0));
153+
154+
assertPayload(resolved, payload);
155+
}
156+
157+
@Test
158+
void optionalMonoPayload() {
159+
Mono<String> payloadMono = Mono.just("payloadValue");
160+
boolean resolved = execute(Optional.of(payloadMono), initMethodParameter(Service.class, "executeOptionalMono", 0));
161+
162+
assertPayloadMono(resolved, payloadMono);
163+
}
164+
165+
166+
private void assertPayload(boolean resolved, String payload) {
167+
assertThat(resolved).isTrue();
168+
assertThat(getRequestValues().getPayloadValue()).isEqualTo(payload);
169+
assertThat(getRequestValues().getPayload()).isNull();
170+
}
171+
172+
private void assertPayloadMono(boolean resolved, Mono<String> payloadMono) {
173+
assertThat(resolved).isTrue();
174+
assertThat(getRequestValues().getPayloadValue()).isNull();
175+
assertThat(getRequestValues().getPayload()).isSameAs(payloadMono);
176+
assertThat(getRequestValues().getPayloadElementType()).isEqualTo(new ParameterizedTypeReference<String>() { });
177+
}
105178

179+
private void assertNullValues(boolean resolved) {
106180
assertThat(resolved).isTrue();
107181
assertThat(getRequestValues().getPayloadValue()).isNull();
108182
assertThat(getRequestValues().getPayload()).isNull();
109183
assertThat(getRequestValues().getPayloadElementType()).isNull();
110184
}
111185

112-
113-
@SuppressWarnings("unused")
186+
@SuppressWarnings({"unused", "OptionalUsedAsFieldOrParameterType"})
114187
private interface Service {
115188

116189
void execute(@Payload String body);
117190

191+
void executeNotRequired(@Payload(required = false) String body);
192+
193+
void executeNullable(@Nullable @Payload String body);
194+
118195
void executeMono(@Payload Mono<String> body);
119196

197+
void executeOptional(@Payload Optional<String> body);
198+
199+
void executeNullableMono(@Nullable @Payload Mono<String> body);
200+
201+
void executeNotRequiredMono(@Payload(required = false) Mono<String> body);
202+
203+
void executeOptionalMono(@Payload Optional<Mono<String>> body);
204+
120205
void executeSingle(@Payload Single<String> body);
121206

122207
void executeMonoVoid(@Payload Mono<Void> body);

‎spring-web/src/main/java/org/springframework/web/service/invoker/AbstractNamedValueArgumentResolver.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
* request header, path variable, cookie, and others.
4040
*
4141
* @author Rossen Stoyanchev
42+
* @author Olga Maciaszek-Sharma
4243
* @since 6.0
4344
*/
4445
public abstract class AbstractNamedValueArgumentResolver implements HttpServiceArgumentResolver {
@@ -145,7 +146,7 @@ private NamedValueInfo updateNamedValueInfo(MethodParameter parameter, NamedValu
145146
.formatted(parameter.getNestedParameterType().getName()));
146147
}
147148
}
148-
boolean required = (info.required && !parameter.getParameterType().equals(Optional.class));
149+
boolean required = (info.required && !parameter.isOptional());
149150
String defaultValue = (ValueConstants.DEFAULT_NONE.equals(info.defaultValue) ? null : info.defaultValue);
150151
return info.update(name, required, defaultValue);
151152
}

‎spring-web/src/main/java/org/springframework/web/service/invoker/HttpMethodArgumentResolver.java

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 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.
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.web.service.invoker;
1818

19+
import java.util.Optional;
20+
1921
import org.apache.commons.logging.Log;
2022
import org.apache.commons.logging.LogFactory;
2123

@@ -41,17 +43,25 @@ public class HttpMethodArgumentResolver implements HttpServiceArgumentResolver {
4143
public boolean resolve(
4244
@Nullable Object argument, MethodParameter parameter, HttpRequestValues.Builder requestValues) {
4345

44-
if (!parameter.getParameterType().equals(HttpMethod.class)) {
46+
parameter = parameter.nestedIfOptional();
47+
48+
if (!parameter.getNestedParameterType().equals(HttpMethod.class)) {
4549
return false;
4650
}
4751

48-
Assert.notNull(argument, "HttpMethod is required");
49-
HttpMethod httpMethod = (HttpMethod) argument;
50-
requestValues.setHttpMethod(httpMethod);
51-
if (logger.isTraceEnabled()) {
52-
logger.trace("Resolved HTTP method to: " + httpMethod.name());
52+
if (argument instanceof Optional<?> optionalValue) {
53+
argument = optionalValue.orElse(null);
5354
}
5455

56+
if(argument != null) {
57+
HttpMethod httpMethod = (HttpMethod) argument;
58+
requestValues.setHttpMethod(httpMethod);
59+
if (logger.isTraceEnabled()) {
60+
logger.trace("Resolved HTTP method to: " + httpMethod.name());
61+
}
62+
return true;
63+
}
64+
Assert.isTrue(parameter.isOptional(), "HttpMethod is required");
5565
return true;
5666
}
5767

‎spring-web/src/main/java/org/springframework/web/service/invoker/RequestBodyArgumentResolver.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 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.
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.web.service.invoker;
1818

19+
import java.util.Optional;
20+
1921
import org.springframework.core.MethodParameter;
2022
import org.springframework.core.ParameterizedTypeReference;
2123
import org.springframework.core.ReactiveAdapter;
@@ -30,6 +32,7 @@
3032
* annotated arguments.
3133
*
3234
* @author Rossen Stoyanchev
35+
* @author Olga Maciaszek-Sharma
3336
* @since 6.0
3437
*/
3538
public class RequestBodyArgumentResolver implements HttpServiceArgumentResolver {
@@ -68,9 +71,16 @@ public boolean resolve(
6871
return false;
6972
}
7073

74+
parameter = parameter.nestedIfOptional();
75+
76+
if (argument instanceof Optional<?> optionalValue) {
77+
argument = optionalValue.orElse(null);
78+
}
79+
7180
if (argument != null) {
7281
if (this.reactiveAdapterRegistry != null) {
73-
ReactiveAdapter adapter = this.reactiveAdapterRegistry.getAdapter(parameter.getParameterType());
82+
ReactiveAdapter adapter = this.reactiveAdapterRegistry
83+
.getAdapter(parameter.getNestedParameterType());
7484
if (adapter != null) {
7585
MethodParameter nestedParameter = parameter.nested();
7686

@@ -93,7 +103,9 @@ public boolean resolve(
93103

94104
// Not a reactive type
95105
requestValues.setBodyValue(argument);
106+
return true;
96107
}
108+
Assert.isTrue(!annot.required() || parameter.isOptional(), "RequestBody is required");
97109

98110
return true;
99111
}

0 commit comments

Comments
 (0)