Require explicit path mappings for @RequestMapping methods

Prior to this commit, handler methods in Spring MVC controllers were
not required to provide explicit path mappings via @RequestMapping (or
any of its specializations such as @GetMapping). Such handler methods
were effectively mapped to all paths. Consequently, developers may have
unwittingly mapped all requests to a single handler method.

This commit addresses this by enforcing that @RequestMapping methods
are mapped to an explicit path. Note, however, that this is enforced
after type-level and method-level @RequestMapping information has been
merged.

Developers wishing to map to all paths should now add an explicit path
mapping to "/**" or "**".

Closes gh-22543

spring-web/src/main/java/org/springframework/web/bind/annotation/GetMapping.java

@@ -31,7 +31,6 @@ import org.springframework.core.annotation.AliasFor;
  * <p>Specifically, {@code @GetMapping} is a <em>composed annotation</em> that
  * acts as a shortcut for {@code @RequestMapping(method = RequestMethod.GET)}.
  *
- *
  * @author Sam Brannen
  * @since 4.3
  * @see PostMapping

spring-web/src/main/java/org/springframework/web/bind/annotation/RequestMapping.java

@@ -88,26 +88,33 @@ public @interface RequestMapping {
 
 	/**
 	 * The primary mapping expressed by this annotation.
-	 * <p>This is an alias for {@link #path}. For example
+	 * <p>This is an alias for {@link #path}. For example,
 	 * {@code @RequestMapping("/foo")} is equivalent to
 	 * {@code @RequestMapping(path="/foo")}.
 	 * <p><b>Supported at the type level as well as at the method level!</b>
 	 * When used at the type level, all method-level mappings inherit
 	 * this primary mapping, narrowing it for a specific handler method.
+	 * <p><strong>NOTE</strong>: Each handler method must be mapped to a
+	 * non-empty path, either at the type level, at the method level, or a
+	 * combination of the two. If you wish to map to all paths, please map
+	 * explicitly to {@code "/**"} or {@code "**"}.
 	 */
 	@AliasFor("path")
 	String[] value() default {};
 
 	/**
-	 * The path mapping URIs (e.g. "/myPath.do").
-	 * Ant-style path patterns are also supported (e.g. "/myPath/*.do").
-	 * At the method level, relative paths (e.g. "edit.do") are supported
+	 * The path mapping URIs (e.g. {@code "/myPath.do"}).
+	 * <p>Ant-style path patterns are also supported (e.g. {@code "/myPath/*.do"}).
+	 * At the method level, relative paths (e.g. {@code "edit.do"}) are supported
 	 * within the primary mapping expressed at the type level.
-	 * Path mapping URIs may contain placeholders (e.g. "/${connect}").
+	 * Path mapping URIs may contain placeholders (e.g. <code>"/${connect}"</code>).
 	 * <p><b>Supported at the type level as well as at the method level!</b>
 	 * When used at the type level, all method-level mappings inherit
 	 * this primary mapping, narrowing it for a specific handler method.
-	 * @see org.springframework.web.bind.annotation.ValueConstants#DEFAULT_NONE
+	 * <p><strong>NOTE</strong>: Each handler method must be mapped to a
+	 * non-empty path, either at the type level, at the method level, or a
+	 * combination of the two. If you wish to map to all paths, please map
+	 * explicitly to {@code "/**"} or {@code "**"}.
 	 * @since 4.2
 	 */
 	@AliasFor("value")

spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java

@@ -43,6 +43,7 @@ import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
+import org.springframework.util.StringUtils;
 import org.springframework.web.cors.CorsConfiguration;
 import org.springframework.web.cors.CorsUtils;
 import org.springframework.web.method.HandlerMethod;
@@ -58,6 +59,7 @@ import org.springframework.web.servlet.HandlerMapping;
  * @author Arjen Poutsma
  * @author Rossen Stoyanchev
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 3.1
  * @param <T> the mapping for a {@link HandlerMethod} containing the conditions
  * needed to match the handler method to incoming request.
@@ -587,6 +589,7 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
 			this.readWriteLock.writeLock().lock();
 			try {
 				HandlerMethod handlerMethod = createHandlerMethod(handler, method);
+				assertMappedPathMethodMapping(handlerMethod, mapping);
 				assertUniqueMethodMapping(handlerMethod, mapping);
 				this.mappingLookup.put(mapping, handlerMethod);
 
@@ -613,6 +616,21 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
 			}
 		}
 
