From 5488906d0c01ca280cbc44b2fb373cfed10ab76f Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 9 Feb 2017 22:36:28 +0100 Subject: [PATCH] Polish Issue: SPR-15206 --- .../reactive/HiddenHttpMethodFilter.java | 58 ++++++++------- .../reactive/HiddenHttpMethodFilterTests.java | 74 ++++++++++++++----- 2 files changed, 86 insertions(+), 46 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilter.java b/spring-web/src/main/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilter.java index de0f19a9df9..1fdeac1e55b 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 the original author or authors. + * Copyright 2002-2017 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. @@ -13,26 +13,28 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.web.filter.reactive; +import java.util.Locale; + +import reactor.core.publisher.Mono; + import org.springframework.http.HttpMethod; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; import org.springframework.web.server.WebFilterChain; -import reactor.core.publisher.Mono; - -import java.util.Locale; -import java.util.Optional; /** * Reactive {@link WebFilter} that converts posted method parameters into HTTP methods, * retrievable via {@link ServerHttpRequest#getMethod()}. Since browsers currently only - * support GET and POST, a common technique - used by the Prototype library, for instance - - * is to use a normal POST with an additional hidden form field ({@code _method}) - * to pass the "real" HTTP method along. This filter reads that parameter and changes - * the {@link ServerHttpRequest#getMethod()} return value using {@link ServerWebExchange#mutate()}. + * support GET and POST, a common technique is to use a normal POST with an additional + * hidden form field ({@code _method}) to pass the "real" HTTP method along. + * This filter reads that parameter and changes the {@link ServerHttpRequest#getMethod()} + * return value using {@link ServerWebExchange#mutate()}. * *

The name of the request parameter defaults to {@code _method}, but can be * adapted via the {@link #setMethodParam(String) methodParam} property. @@ -61,37 +63,41 @@ public class HiddenHttpMethodFilter implements WebFilter { * * @param exchange the current server exchange * @param chain provides a way to delegate to the next filter - * @return + * @return {@code Mono} to indicate when request processing is complete */ @Override public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { if (exchange.getRequest().getMethod() == HttpMethod.POST) { return exchange.getFormData() - .map(map -> Optional.ofNullable(map.getFirst(methodParam))) - .map(method -> convertedRequest(exchange, method)) - .then(convertedExchange -> chain.filter(convertedExchange)); - } else { + .map(formData -> { + String method = formData.getFirst(methodParam); + if (StringUtils.hasLength(method)) { + return convertedRequest(exchange, method); + } + else { + return exchange; + } + }) + .then(convertedExchange -> chain.filter(convertedExchange)); + } + else { return chain.filter(exchange); } } /** - * Mutate exchange into a new HTTP method. + * Mutate exchange into a new HTTP request method. * - * @param exchange - original request - * @param method - request HTTP method based on form data + * @param exchange original {@link ServerWebExchange} + * @param method request HTTP method based on form data * @return a mutated {@link ServerWebExchange} */ - private ServerWebExchange convertedRequest(ServerWebExchange exchange, Optional method) { - - String upperMethod = method - .map(String::toString) - .orElse(HttpMethod.POST.toString()) - .toUpperCase(Locale.ENGLISH); - + private ServerWebExchange convertedRequest(ServerWebExchange exchange, String method) { + HttpMethod resolved = HttpMethod.resolve(method.toUpperCase(Locale.ENGLISH)); + Assert.notNull(resolved, () -> "HttpMethod '" + method + "' is not supported"); return exchange.mutate() - .request(builder -> builder.method(HttpMethod.resolve(upperMethod))) - .build(); + .request(builder -> builder.method(resolved)) + .build(); } } diff --git a/spring-web/src/test/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilterTests.java index 84af27f7417..88e7c5ab574 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/reactive/HiddenHttpMethodFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2017 the original author or authors. + * Copyright 2002-2017 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. @@ -13,9 +13,16 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.web.filter.reactive; +import java.util.Optional; + +import org.hamcrest.Matchers; import org.junit.Test; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; @@ -24,14 +31,13 @@ import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilterChain; import org.springframework.web.server.adapter.DefaultServerWebExchange; -import reactor.core.publisher.Mono; -import reactor.test.StepVerifier; - -import java.util.Optional; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; /** + * Tests for {@link HiddenHttpMethodFilter} + * * @author Greg Turnquist */ public class HiddenHttpMethodFilterTests { @@ -48,8 +54,22 @@ public class HiddenHttpMethodFilterTests { }; StepVerifier.create(filter.filter(mockExchange, filterChain)) - .expectComplete() - .verify(); + .expectComplete() + .verify(); + } + + @Test + public void filterWithInvalidParameter() { + ServerWebExchange mockExchange = createExchange(Optional.of("INVALID")); + + WebFilterChain filterChain = exchange -> Mono.empty(); + + StepVerifier.create(filter.filter(mockExchange, filterChain)) + .consumeErrorWith(error -> { + assertThat(error, Matchers.instanceOf(IllegalArgumentException.class)); + assertEquals(error.getMessage(), "HttpMethod 'INVALID' is not supported"); + }) + .verify(); } @Test @@ -62,8 +82,22 @@ public class HiddenHttpMethodFilterTests { }; StepVerifier.create(filter.filter(mockExchange, filterChain)) - .expectComplete() - .verify(); + .expectComplete() + .verify(); + } + + @Test + public void filterWithEmptyStringParameter() { + ServerWebExchange mockExchange = createExchange(Optional.of("")); + + WebFilterChain filterChain = exchange -> { + assertEquals("Invalid method", HttpMethod.POST, exchange.getRequest().getMethod()); + return Mono.empty(); + }; + + StepVerifier.create(filter.filter(mockExchange, filterChain)) + .expectComplete() + .verify(); } @Test @@ -78,15 +112,15 @@ public class HiddenHttpMethodFilterTests { filter.setMethodParam("_foo"); StepVerifier.create(filter.filter(mockExchange, filterChain)) - .expectComplete() - .verify(); + .expectComplete() + .verify(); } @Test public void filterWithoutPost() { ServerWebExchange mockExchange = createExchange(Optional.of("DELETE")).mutate() - .request(builder -> builder.method(HttpMethod.PUT)) - .build(); + .request(builder -> builder.method(HttpMethod.PUT)) + .build(); WebFilterChain filterChain = exchange -> { assertEquals("Invalid method", HttpMethod.PUT, exchange.getRequest().getMethod()); @@ -94,8 +128,8 @@ public class HiddenHttpMethodFilterTests { }; StepVerifier.create(filter.filter(mockExchange, filterChain)) - .expectComplete() - .verify(); + .expectComplete() + .verify(); } private ServerWebExchange createExchange(Optional optionalMethod) { @@ -104,12 +138,12 @@ public class HiddenHttpMethodFilterTests { private ServerWebExchange createExchange(String methodName, Optional optionalBody) { MockServerHttpRequest.BodyBuilder builder = MockServerHttpRequest - .post("/hotels") - .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE); + .post("/hotels") + .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE); MockServerHttpRequest request = optionalBody - .map(method -> builder.body(methodName + "=" + method)) - .orElse(builder.build()); + .map(method -> builder.body(methodName + "=" + method)) + .orElse(builder.build()); MockServerHttpResponse response = new MockServerHttpResponse();