Skip to content

Commit b8b31ff

Browse files
sbrannensbrannen
committed
Reject multiple @⁠HttpExchange declarations in MVC and WebFlux
This commit updates the RequestMappingHandlerMapping implementations in Spring MVC and Spring WebFlux so that multiple @⁠HttpExchange declarations on the same element are rejected. Closes gh-32049
1 parent c5c77b9 commit b8b31ff

File tree

4 files changed

+129
-6
lines changed

4 files changed

+129
-6
lines changed

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,9 +200,13 @@ private RequestMappingInfo createRequestMappingInfo(AnnotatedElement element) {
200200
return createRequestMappingInfo(requestMappings.get(0).annotation, customCondition);
201201
}
202202

203-
HttpExchange httpExchange = AnnotatedElementUtils.findMergedAnnotation(element, HttpExchange.class);
204-
if (httpExchange != null) {
205-
return createRequestMappingInfo(httpExchange, customCondition);
203+
List<AnnotationDescriptor<HttpExchange>> httpExchanges = getAnnotationDescriptors(
204+
mergedAnnotations, HttpExchange.class);
205+
if (!httpExchanges.isEmpty()) {
206+
Assert.state(httpExchanges.size() == 1,
207+
() -> "Multiple @HttpExchange annotations found on %s, but only one is allowed: %s"
208+
.formatted(element, httpExchanges));
209+
return createRequestMappingInfo(httpExchanges.get(0).annotation, customCondition);
206210
}
207211

208212
return null;

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,12 @@
4848
import org.springframework.web.reactive.result.method.RequestMappingInfo;
4949
import org.springframework.web.service.annotation.HttpExchange;
5050
import org.springframework.web.service.annotation.PostExchange;
51+
import org.springframework.web.service.annotation.PutExchange;
5152
import org.springframework.web.util.pattern.PathPattern;
5253
import org.springframework.web.util.pattern.PathPatternParser;
5354

5455
import static org.assertj.core.api.Assertions.assertThat;
56+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
5557
import static org.mockito.Mockito.mock;
5658

5759
/**
@@ -154,6 +156,38 @@ void patchMapping() {
154156
assertComposedAnnotationMapping(RequestMethod.PATCH);
155157
}
156158

159+
@Test // gh-32049
160+
void httpExchangeWithMultipleAnnotationsAtClassLevel() throws NoSuchMethodException {
161+
this.handlerMapping.afterPropertiesSet();
162+
163+
Class<?> controllerClass = MultipleClassLevelAnnotationsHttpExchangeController.class;
164+
Method method = controllerClass.getDeclaredMethod("post");
165+
166+
assertThatIllegalStateException()
167+
.isThrownBy(() -> this.handlerMapping.getMappingForMethod(method, controllerClass))
168+
.withMessageContainingAll(
169+
"Multiple @HttpExchange annotations found on " + controllerClass,
170+
"@" + HttpExchange.class.getName(),
171+
"@" + ExtraHttpExchange.class.getName()
172+
);
173+
}
174+
175+
@Test // gh-32049
176+
void httpExchangeWithMultipleAnnotationsAtMethodLevel() throws NoSuchMethodException {
177+
this.handlerMapping.afterPropertiesSet();
178+
179+
Class<?> controllerClass = MultipleMethodLevelAnnotationsHttpExchangeController.class;
180+
Method method = controllerClass.getDeclaredMethod("post");
181+
182+
assertThatIllegalStateException()
183+
.isThrownBy(() -> this.handlerMapping.getMappingForMethod(method, controllerClass))
184+
.withMessageContainingAll(
185+
"Multiple @HttpExchange annotations found on " + method,
186+
"@" + PostExchange.class.getName(),
187+
"@" + PutExchange.class.getName()
188+
);
189+
}
190+
157191
@SuppressWarnings("DataFlowIssue")
158192
@Test
159193
void httpExchangeWithDefaultValues() throws NoSuchMethodException {
@@ -313,4 +347,27 @@ public void defaultValuesExchange() {}
313347
public void customValuesExchange(){}
314348
}
315349

350+
@HttpExchange("/exchange")
351+
@ExtraHttpExchange
352+
static class MultipleClassLevelAnnotationsHttpExchangeController {
353+
354+
@PostExchange("/post")
355+
void post() {}
356+
}
357+
358+
359+
static class MultipleMethodLevelAnnotationsHttpExchangeController {
360+
361+
@PostExchange("/post")
362+
@PutExchange("/post")
363+
void post() {}
364+
}
365+
366+
367+
@HttpExchange
368+
@Target(ElementType.TYPE)
369+
@Retention(RetentionPolicy.RUNTIME)
370+
@interface ExtraHttpExchange {
371+
}
372+
316373
}

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,13 @@ private RequestMappingInfo createRequestMappingInfo(AnnotatedElement element) {
360360
return createRequestMappingInfo(requestMappings.get(0).annotation, customCondition);
361361
}
362362

363-
HttpExchange httpExchange = AnnotatedElementUtils.findMergedAnnotation(element, HttpExchange.class);
364-
if (httpExchange != null) {
365-
return createRequestMappingInfo(httpExchange, customCondition);
363+
List<AnnotationDescriptor<HttpExchange>> httpExchanges = getAnnotationDescriptors(
364+
mergedAnnotations, HttpExchange.class);
365+
if (!httpExchanges.isEmpty()) {
366+
Assert.state(httpExchanges.size() == 1,
367+
() -> "Multiple @HttpExchange annotations found on %s, but only one is allowed: %s"
368+
.formatted(element, httpExchanges));
369+
return createRequestMappingInfo(httpExchanges.get(0).annotation, customCondition);
366370
}
367371

368372
return null;

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.springframework.web.method.HandlerTypePredicate;
4949
import org.springframework.web.service.annotation.HttpExchange;
5050
import org.springframework.web.service.annotation.PostExchange;
51+
import org.springframework.web.service.annotation.PutExchange;
5152
import org.springframework.web.servlet.handler.PathPatternsParameterizedTest;
5253
import org.springframework.web.servlet.mvc.condition.ConsumesRequestCondition;
5354
import org.springframework.web.servlet.mvc.condition.MediaTypeExpression;
@@ -58,6 +59,7 @@
5859
import org.springframework.web.util.pattern.PathPatternParser;
5960

6061
import static org.assertj.core.api.Assertions.assertThat;
62+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
6163
import static org.mockito.Mockito.mock;
6264

6365
/**
@@ -276,6 +278,38 @@ void patchMapping() {
276278
assertComposedAnnotationMapping(RequestMethod.PATCH);
277279
}
278280

281+
@Test // gh-32049
282+
void httpExchangeWithMultipleAnnotationsAtClassLevel() throws NoSuchMethodException {
283+
RequestMappingHandlerMapping mapping = createMapping();
284+
285+
Class<?> controllerClass = MultipleClassLevelAnnotationsHttpExchangeController.class;
286+
Method method = controllerClass.getDeclaredMethod("post");
287+
288+
assertThatIllegalStateException()
289+
.isThrownBy(() -> mapping.getMappingForMethod(method, controllerClass))
290+
.withMessageContainingAll(
291+
"Multiple @HttpExchange annotations found on " + controllerClass,
292+
"@" + HttpExchange.class.getName(),
293+
"@" + ExtraHttpExchange.class.getName()
294+
);
295+
}
296+
297+
@Test // gh-32049
298+
void httpExchangeWithMultipleAnnotationsAtMethodLevel() throws NoSuchMethodException {
299+
RequestMappingHandlerMapping mapping = createMapping();
300+
301+
Class<?> controllerClass = MultipleMethodLevelAnnotationsHttpExchangeController.class;
302+
Method method = controllerClass.getDeclaredMethod("post");
303+
304+
assertThatIllegalStateException()
305+
.isThrownBy(() -> mapping.getMappingForMethod(method, controllerClass))
306+
.withMessageContainingAll(
307+
"Multiple @HttpExchange annotations found on " + method,
308+
"@" + PostExchange.class.getName(),
309+
"@" + PutExchange.class.getName()
310+
);
311+
}
312+
279313
@SuppressWarnings("DataFlowIssue")
280314
@Test
281315
void httpExchangeWithDefaultValues() throws NoSuchMethodException {
@@ -437,6 +471,30 @@ public void customValuesExchange(){}
437471
}
438472

439473

474+
@HttpExchange("/exchange")
475+
@ExtraHttpExchange
476+
static class MultipleClassLevelAnnotationsHttpExchangeController {
477+
478+
@PostExchange("/post")
479+
void post() {}
480+
}
481+
482+
483+
static class MultipleMethodLevelAnnotationsHttpExchangeController {
484+
485+
@PostExchange("/post")
486+
@PutExchange("/post")
487+
void post() {}
488+
}
489+
490+
491+
@HttpExchange
492+
@Target(ElementType.TYPE)
493+
@Retention(RetentionPolicy.RUNTIME)
494+
@interface ExtraHttpExchange {
495+
}
496+
497+
440498
private static class Foo {
441499
}
442500

0 commit comments

Comments
 (0)