Skip to content

Commit

Permalink
Fix regression in WebFlux support for WebDAV methods
Browse files Browse the repository at this point in the history
This commit ensures that WebFlux's RequestMethodsRequestCondition
supports HTTP methods that are not in the RequestMethod enum.

- RequestMethod::resolve is introduced, to convert from a HttpMethod
(name) to enum values.
- RequestMethod::asHttpMethod is introduced, to convert from enum value
to HttpMethod.
- HttpMethod::valueOf replaced Map-based lookup to a switch statement
- Enabled tests that check for WebDAV methods

See gh-27697
Closes gh-29981
  • Loading branch information
poutsma committed Feb 17, 2023
1 parent 1999c78 commit 88e6544
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 43 deletions.
25 changes: 11 additions & 14 deletions spring-web/src/main/java/org/springframework/http/HttpMethod.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
package org.springframework.http;

import java.io.Serializable;
import java.util.Arrays;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;

import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
Expand Down Expand Up @@ -88,9 +84,6 @@ public final class HttpMethod implements Comparable<HttpMethod>, Serializable {

private static final HttpMethod[] values = new HttpMethod[] { GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE };

private static final Map<String, HttpMethod> mappings = Arrays.stream(values)
.collect(Collectors.toUnmodifiableMap(HttpMethod::name, Function.identity()));


private final String name;

Expand Down Expand Up @@ -121,13 +114,17 @@ public static HttpMethod[] values() {
*/
public static HttpMethod valueOf(String method) {
Assert.notNull(method, "Method must not be null");
HttpMethod result = mappings.get(method);
if (result != null) {
return result;
}
else {
return new HttpMethod(method);
}
return switch (method) {
case "GET" -> GET;
case "HEAD" -> HEAD;
case "POST" -> POST;
case "PUT" -> PUT;
case "PATCH" -> PATCH;
case "DELETE" -> DELETE;
case "OPTIONS" -> OPTIONS;
case "TRACE" -> TRACE;
default -> new HttpMethod(method);
};
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

package org.springframework.web.bind.annotation;

import org.springframework.http.HttpMethod;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;

/**
* Enumeration of HTTP request methods. Intended for use with the
* {@link RequestMapping#method()} attribute of the {@link RequestMapping} annotation.
Expand All @@ -34,6 +38,62 @@
*/
public enum RequestMethod {

GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE
GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, TRACE;


/**
* Resolve the given method value to an {@code RequestMethod} enum value.
* Returns {@code null} if {@code method} has no corresponding value.
* @param method the method value as a String
* @return the corresponding {@code RequestMethod}, or {@code null} if not found
* @since 6.0.6
*/
@Nullable
public static RequestMethod resolve(String method) {
Assert.notNull(method, "Method must not be null");
return switch (method) {
case "GET" -> GET;
case "HEAD" -> HEAD;
case "POST" -> POST;
case "PUT" -> PUT;
case "PATCH" -> PATCH;
case "DELETE" -> DELETE;
case "OPTIONS" -> OPTIONS;
case "TRACE" -> TRACE;
default -> null;
};
}

/**
* Resolve the given {@link HttpMethod} to a {@code RequestMethod} enum value.
* Returns {@code null} if {@code httpMethod} has no corresponding value.
* @param httpMethod the http method object
* @return the corresponding {@code RequestMethod}, or {@code null} if not found
* @since 6.0.6
*/
@Nullable
public static RequestMethod resolve(HttpMethod httpMethod) {
Assert.notNull(httpMethod, "HttpMethod must not be null");
return resolve(httpMethod.name());
}


/**
* Return the {@link HttpMethod} corresponding to this {@code RequestMethod}.
* @return the http method for this request method
* @since 6.0.6
*/
public HttpMethod asHttpMethod() {
return switch (this) {
case GET -> HttpMethod.GET;
case HEAD -> HttpMethod.HEAD;
case POST -> HttpMethod.POST;
case PUT -> HttpMethod.PUT;
case PATCH -> HttpMethod.PATCH;
case DELETE -> HttpMethod.DELETE;
case OPTIONS -> HttpMethod.OPTIONS;
case TRACE -> HttpMethod.TRACE;
};
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright 2002-2023 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.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.web.bind.annotation;

import org.junit.jupiter.api.Test;

import org.springframework.http.HttpMethod;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Arjen Poutsma
*/
class RequestMethodTests {

@Test
void resolveString() {
String[] methods = new String[]{"GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "OPTIONS", "TRACE"};
for (String httpMethod : methods) {
RequestMethod requestMethod = RequestMethod.resolve(httpMethod);
assertThat(requestMethod).isNotNull();
assertThat(requestMethod.name()).isEqualTo(httpMethod);
}
assertThat(RequestMethod.resolve("PROPFIND")).isNull();
}

@Test
void resolveHttpMethod() {
for (HttpMethod httpMethod : HttpMethod.values()) {
RequestMethod requestMethod = RequestMethod.resolve(httpMethod);
assertThat(requestMethod).isNotNull();
assertThat(requestMethod.name()).isEqualTo(httpMethod.name());
}
assertThat(RequestMethod.resolve(HttpMethod.valueOf("PROPFIND"))).isNull();
}

@Test
void asHttpMethod() {
for (RequestMethod requestMethod : RequestMethod.values()) {
HttpMethod httpMethod = requestMethod.asHttpMethod();
assertThat(httpMethod).isNotNull();
assertThat(httpMethod.name()).isEqualTo(requestMethod.name());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi

static {
requestMethodConditionCache = CollectionUtils.newHashMap(RequestMethod.values().length);
for (RequestMethod method : RequestMethod.values()) {
requestMethodConditionCache.put(
HttpMethod.valueOf(method.name()), new RequestMethodsRequestCondition(method));
for (RequestMethod requestMethod : RequestMethod.values()) {
requestMethodConditionCache.put(requestMethod.asHttpMethod(),
new RequestMethodsRequestCondition(requestMethod));
}
}

Expand Down Expand Up @@ -150,16 +150,15 @@ private RequestMethodsRequestCondition matchPreFlight(ServerHttpRequest request)
}

@Nullable
private RequestMethodsRequestCondition matchRequestMethod(@Nullable HttpMethod httpMethod) {
if (httpMethod == null) {
return null;
}
RequestMethod requestMethod = RequestMethod.valueOf(httpMethod.name());
if (getMethods().contains(requestMethod)) {
return requestMethodConditionCache.get(httpMethod);
}
if (requestMethod.equals(RequestMethod.HEAD) && getMethods().contains(RequestMethod.GET)) {
return requestMethodConditionCache.get(HttpMethod.GET);
private RequestMethodsRequestCondition matchRequestMethod(HttpMethod httpMethod) {
RequestMethod requestMethod = RequestMethod.resolve(httpMethod);
if (requestMethod != null) {
if (getMethods().contains(requestMethod)) {
return requestMethodConditionCache.get(httpMethod);
}
if (requestMethod.equals(RequestMethod.HEAD) && getMethods().contains(RequestMethod.GET)) {
return requestMethodConditionCache.get(HttpMethod.GET);
}
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.net.URISyntaxException;
import java.util.Collections;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;

import org.springframework.http.HttpHeaders;
Expand All @@ -44,8 +43,6 @@
*/
public class RequestMethodsRequestConditionTests {

// TODO: custom method, CORS pre-flight (see @Disabled)

@Test
public void getMatchingCondition() throws Exception {
testMatch(new RequestMethodsRequestCondition(GET), GET);
Expand Down Expand Up @@ -73,19 +70,19 @@ public void getMatchingConditionWithEmptyConditions() throws Exception {
}

@Test
@Disabled
public void getMatchingConditionWithCustomMethod() throws Exception {
ServerWebExchange exchange = getExchange("PROPFIND");
assertThat(new RequestMethodsRequestCondition().getMatchingCondition(exchange)).isNotNull();
assertThat(new RequestMethodsRequestCondition(GET, POST).getMatchingCondition(exchange)).isNull();
}

@Test
@Disabled
public void getMatchingConditionWithCorsPreFlight() throws Exception {
ServerWebExchange exchange = getExchange("OPTIONS");
exchange.getRequest().getHeaders().add("Origin", "https://example.com");
exchange.getRequest().getHeaders().add(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "PUT");
public void getMatchingConditionWithCorsPreFlight() {
MockServerHttpRequest request = MockServerHttpRequest.method(HttpMethod.valueOf("OPTIONS"), "/")
.header("Origin", "https://example.com")
.header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "PUT")
.build();
ServerWebExchange exchange = MockServerWebExchange.from(request);

assertThat(new RequestMethodsRequestCondition().getMatchingCondition(exchange)).isNotNull();
assertThat(new RequestMethodsRequestCondition(PUT).getMatchingCondition(exchange)).isNotNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,19 +157,15 @@ private RequestMethodsRequestCondition matchPreFlight(HttpServletRequest request

@Nullable
private RequestMethodsRequestCondition matchRequestMethod(String httpMethodValue) {
RequestMethod requestMethod;
try {
requestMethod = RequestMethod.valueOf(httpMethodValue);
RequestMethod requestMethod = RequestMethod.resolve(httpMethodValue);
if (requestMethod != null) {
if (getMethods().contains(requestMethod)) {
return requestMethodConditionCache.get(httpMethodValue);
}
if (requestMethod.equals(RequestMethod.HEAD) && getMethods().contains(RequestMethod.GET)) {
return requestMethodConditionCache.get(HttpMethod.GET.name());
}
}
catch (IllegalArgumentException ex) {
// Custom request method
}
return null;
}

Expand Down

0 comments on commit 88e6544

Please sign in to comment.