From 02a2c400c77cf7ea1d867e933654b56da09856ed Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Wed, 6 Sep 2017 14:39:09 +0200 Subject: [PATCH] Move URL transform methods from ServerHttpResponse to ServerWebExchange This commit moves `encodeUrl` and `registerUrlEncoder` from ServerHttpResponse to ServerWebExchange. It also renames `encodeUrl` to `transformUrl` and `registerUrlEncoder` to `addUrlTransformer` to make it clearer that these methods do not perform actual URL encodings (i.e. they do not replaceinvalid characters). The `add` prefix (instead of `register`) makes it clearer that each function is added in addition to the previous one. Issue: SPR-15924 --- .../reactive/AbstractServerHttpResponse.java | 14 ----- .../server/reactive/ServerHttpResponse.java | 20 ------- .../reactive/ServerHttpResponseDecorator.java | 11 ---- .../web/server/ServerWebExchange.java | 18 ++++++ .../server/ServerWebExchangeDecorator.java | 12 +++- .../adapter/DefaultServerWebExchange.java | 15 +++++ .../reactive/ServerHttpResponseTests.java | 26 +------- .../adapter/ServerWebExchangeTests.java | 60 +++++++++++++++++++ .../reactive/result/view/RedirectView.java | 4 +- .../reactive/result/view/RequestContext.java | 4 +- 10 files changed, 110 insertions(+), 74 deletions(-) create mode 100644 spring-web/src/test/java/org/springframework/web/server/adapter/ServerWebExchangeTests.java diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java index 02b729fac3..c786c198f2 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractServerHttpResponse.java @@ -19,7 +19,6 @@ package org.springframework.http.server.reactive; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -69,9 +68,6 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { private final MultiValueMap cookies; - @Nullable - private Function urlEncoder = url -> url; - private final AtomicReference state = new AtomicReference<>(State.NEW); private final List>> commitActions = new ArrayList<>(4); @@ -136,16 +132,6 @@ public abstract class AbstractServerHttpResponse implements ServerHttpResponse { } } - @Override - public String encodeUrl(String url) { - return (this.urlEncoder != null ? this.urlEncoder.apply(url) : url); - } - - @Override - public void registerUrlEncoder(Function encoder) { - this.urlEncoder = (this.urlEncoder != null ? this.urlEncoder.andThen(encoder) : encoder); - } - @Override public void beforeCommit(Supplier> action) { this.commitActions.add(action); diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java index 7eea41af1d..099fee14cd 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponse.java @@ -16,8 +16,6 @@ package org.springframework.http.server.reactive; -import java.util.function.Function; - import org.springframework.http.HttpStatus; import org.springframework.http.ReactiveHttpOutputMessage; import org.springframework.http.ResponseCookie; @@ -59,22 +57,4 @@ public interface ServerHttpResponse extends ReactiveHttpOutputMessage { */ void addCookie(ResponseCookie cookie); - /** - * A mechanism for URL rewriting that applications and libraries such as - * HTML template libraries to use consistently for all URLs emitted by - * the application. Doing so enables the registration of URL encoders via - * {@link #registerUrlEncoder} that can insert an id for authentication, - * a nonce for CSRF protection, or other. - * @param url the URL to encode - * @return the encoded URL or the same - */ - String encodeUrl(String url); - - /** - * Register a URL rewriting function for use with {@link #encodeUrl}. - * The function must return an encoded URL or the same URL. - * @param encoder a URL encoding function to use - */ - void registerUrlEncoder(Function encoder); - } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponseDecorator.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponseDecorator.java index 036f2c8515..abfeb9eb88 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponseDecorator.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpResponseDecorator.java @@ -15,7 +15,6 @@ */ package org.springframework.http.server.reactive; -import java.util.function.Function; import java.util.function.Supplier; import org.reactivestreams.Publisher; @@ -80,16 +79,6 @@ public class ServerHttpResponseDecorator implements ServerHttpResponse { getDelegate().addCookie(cookie); } - @Override - public String encodeUrl(String url) { - return getDelegate().encodeUrl(url); - } - - @Override - public void registerUrlEncoder(Function encoder) { - getDelegate().registerUrlEncoder(encoder); - } - @Override public DataBufferFactory bufferFactory() { return getDelegate().bufferFactory(); diff --git a/spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java b/spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java index 1d41a3ba81..c68f3358b1 100644 --- a/spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java +++ b/spring-web/src/main/java/org/springframework/web/server/ServerWebExchange.java @@ -20,6 +20,7 @@ import java.security.Principal; import java.time.Instant; import java.util.Map; import java.util.function.Consumer; +import java.util.function.Function; import reactor.core.publisher.Mono; @@ -177,6 +178,23 @@ public interface ServerWebExchange { */ boolean checkNotModified(@Nullable String etag, Instant lastModified); + /** + * Transform the given url according to the registered transformation function(s). + * By default, this method returns the given {@code url}, though additional + * transformation functions can by registered with {@link #addUrlTransformer} + * @param url the URL to transform + * @return the transformed URL + */ + String transformUrl(String url); + + /** + * Register an additional URL transformation function for use with {@link #transformUrl}. + * The given function can be used to insert an id for authentication, a nonce for CSRF + * protection, etc. + *

Note that the given function is applied after any previously registered functions. + * @param transformer a URL transformation function to add + */ + void addUrlTransformer(Function transformer); /** * Return a builder to mutate properties of this exchange by wrapping it diff --git a/spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java b/spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java index 55eeb6a9be..109975d1f9 100644 --- a/spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java +++ b/spring-web/src/main/java/org/springframework/web/server/ServerWebExchangeDecorator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 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. @@ -18,6 +18,7 @@ package org.springframework.web.server; import java.security.Principal; import java.time.Instant; import java.util.Map; +import java.util.function.Function; import reactor.core.publisher.Mono; @@ -120,6 +121,15 @@ public class ServerWebExchangeDecorator implements ServerWebExchange { return getDelegate().checkNotModified(etag, lastModified); } + @Override + public String transformUrl(String url) { + return getDelegate().transformUrl(url); + } + + @Override + public void addUrlTransformer(Function transformer) { + getDelegate().addUrlTransformer(transformer); + } @Override public String toString() { diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java b/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java index bec32d1e4b..854b1238f1 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/DefaultServerWebExchange.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; import reactor.core.publisher.Mono; @@ -91,6 +92,8 @@ public class DefaultServerWebExchange implements ServerWebExchange { private volatile boolean notModified; + private Function urlTransformer = url -> url; + public DefaultServerWebExchange(ServerHttpRequest request, ServerHttpResponse response, WebSessionManager sessionManager, ServerCodecConfigurer codecConfigurer, LocaleContextResolver localeContextResolver) { @@ -322,4 +325,16 @@ public class DefaultServerWebExchange implements ServerWebExchange { return true; } + @Override + public String transformUrl(String url) { + return this.urlTransformer.apply(url); + } + + @Override + public void addUrlTransformer(Function transformer) { + Assert.notNull(transformer, "'encoder' must not be null"); + this.urlTransformer = this.urlTransformer.andThen(transformer); + } + + } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java index b5c81ba8d3..acf0b364d9 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpResponseTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 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. @@ -32,9 +32,7 @@ import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.ResponseCookie; import static junit.framework.TestCase.assertTrue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertSame; +import static org.junit.Assert.*; /** * @author Rossen Stoyanchev @@ -42,26 +40,6 @@ import static org.junit.Assert.assertSame; */ public class ServerHttpResponseTests { - @Test - public void encodeUrlDefault() throws Exception { - TestServerHttpResponse response = new TestServerHttpResponse(); - assertEquals("/foo", response.encodeUrl("/foo")); - } - - @Test - public void encodeUrlWithEncoder() throws Exception { - TestServerHttpResponse response = new TestServerHttpResponse(); - response.registerUrlEncoder(s -> s + "?nonce=123"); - assertEquals("/foo?nonce=123", response.encodeUrl("/foo")); - } - - @Test - public void encodeUrlWithMultipleEncoders() throws Exception { - TestServerHttpResponse response = new TestServerHttpResponse(); - response.registerUrlEncoder(s -> s + ";p=abc"); - response.registerUrlEncoder(s -> s + "?q=123"); - assertEquals("/foo;p=abc?q=123", response.encodeUrl("/foo")); - } @Test public void writeWith() throws Exception { diff --git a/spring-web/src/test/java/org/springframework/web/server/adapter/ServerWebExchangeTests.java b/spring-web/src/test/java/org/springframework/web/server/adapter/ServerWebExchangeTests.java new file mode 100644 index 0000000000..38eeaa760a --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/server/adapter/ServerWebExchangeTests.java @@ -0,0 +1,60 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * http://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.server.adapter; + +import org.junit.Before; +import org.junit.Test; + +import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; +import org.springframework.mock.http.server.reactive.test.MockServerWebExchange; +import org.springframework.web.server.ServerWebExchange; + +import static org.junit.Assert.*; + +/** + * @author Arjen Poutsma + */ +public class ServerWebExchangeTests { + + private ServerWebExchange exchange; + + @Before + public void createExchange() { + MockServerHttpRequest request = MockServerHttpRequest.get("http://example.com").build(); + this.exchange = new MockServerWebExchange(request); + } + + @Test + public void transformUrlDefault() throws Exception { + assertEquals("/foo", this.exchange.transformUrl("/foo")); + } + + @Test + public void transformUrlWithEncoder() throws Exception { + this.exchange.addUrlTransformer(s -> s + "?nonce=123"); + assertEquals("/foo?nonce=123", this.exchange.transformUrl("/foo")); + } + + @Test + public void transformUrlWithMultipleEncoders() throws Exception { + this.exchange.addUrlTransformer(s -> s + ";p=abc"); + this.exchange.addUrlTransformer(s -> s + "?q=123"); + assertEquals("/foo;p=abc?q=123", this.exchange.transformUrl("/foo")); + } + + +} \ No newline at end of file diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/RedirectView.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/RedirectView.java index def02a1b59..c61b0e24f7 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/RedirectView.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/RedirectView.java @@ -289,9 +289,9 @@ public class RedirectView extends AbstractUrlBasedView { * @param exchange current exchange */ protected Mono sendRedirect(String targetUrl, ServerWebExchange exchange) { + String transformedUrl = (isRemoteHost(targetUrl) ? targetUrl : exchange.transformUrl(targetUrl)); ServerHttpResponse response = exchange.getResponse(); - String encodedURL = (isRemoteHost(targetUrl) ? targetUrl : response.encodeUrl(targetUrl)); - response.getHeaders().setLocation(URI.create(encodedURL)); + response.getHeaders().setLocation(URI.create(transformedUrl)); response.setStatusCode(getStatusCode()); return Mono.empty(); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/RequestContext.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/RequestContext.java index f36eac690c..58c528baaa 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/RequestContext.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/view/RequestContext.java @@ -203,7 +203,7 @@ public class RequestContext { */ public String getContextUrl(String relativeUrl) { String url = StringUtils.applyRelativePath(getContextPath() + "/", relativeUrl); - return getExchange().getResponse().encodeUrl(url); + return getExchange().transformUrl(url); } /** @@ -220,7 +220,7 @@ public class RequestContext { String url = StringUtils.applyRelativePath(getContextPath() + "/", relativeUrl); UriTemplate template = new UriTemplate(url); url = template.expand(params).toASCIIString(); - return getExchange().getResponse().encodeUrl(url); + return getExchange().transformUrl(url); } /**