From 2607a22537cdf3ff9acfe5e98c2c24c2cf384964 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Mon, 25 Jan 2016 16:32:22 -0500 Subject: [PATCH] HTTP OPTIONS lists all HTTP methods except TRACE This is in line with the current behavior of HttpServlet that would have been in used with dispatchOptionsRequest on the DispatcherSerlvet set to false (the default prior to 4.3). Issue: SPR-13130 --- .../web/servlet/mvc/AbstractController.java | 10 ++-------- .../mvc/method/RequestMappingInfoHandlerMapping.java | 7 +++++-- .../servlet/resource/ResourceHttpRequestHandler.java | 12 ++++++------ .../mvc/ParameterizableViewControllerTests.java | 2 +- .../RequestMappingInfoHandlerMappingTests.java | 2 +- .../resource/ResourceHttpRequestHandlerTests.java | 2 +- src/asciidoc/web-mvc.adoc | 4 ++-- 7 files changed, 18 insertions(+), 21 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/AbstractController.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/AbstractController.java index 42ce91e6ed..c37e656a7e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/AbstractController.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/AbstractController.java @@ -16,15 +16,11 @@ package org.springframework.web.servlet.mvc; -import java.util.Arrays; -import java.util.List; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; import org.springframework.http.HttpMethod; -import org.springframework.util.ObjectUtils; -import org.springframework.util.StringUtils; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.support.WebContentGenerator; import org.springframework.web.util.WebUtils; @@ -155,10 +151,8 @@ public abstract class AbstractController extends WebContentGenerator implements public ModelAndView handleRequest(HttpServletRequest request, HttpServletResponse response) throws Exception { - String[] supportedMethods = getSupportedMethods(); - if (HttpMethod.OPTIONS.matches(request.getMethod()) && !ObjectUtils.isEmpty(supportedMethods)) { - List value = Arrays.asList(supportedMethods); - response.setHeader("Allow", StringUtils.collectionToCommaDelimitedString(value)); + if (HttpMethod.OPTIONS.matches(request.getMethod())) { + response.setHeader("Allow", getAllowHeader()); return null; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java index 12c830d5ab..1d3b8cd3fe 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java @@ -317,8 +317,11 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe private static Set initAllowedHttpMethods(Set declaredMethods) { Set result = new LinkedHashSet(declaredMethods.size()); if (declaredMethods.isEmpty()) { - result.add(HttpMethod.GET); - result.add(HttpMethod.HEAD); + for (HttpMethod method : HttpMethod.values()) { + if (!HttpMethod.TRACE.equals(method)) { + result.add(method); + } + } } else { boolean hasHead = declaredMethods.contains("HEAD"); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index aac67b358c..8b7d4c3471 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -112,7 +112,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator public ResourceHttpRequestHandler() { - super(HttpMethod.GET.name(), HttpMethod.HEAD.name(), HttpMethod.OPTIONS.name()); + super(HttpMethod.GET.name(), HttpMethod.HEAD.name()); this.resourceResolvers.add(new PathResourceResolver()); } @@ -226,6 +226,11 @@ public class ResourceHttpRequestHandler extends WebContentGenerator public void handleRequest(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + if (HttpMethod.OPTIONS.matches(request.getMethod())) { + response.setHeader("Allow", getAllowHeader()); + return; + } + // Supported methods and required session checkRequest(request); @@ -237,11 +242,6 @@ public class ResourceHttpRequestHandler extends WebContentGenerator return; } - if (HttpMethod.OPTIONS.matches(request.getMethod())) { - response.setHeader("Allow", "GET,HEAD"); - return; - } - // Header phase if (new ServletWebRequest(request, response).checkNotModified(resource.lastModified())) { logger.trace("Resource not modified - returning 304"); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/ParameterizableViewControllerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/ParameterizableViewControllerTests.java index efa34af223..721f8a67a2 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/ParameterizableViewControllerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/ParameterizableViewControllerTests.java @@ -77,7 +77,7 @@ public class ParameterizableViewControllerTests { ModelAndView mav = this.controller.handleRequest(this.request, response); assertNull(mav); - assertEquals("GET,HEAD", response.getHeader("Allow")); + assertEquals("GET,HEAD,OPTIONS", response.getHeader("Allow")); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java index 96a063c48a..e179252010 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java @@ -180,7 +180,7 @@ public class RequestMappingInfoHandlerMappingTests { public void getHandlerHttpOptions() throws Exception { testHttpOptions("/foo", "GET,HEAD"); testHttpOptions("/person/1", "PUT"); - testHttpOptions("/persons", "GET,HEAD"); + testHttpOptions("/persons", "GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS"); testHttpOptions("/something", "PUT,POST"); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java index 6a18748f1d..818bb5cfcc 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java @@ -119,7 +119,7 @@ public class ResourceHttpRequestHandlerTests { this.handler.handleRequest(this.request, this.response); assertEquals(200, this.response.getStatus()); - assertEquals("GET,HEAD", this.response.getHeader("Allow")); + assertEquals("GET,HEAD,OPTIONS", this.response.getHeader("Allow")); } @Test diff --git a/src/asciidoc/web-mvc.adoc b/src/asciidoc/web-mvc.adoc index b9a2e32bfe..cb0c19ac6b 100644 --- a/src/asciidoc/web-mvc.adoc +++ b/src/asciidoc/web-mvc.adoc @@ -1174,8 +1174,8 @@ the number of bytes are counted and the "Content-Length" header set. HTTP OPTIONS request is handled by setting the "Allow" response header to the HTTP methods explicitly declared on all `@RequestMapping` methods with matching URL patterns. When no HTTP methods are explicitly declared the "Allow" header -is set to "GET,HEAD". Therefore it's highly recommended to always explicitly -declare the HTTP method(s) an `@RequestMapping` method is meant to handle. +is set to "GET,HEAD,POST,PUT,PATCH,DELETE,OPTIONS". Ideally always declare the +HTTP method(s) an `@RequestMapping` method is intended to handle. Although not necessary an `@RequestMapping` method can be mapped to and handle either HTTP HEAD or HTTP OPTIONS, or both.