+		/**
+		 * Assert that the supplied {@code mapping} maps the supplied {@link HandlerMethod}
+		 * to explicit, non-empty paths.
+		 * @since 5.2
+		 * @see StringUtils#hasText(String)
+		 */
+		private void assertMappedPathMethodMapping(HandlerMethod handlerMethod, T mapping) {
+			if (!getMappingPathPatterns(mapping).stream().allMatch(StringUtils::hasText)) {
+				throw new IllegalStateException(String.format("Missing path mapping. " +
+						"Handler method '%s' in bean '%s' must be mapped to a non-empty path. " +
+						"If you wish to map to all paths, please map explicitly to \"/**\" or \"**\".",
+					handlerMethod, handlerMethod.getBean()));
+			}
+		}
+
 		private void assertUniqueMethodMapping(HandlerMethod newHandlerMethod, T mapping) {
 			HandlerMethod handlerMethod = this.mappingLookup.get(mapping);
 			if (handlerMethod != null && !handlerMethod.equals(newHandlerMethod)) {

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java

@@ -78,7 +78,7 @@ public class RequestMappingInfoHandlerMappingTests {
 
 	private HandlerMethod barMethod;
 
-	private HandlerMethod emptyMethod;
+	private HandlerMethod rootMethod;
 
 
 	@Before
@@ -88,7 +88,7 @@ public class RequestMappingInfoHandlerMappingTests {
 		this.fooMethod = new HandlerMethod(testController, "foo");
 		this.fooParamMethod = new HandlerMethod(testController, "fooParam");
 		this.barMethod = new HandlerMethod(testController, "bar");
-		this.emptyMethod = new HandlerMethod(testController, "empty");
+		this.rootMethod = new HandlerMethod(testController, "root");
 
 		this.handlerMapping = new TestRequestMappingInfoHandlerMapping();
 		this.handlerMapping.registerHandler(testController);
@@ -125,12 +125,12 @@ public class RequestMappingInfoHandlerMappingTests {
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
 		HandlerMethod handlerMethod = getHandler(request);
 
-		assertEquals(this.emptyMethod.getMethod(), handlerMethod.getMethod());
+		assertEquals(this.rootMethod.getMethod(), handlerMethod.getMethod());
 
 		request = new MockHttpServletRequest("GET", "/");
 		handlerMethod = getHandler(request);
 
-		assertEquals(this.emptyMethod.getMethod(), handlerMethod.getMethod());
+		assertEquals(this.rootMethod.getMethod(), handlerMethod.getMethod());
 	}
 
 	@Test
@@ -465,8 +465,8 @@ public class RequestMappingInfoHandlerMappingTests {
 		public void bar() {
 		}
 
-		@RequestMapping(value = "")
-		public void empty() {
+		@RequestMapping("/")
+		public void root() {
 		}
 
 		@RequestMapping(value = "/person/{id}", method = RequestMethod.PUT, consumes="application/xml")

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -228,7 +228,7 @@ public class RequestMappingHandlerMappingTests {
 	@RequestMapping(consumes = MediaType.APPLICATION_JSON_VALUE)
 	static class ComposedAnnotationController {
 
-		@RequestMapping
+		@RequestMapping("/**")
 		public void handle() {
 		}
 
@@ -236,7 +236,7 @@ public class RequestMappingHandlerMappingTests {
 		public void postJson() {
 		}
 
-		@GetMapping(value = "/get", consumes = MediaType.ALL_VALUE)
+		@GetMapping(path = "/get", consumes = MediaType.ALL_VALUE)
 		public void get() {
 		}
 
@@ -266,7 +266,7 @@ public class RequestMappingHandlerMappingTests {
 	@Retention(RetentionPolicy.RUNTIME)
 	@interface PostJson {
 
-		@AliasFor(annotation = RequestMapping.class, attribute = "path")
+		@AliasFor(annotation = RequestMapping.class)
 		String[] value() default {};
 	}
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -67,10 +67,10 @@ import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.beans.factory.config.BeanDefinition;
-import org.springframework.beans.factory.config.PropertyPlaceholderConfigurer;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.beans.propertyeditors.CustomDateEditor;
 import org.springframework.context.annotation.AnnotationConfigUtils;
+import org.springframework.context.support.PropertySourcesPlaceholderConfigurer;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.format.annotation.DateTimeFormat;
@@ -151,11 +151,13 @@ import org.springframework.web.servlet.support.RequestContextUtils;
 import org.springframework.web.servlet.view.AbstractView;
 import org.springframework.web.servlet.view.InternalResourceViewResolver;
 
+import static org.assertj.core.api.Assertions.*;
 import static org.junit.Assert.*;
 
 /**
  * @author Rossen Stoyanchev
  * @author Juergen Hoeller
+ * @author Sam Brannen
  */
 public class ServletAnnotationControllerHandlerMethodTests extends AbstractServletHandlerMethodTests {
 
@@ -251,7 +253,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 	@Test
 	public void defaultExpressionParameters() throws Exception {
 		initServlet(wac -> {
-			RootBeanDefinition ppc = new RootBeanDefinition(PropertyPlaceholderConfigurer.class);
+			RootBeanDefinition ppc = new RootBeanDefinition(PropertySourcesPlaceholderConfigurer.class);
 			ppc.getPropertyValues().add("properties", "myKey=foo");
 			wac.registerBeanDefinition("ppc", ppc);
 		}, DefaultExpressionValueParamController.class);
@@ -787,15 +789,19 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 	}
 
 	@Test
-	public void equivalentMappingsWithSameMethodName() throws Exception {
-		try {
-			initServletWithControllers(ChildController.class);
-			fail("Expected 'method already mapped' error");
-		}
-		catch (BeanCreationException e) {
-			assertTrue(e.getCause() instanceof IllegalStateException);
-			assertTrue(e.getCause().getMessage().contains("Ambiguous mapping"));
-		}
+	public void equivalentMappingsWithSameMethodName() {
+		assertThatThrownBy(() -> initServletWithControllers(ChildController.class))
+			.isInstanceOf(BeanCreationException.class)
+			.hasCauseInstanceOf(IllegalStateException.class)
+			.hasMessageContaining("Ambiguous mapping");
+	}
+
+	@Test
+	public void unmappedPathMapping() {
+		assertThatThrownBy(() -> initServletWithControllers(UnmappedPathController.class))
+			.isInstanceOf(BeanCreationException.class)
+			.hasCauseInstanceOf(IllegalStateException.class)
+			.hasMessageContaining("Missing path mapping");
 	}
 
 	@Test
@@ -1993,7 +1999,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 	@Controller
 	static class ControllerWithEmptyValueMapping {
 
-		@RequestMapping("")
+		@RequestMapping("/**")
 		public void myPath2(HttpServletResponse response) throws IOException {
 			throw new IllegalStateException("test");
 		}
@@ -2012,7 +2018,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 	@Controller
 	private static class ControllerWithErrorThrown {
 
-		@RequestMapping("")
+		@RequestMapping("/**")
 		public void myPath2(HttpServletResponse response) throws IOException {
 			throw new AssertionError("test");
 		}
@@ -2726,6 +2732,15 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		}
 	}
 
+	@Controller
+	@RequestMapping // path intentionally omitted
+	static class UnmappedPathController {
+
+		@GetMapping // path intentionally omitted
+		public void get(@RequestParam(required = false) String id) {
+		}
+	}
+
 	@Target({ElementType.TYPE})
 	@Retention(RetentionPolicy.RUNTIME)
 	@Controller
@@ -3586,7 +3601,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 	@Controller
 	static class HttpHeadersResponseController {
 
-		@RequestMapping(value = "", method = RequestMethod.POST)
+		@RequestMapping(value = "/*", method = RequestMethod.POST)
 		@ResponseStatus(HttpStatus.CREATED)
 		public HttpHeaders create() throws URISyntaxException {
 			HttpHeaders headers = new HttpHeaders();