From 058279bc7ed0466c0106b62c6a3455f1f316cb0c Mon Sep 17 00:00:00 2001
From: Rossen Stoyanchev <rstoyanchev@pivotal.io>
Date: Tue, 21 Jun 2016 13:32:40 -0400
Subject: [PATCH] HEAD mapping has higher priority over other conditions

When comparing multiple matching @RequestMapping's, the HTTP method
condition has the lowest precedence. It's mainly about ensuring an
explicit mapping wins over an implicit (i.e. no method) one.

As of 4.3 HTTP HEAD is handled automatically for controller methods
that match to GET. However an explicit mapping HTTP HEAD allows an
application to take control.

This commit ensures that for HTTP HEAD requests the HTTP method
condition is checked first which means that an explicit HEAD mapping
now trumps all other conditions.

Normally we look for the most specific matching @RequestMapping.
For HTTP HEAD we now look for the most specific match among
@RequestMapping methods with a HEAD mapping first.

Issue: SPR-14383
---
 .../mvc/method/RequestMappingInfo.java        | 12 +++++++++-
 .../mvc/method/RequestMappingInfoTests.java   | 22 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java
index 445f55f5a8..a9cf31bb3e 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java
@@ -19,6 +19,7 @@ package org.springframework.web.servlet.mvc.method;
 import java.util.List;
 import javax.servlet.http.HttpServletRequest;
 
+import org.springframework.http.HttpMethod;
 import org.springframework.util.PathMatcher;
 import org.springframework.util.StringUtils;
 import org.springframework.web.accept.ContentNegotiationManager;
@@ -238,7 +239,15 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
 	 */
 	@Override
 	public int compareTo(RequestMappingInfo other, HttpServletRequest request) {
-		int result = this.patternsCondition.compareTo(other.getPatternsCondition(), request);
+		int result;
+		// Automatic vs explicit HTTP HEAD mapping
+		if (HttpMethod.HEAD.matches(request.getMethod())) {
+			result = this.methodsCondition.compareTo(other.getMethodsCondition(), request);
+			if (result != 0) {
+				return result;
+			}
+		}
+		result = this.patternsCondition.compareTo(other.getPatternsCondition(), request);
 		if (result != 0) {
 			return result;
 		}
@@ -258,6 +267,7 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
 		if (result != 0) {
 			return result;
 		}
+		// Implicit (no method) vs explicit HTTP method mappings
 		result = this.methodsCondition.compareTo(other.getMethodsCondition(), request);
 		if (result != 0) {
 			return result;
diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java
index 879237584a..c36f8c53b7 100644
--- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java
+++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java
@@ -34,6 +34,7 @@ import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.springframework.web.bind.annotation.RequestMethod.GET;
+import static org.springframework.web.bind.annotation.RequestMethod.HEAD;
 import static org.springframework.web.servlet.mvc.method.RequestMappingInfo.paths;
 
 /**
@@ -171,6 +172,27 @@ public class RequestMappingInfoTests {
 		assertEquals(noMethods, list.get(2));
 	}
 
+	@Test // SPR-14383
+	public void compareToWithHttpHeadMapping() {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.setMethod("HEAD");
+		request.addHeader("Accept", "application/json");
+
+		RequestMappingInfo noMethods = paths().build();
+		RequestMappingInfo getMethod = paths().methods(GET).produces("application/json").build();
+		RequestMappingInfo headMethod = paths().methods(HEAD).build();
+
+		Comparator<RequestMappingInfo> comparator = (info, otherInfo) -> info.compareTo(otherInfo, request);
+
+		List<RequestMappingInfo> list = asList(noMethods, getMethod, headMethod);
+		Collections.shuffle(list);
+		Collections.sort(list, comparator);
+
+		assertEquals(headMethod, list.get(0));
+		assertEquals(getMethod, list.get(1));
+		assertEquals(noMethods, list.get(2));
+	}
+
 	@Test
 	public void equals() {
 		RequestMappingInfo info1 = paths("/foo").methods(GET)