Fix regression in WebFlux support for WebDAV methods

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
This commit is contained in:
Arjen Poutsma 2023-02-17 11:27:47 +01:00
parent 1999c78350
commit 88e6544d9d
6 changed files with 151 additions and 43 deletions

View File

@ -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;
@ -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;
@ -121,13 +114,17 @@ public final class HttpMethod implements Comparable<HttpMethod>, Serializable {
*/
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);
};
}
/**

View File

@ -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.
@ -34,6 +38,62 @@ package org.springframework.web.bind.annotation;
*/
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;
};
}
}

View File

@ -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());
}
}
}

View File

@ -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));
}
}
@ -150,16 +150,15 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi
}
@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;
}

View File

@ -19,7 +19,6 @@ package org.springframework.web.reactive.result.condition;
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;
@ -44,8 +43,6 @@ import static org.springframework.web.bind.annotation.RequestMethod.PUT;
*/
public class RequestMethodsRequestConditionTests {
// TODO: custom method, CORS pre-flight (see @Disabled)
@Test
public void getMatchingCondition() throws Exception {
testMatch(new RequestMethodsRequestCondition(GET), GET);
@ -73,7 +70,6 @@ public class RequestMethodsRequestConditionTests {
}
@Test
@Disabled
public void getMatchingConditionWithCustomMethod() throws Exception {
ServerWebExchange exchange = getExchange("PROPFIND");
assertThat(new RequestMethodsRequestCondition().getMatchingCondition(exchange)).isNotNull();
@ -81,11 +77,12 @@ public class RequestMethodsRequestConditionTests {
}
@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();

View File

@ -157,9 +157,8 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi
@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);
}
@ -167,9 +166,6 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi
return requestMethodConditionCache.get(HttpMethod.GET.name());
}
}
catch (IllegalArgumentException ex) {
// Custom request method
}
return null;
}