From ccd17dfaea76910152f5931c159277f1a8598cdf Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Sun, 24 Jan 2016 20:05:45 -0500 Subject: [PATCH] Support HTTP OPTIONS Issue: SPR-13130 --- .../web/servlet/DispatcherServlet.java | 2 + .../web/servlet/FrameworkServlet.java | 10 +-- .../ServletAnnotationMappingUtils.java | 8 ++- .../RequestMethodsRequestCondition.java | 27 ++++---- .../RequestMappingInfoHandlerMapping.java | 64 ++++++++++++++++++- .../resource/ResourceHttpRequestHandler.java | 8 ++- .../web/servlet/DispatcherServletTests.java | 3 +- .../ServletAnnotationControllerTests.java | 13 +++- .../RequestMethodsRequestConditionTests.java | 3 +- ...RequestMappingInfoHandlerMappingTests.java | 32 ++++++++++ ...nnotationControllerHandlerMethodTests.java | 15 ++++- .../ResourceHttpRequestHandlerTests.java | 25 ++++++++ 12 files changed, 180 insertions(+), 30 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index 4dab2ac9c19..4db91142324 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -347,6 +347,7 @@ public class DispatcherServlet extends FrameworkServlet { */ public DispatcherServlet() { super(); + this.setDispatchOptionsRequest(true); } /** @@ -390,6 +391,7 @@ public class DispatcherServlet extends FrameworkServlet { */ public DispatcherServlet(WebApplicationContext webApplicationContext) { super(webApplicationContext); + this.setDispatchOptionsRequest(true); } /** diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java index 773b7cf75ba..46ad584e284 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -428,9 +428,11 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic /** * Set whether this servlet should dispatch an HTTP OPTIONS request to * the {@link #doService} method. - *

Default is "false", applying {@link javax.servlet.http.HttpServlet}'s - * default behavior (i.e. enumerating all standard HTTP request methods - * as a response to the OPTIONS request). + *

Default in the {@code FrameworkServlet} is "false", applying + * {@link javax.servlet.http.HttpServlet}'s default behavior (i.e.enumerating + * all standard HTTP request methods as a response to the OPTIONS request). + * Note however that as of 4.3 the {@code DispatcherServlet} sets this + * property to "true" by default due to its built-in support for OPTIONS. *

Turn this flag on if you prefer OPTIONS requests to go through the * regular dispatching chain, just like other HTTP requests. This usually * means that your controllers will receive those requests; make sure diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationMappingUtils.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationMappingUtils.java index 6c1288fcd39..331da2050c2 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationMappingUtils.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationMappingUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -30,6 +30,7 @@ import org.springframework.web.util.WebUtils; * * @author Juergen Hoeller * @author Arjen Poutsma + * @author Rossen Stoyanchev * @since 2.5.2 * @deprecated as of Spring 3.2, together with {@link DefaultAnnotationHandlerMapping}, * {@link AnnotationMethodHandlerAdapter}, and {@link AnnotationMethodHandlerExceptionResolver}. @@ -43,11 +44,12 @@ abstract class ServletAnnotationMappingUtils { * @param request the current HTTP request to check */ public static boolean checkRequestMethod(RequestMethod[] methods, HttpServletRequest request) { - if (ObjectUtils.isEmpty(methods)) { + String inputMethod = request.getMethod(); + if (ObjectUtils.isEmpty(methods) && !RequestMethod.OPTIONS.name().equals(inputMethod)) { return true; } for (RequestMethod method : methods) { - if (method.name().equals(request.getMethod())) { + if (method.name().equals(inputMethod)) { return true; } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java index d73d1ab1cf7..0ff3d5337c9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java @@ -99,27 +99,24 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi */ @Override public RequestMethodsRequestCondition getMatchingCondition(HttpServletRequest request) { - if (this.methods.isEmpty()) { - return this; - } RequestMethod requestMethod = getRequestMethod(request); - if (requestMethod != null) { - for (RequestMethod method : this.methods) { - if (method.equals(requestMethod)) { - return new RequestMethodsRequestCondition(method); - } - } - if (isHeadRequest(requestMethod) && getMethods().contains(RequestMethod.GET)) { - return HEAD_CONDITION; + if (requestMethod == null) { + return null; + } + if (this.methods.isEmpty()) { + return (RequestMethod.OPTIONS.equals(requestMethod) ? null : this); + } + for (RequestMethod method : this.methods) { + if (method.equals(requestMethod)) { + return new RequestMethodsRequestCondition(method); } } + if (RequestMethod.HEAD.equals(requestMethod) && getMethods().contains(RequestMethod.GET)) { + return HEAD_CONDITION; + } return null; } - private boolean isHeadRequest(RequestMethod requestMethod) { - return (requestMethod != null && RequestMethod.HEAD.equals(requestMethod)); - } - private RequestMethod getRequestMethod(HttpServletRequest request) { try { return RequestMethod.valueOf(request.getMethod()); 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 5c18540afe7..12c830d5ab4 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -16,6 +16,7 @@ package org.springframework.web.servlet.mvc.method; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; @@ -29,6 +30,8 @@ import java.util.Set; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; import org.springframework.http.InvalidMediaTypeException; import org.springframework.http.MediaType; import org.springframework.util.CollectionUtils; @@ -56,6 +59,19 @@ import org.springframework.web.util.WebUtils; */ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMethodMapping { + private static final Method HTTP_OPTIONS_HANDLE_METHOD; + + static { + try { + HTTP_OPTIONS_HANDLE_METHOD = HttpOptionsHandler.class.getMethod("handle"); + } + catch (NoSuchMethodException ex) { + // Should never happen + throw new IllegalStateException("No handler for HTTP OPTIONS", ex); + } + } + + protected RequestMappingInfoHandlerMapping() { setHandlerMethodMappingNamingStrategy(new RequestMappingInfoHandlerMethodMappingNamingStrategy()); } @@ -200,8 +216,14 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe if (patternMatches.isEmpty()) { return null; } - else if (patternAndMethodMatches.isEmpty() && !allowedMethods.isEmpty()) { - throw new HttpRequestMethodNotSupportedException(request.getMethod(), allowedMethods); + else if (patternAndMethodMatches.isEmpty()) { + if (HttpMethod.OPTIONS.matches(request.getMethod())) { + HttpOptionsHandler handler = new HttpOptionsHandler(allowedMethods); + return new HandlerMethod(handler, HTTP_OPTIONS_HANDLE_METHOD); + } + else if (!allowedMethods.isEmpty()) { + throw new HttpRequestMethodNotSupportedException(request.getMethod(), allowedMethods); + } } Set consumableMediaTypes; @@ -279,4 +301,40 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe return result; } + + /** + * Default handler for HTTP OPTIONS. + */ + private static class HttpOptionsHandler { + + private final HttpHeaders headers = new HttpHeaders(); + + + public HttpOptionsHandler(Set declaredMethods) { + this.headers.setAllow(initAllowedHttpMethods(declaredMethods)); + } + + private static Set initAllowedHttpMethods(Set declaredMethods) { + Set result = new LinkedHashSet(declaredMethods.size()); + if (declaredMethods.isEmpty()) { + result.add(HttpMethod.GET); + result.add(HttpMethod.HEAD); + } + else { + boolean hasHead = declaredMethods.contains("HEAD"); + for (String method : declaredMethods) { + result.add(HttpMethod.valueOf(method)); + if (!hasHead && "GET".equals(method)) { + result.add(HttpMethod.HEAD); + } + } + } + return result; + } + + public HttpHeaders handle() { + return this.headers; + } + } + } 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 8d322fed4ee..aac67b358ca 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 @@ -37,6 +37,7 @@ import org.springframework.beans.factory.InitializingBean; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpRange; import org.springframework.http.MediaType; import org.springframework.http.server.ServletServerHttpRequest; @@ -111,7 +112,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator public ResourceHttpRequestHandler() { - super(METHOD_GET, METHOD_HEAD); + super(HttpMethod.GET.name(), HttpMethod.HEAD.name(), HttpMethod.OPTIONS.name()); this.resourceResolvers.add(new PathResourceResolver()); } @@ -236,6 +237,11 @@ 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/DispatcherServletTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java index d5cb10f78ca..3c5f9632010 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -862,6 +862,7 @@ public class DispatcherServletTests { MockHttpServletRequest request = new MockHttpServletRequest(getServletContext(), "OPTIONS", "/foo"); MockHttpServletResponse response = spy(new MockHttpServletResponse()); DispatcherServlet servlet = new DispatcherServlet(); + servlet.setDispatchOptionsRequest(false); servlet.service(request, response); verify(response, never()).getHeader(anyString()); // SPR-10341 assertThat(response.getHeader("Allow"), equalTo("GET, HEAD, POST, PUT, DELETE, TRACE, OPTIONS, PATCH")); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java index 8c50836c750..69ed0465af7 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -1966,6 +1966,17 @@ public class ServletAnnotationControllerTests { assertEquals("1-2", response.getContentAsString()); } + @Test + public void httpOptions() throws ServletException, IOException { + initServlet(ResponseEntityController.class); + + MockHttpServletRequest request = new MockHttpServletRequest("OPTIONS", "/foo"); + MockHttpServletResponse response = new MockHttpServletResponse(); + servlet.service(request, response); + assertEquals(404, response.getStatus()); + } + + public static class ListEditorRegistrar implements PropertyEditorRegistrar { @Override diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestConditionTests.java index 94dc781511e..cddbd8f356a 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestConditionTests.java @@ -82,12 +82,13 @@ public class RequestMethodsRequestConditionTests { } @Test - public void noDeclaredMethodsMatchesAllMethods() { + public void noDeclaredMethodsMatchesAllMethodsExceptOptions() { RequestCondition condition = new RequestMethodsRequestCondition(); assertNotNull(condition.getMatchingCondition(new MockHttpServletRequest("GET", ""))); assertNotNull(condition.getMatchingCondition(new MockHttpServletRequest("POST", ""))); assertNotNull(condition.getMatchingCondition(new MockHttpServletRequest("HEAD", ""))); + assertNull(condition.getMatchingCondition(new MockHttpServletRequest("OPTIONS", ""))); } @Test 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 5a88d03ceeb..96a063c48a2 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 @@ -33,6 +33,7 @@ import org.junit.Before; import org.junit.Test; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.stereotype.Controller; @@ -44,8 +45,11 @@ import org.springframework.web.bind.UnsatisfiedServletRequestParameterException; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.context.support.StaticWebApplicationContext; import org.springframework.web.method.HandlerMethod; +import org.springframework.web.method.support.InvocableHandlerMethod; +import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.servlet.HandlerExecutionChain; import org.springframework.web.servlet.HandlerInterceptor; import org.springframework.web.servlet.HandlerMapping; @@ -172,6 +176,14 @@ public class RequestMappingInfoHandlerMappingTests { testHttpMediaTypeNotSupportedException("/person/1.json"); } + @Test + public void getHandlerHttpOptions() throws Exception { + testHttpOptions("/foo", "GET,HEAD"); + testHttpOptions("/person/1", "PUT"); + testHttpOptions("/persons", "GET,HEAD"); + testHttpOptions("/something", "PUT,POST"); + } + @Test public void getHandlerTestInvalidContentType() throws Exception { try { @@ -388,6 +400,19 @@ public class RequestMappingInfoHandlerMappingTests { } } + private void testHttpOptions(String requestURI, String allowHeader) throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("OPTIONS", requestURI); + HandlerMethod handlerMethod = getHandler(request); + + ServletWebRequest webRequest = new ServletWebRequest(request); + ModelAndViewContainer mavContainer = new ModelAndViewContainer(); + Object result = new InvocableHandlerMethod(handlerMethod).invokeForRequest(webRequest, mavContainer); + + assertNotNull(result); + assertEquals(HttpHeaders.class, result.getClass()); + assertEquals(allowHeader, ((HttpHeaders) result).getFirst("Allow")); + } + private void testHttpMediaTypeNotAcceptableException(String url) throws Exception { try { MockHttpServletRequest request = new MockHttpServletRequest("GET", url); @@ -468,6 +493,13 @@ public class RequestMappingInfoHandlerMappingTests { public String nonXmlContent() { return ""; } + + @RequestMapping(value = "/something", method = RequestMethod.OPTIONS) + public HttpHeaders fooOptions() { + HttpHeaders headers = new HttpHeaders(); + headers.add("Allow", "PUT,POST"); + return headers; + } } @SuppressWarnings("unused") diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java index a93b70ab39c..19fa225150b 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java @@ -1771,6 +1771,19 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl assertEquals("body", response.getContentAsString()); } + @Test + public void httpOptions() throws ServletException, IOException { + initServletWithControllers(ResponseEntityController.class); + + MockHttpServletRequest request = new MockHttpServletRequest("OPTIONS", "/baz"); + MockHttpServletResponse response = new MockHttpServletResponse(); + getServlet().service(request, response); + + assertEquals(200, response.getStatus()); + assertEquals("GET,HEAD", response.getHeader("Allow")); + assertTrue(response.getContentAsByteArray().length == 0); + } + /* * Controllers @@ -3059,7 +3072,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl return ResponseEntity.notFound().header("MyResponseHeader", "MyValue").build(); } - @RequestMapping("/baz") + @RequestMapping(path = "/baz", method = RequestMethod.GET) public ResponseEntity baz() { return ResponseEntity.ok().header("MyResponseHeader", "MyValue").body("body"); } 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 bfbda11fc90..6a18748f1d3 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 @@ -97,6 +97,31 @@ public class ResourceHttpRequestHandlerTests { assertEquals("h1 { color:red; }", this.response.getContentAsString()); } + @Test + public void getResourceHttpHeader() throws Exception { + this.request.setMethod("HEAD"); + this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css"); + this.handler.handleRequest(this.request, this.response); + + assertEquals(200, this.response.getStatus()); + assertEquals("text/css", this.response.getContentType()); + assertEquals(17, this.response.getContentLength()); + assertEquals("max-age=3600", this.response.getHeader("Cache-Control")); + assertTrue(this.response.containsHeader("Last-Modified")); + assertEquals(this.response.getHeader("Last-Modified"), resourceLastModifiedDate("test/foo.css")); + assertEquals(0, this.response.getContentAsByteArray().length); + } + + @Test + public void getResourceHttpOptions() throws Exception { + this.request.setMethod("OPTIONS"); + this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css"); + this.handler.handleRequest(this.request, this.response); + + assertEquals(200, this.response.getStatus()); + assertEquals("GET,HEAD", this.response.getHeader("Allow")); + } + @Test public void getResourceNoCache() throws Exception { this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "foo.css");