diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java index ce5a3d2b37..e664157ac0 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/DefaultServerHttpRequestBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -20,7 +20,6 @@ import java.net.InetSocketAddress; import java.net.URI; import java.net.URISyntaxException; import java.util.Arrays; -import java.util.LinkedList; import java.util.function.Consumer; import reactor.core.publisher.Flux; @@ -31,7 +30,6 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; @@ -46,12 +44,10 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { private URI uri; - private HttpHeaders httpHeaders; + private HttpHeaders headers; private String httpMethodValue; - private final MultiValueMap cookies; - @Nullable private String uriPath; @@ -70,21 +66,12 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { Assert.notNull(original, "ServerHttpRequest is required"); this.uri = original.getURI(); + this.headers = HttpHeaders.writableHttpHeaders(original.getHeaders()); this.httpMethodValue = original.getMethodValue(); this.body = original.getBody(); - - this.httpHeaders = HttpHeaders.writableHttpHeaders(original.getHeaders()); - - this.cookies = new LinkedMultiValueMap<>(original.getCookies().size()); - copyMultiValueMap(original.getCookies(), this.cookies); - this.originalRequest = original; } - private static void copyMultiValueMap(MultiValueMap source, MultiValueMap target) { - source.forEach((key, value) -> target.put(key, new LinkedList<>(value))); - } - @Override public ServerHttpRequest.Builder method(HttpMethod httpMethod) { @@ -113,14 +100,14 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { @Override public ServerHttpRequest.Builder header(String headerName, String... headerValues) { - this.httpHeaders.put(headerName, Arrays.asList(headerValues)); + this.headers.put(headerName, Arrays.asList(headerValues)); return this; } @Override public ServerHttpRequest.Builder headers(Consumer headersConsumer) { Assert.notNull(headersConsumer, "'headersConsumer' must not be null"); - headersConsumer.accept(this.httpHeaders); + headersConsumer.accept(this.headers); return this; } @@ -132,8 +119,8 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { @Override public ServerHttpRequest build() { - return new MutatedServerHttpRequest(getUriToUse(), this.contextPath, this.httpHeaders, - this.httpMethodValue, this.cookies, this.sslInfo, this.body, this.originalRequest); + return new MutatedServerHttpRequest(getUriToUse(), this.contextPath, + this.httpMethodValue, this.sslInfo, this.body, this.originalRequest); } private URI getUriToUse() { @@ -179,8 +166,6 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { private final String methodValue; - private final MultiValueMap cookies; - @Nullable private final SslInfo sslInfo; @@ -190,12 +175,11 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { public MutatedServerHttpRequest(URI uri, @Nullable String contextPath, - HttpHeaders headers, String methodValue, MultiValueMap cookies, - @Nullable SslInfo sslInfo, Flux body, ServerHttpRequest originalRequest) { + String methodValue, @Nullable SslInfo sslInfo, + Flux body, ServerHttpRequest originalRequest) { - super(uri, contextPath, headers); + super(uri, contextPath, originalRequest.getHeaders()); this.methodValue = methodValue; - this.cookies = cookies; this.sslInfo = sslInfo != null ? sslInfo : originalRequest.getSslInfo(); this.body = body; this.originalRequest = originalRequest; @@ -208,7 +192,7 @@ class DefaultServerHttpRequestBuilder implements ServerHttpRequest.Builder { @Override protected MultiValueMap initCookies() { - return this.cookies; + return this.originalRequest.getCookies(); } @Nullable diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java index c6917031bc..8662d28c0a 100644 --- a/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/ServerHttpRequestTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -49,13 +49,13 @@ public class ServerHttpRequestTests { @Test public void queryParamsNone() throws Exception { - MultiValueMap params = createHttpRequest("/path").getQueryParams(); + MultiValueMap params = createRequest("/path").getQueryParams(); assertThat(params.size()).isEqualTo(0); } @Test public void queryParams() throws Exception { - MultiValueMap params = createHttpRequest("/path?a=A&b=B").getQueryParams(); + MultiValueMap params = createRequest("/path?a=A&b=B").getQueryParams(); assertThat(params.size()).isEqualTo(2); assertThat(params.get("a")).isEqualTo(Collections.singletonList("A")); assertThat(params.get("b")).isEqualTo(Collections.singletonList("B")); @@ -63,84 +63,87 @@ public class ServerHttpRequestTests { @Test public void queryParamsWithMultipleValues() throws Exception { - MultiValueMap params = createHttpRequest("/path?a=1&a=2").getQueryParams(); + MultiValueMap params = createRequest("/path?a=1&a=2").getQueryParams(); assertThat(params.size()).isEqualTo(1); assertThat(params.get("a")).isEqualTo(Arrays.asList("1", "2")); } @Test // SPR-15140 public void queryParamsWithEncodedValue() throws Exception { - MultiValueMap params = createHttpRequest("/path?a=%20%2B+%C3%A0").getQueryParams(); + MultiValueMap params = createRequest("/path?a=%20%2B+%C3%A0").getQueryParams(); assertThat(params.size()).isEqualTo(1); assertThat(params.get("a")).isEqualTo(Collections.singletonList(" + \u00e0")); } @Test public void queryParamsWithEmptyValue() throws Exception { - MultiValueMap params = createHttpRequest("/path?a=").getQueryParams(); + MultiValueMap params = createRequest("/path?a=").getQueryParams(); assertThat(params.size()).isEqualTo(1); assertThat(params.get("a")).isEqualTo(Collections.singletonList("")); } @Test public void queryParamsWithNoValue() throws Exception { - MultiValueMap params = createHttpRequest("/path?a").getQueryParams(); + MultiValueMap params = createRequest("/path?a").getQueryParams(); assertThat(params.size()).isEqualTo(1); assertThat(params.get("a")).isEqualTo(Collections.singletonList(null)); } @Test - public void mutateRequest() throws Exception { - SslInfo sslInfo = mock(SslInfo.class); - ServerHttpRequest request = createHttpRequest("/").mutate().sslInfo(sslInfo).build(); - assertThat(request.getSslInfo()).isSameAs(sslInfo); - - request = createHttpRequest("/").mutate().method(HttpMethod.DELETE).build(); + public void mutateRequestMethod() throws Exception { + ServerHttpRequest request = createRequest("/").mutate().method(HttpMethod.DELETE).build(); assertThat(request.getMethod()).isEqualTo(HttpMethod.DELETE); + } + @Test + public void mutateSslInfo() throws Exception { + SslInfo sslInfo = mock(SslInfo.class); + ServerHttpRequest request = createRequest("/").mutate().sslInfo(sslInfo).build(); + assertThat(request.getSslInfo()).isSameAs(sslInfo); + } + + @Test + public void mutateUriAndPath() throws Exception { String baseUri = "https://aaa.org:8080/a"; - request = createHttpRequest(baseUri).mutate().uri(URI.create("https://bbb.org:9090/b")).build(); + ServerHttpRequest request = createRequest(baseUri).mutate().uri(URI.create("https://bbb.org:9090/b")).build(); assertThat(request.getURI().toString()).isEqualTo("https://bbb.org:9090/b"); - request = createHttpRequest(baseUri).mutate().path("/b/c/d").build(); + request = createRequest(baseUri).mutate().path("/b/c/d").build(); assertThat(request.getURI().toString()).isEqualTo("https://aaa.org:8080/b/c/d"); - request = createHttpRequest(baseUri).mutate().path("/app/b/c/d").contextPath("/app").build(); + request = createRequest(baseUri).mutate().path("/app/b/c/d").contextPath("/app").build(); assertThat(request.getURI().toString()).isEqualTo("https://aaa.org:8080/app/b/c/d"); assertThat(request.getPath().contextPath().value()).isEqualTo("/app"); } - @Test - public void mutateWithInvalidPath() throws Exception { - assertThatIllegalArgumentException().isThrownBy(() -> - createHttpRequest("/").mutate().path("foo-bar")); - } - @Test // SPR-16434 public void mutatePathWithEncodedQueryParams() throws Exception { - ServerHttpRequest request = createHttpRequest("/path?name=%E6%89%8E%E6%A0%B9"); + ServerHttpRequest request = createRequest("/path?name=%E6%89%8E%E6%A0%B9"); request = request.mutate().path("/mutatedPath").build(); assertThat(request.getURI().getRawPath()).isEqualTo("/mutatedPath"); assertThat(request.getURI().getRawQuery()).isEqualTo("name=%E6%89%8E%E6%A0%B9"); } + @Test + public void mutateWithInvalidPath() { + assertThatIllegalArgumentException().isThrownBy(() -> createRequest("/").mutate().path("foo-bar")); + } + @Test public void mutateHeadersViaConsumer() throws Exception { String headerName = "key"; String headerValue1 = "value1"; String headerValue2 = "value2"; - ServerHttpRequest request = createHttpRequest("/path"); + ServerHttpRequest request = createRequest("/path"); assertThat(request.getHeaders().get(headerName)).isNull(); request = request.mutate().headers(headers -> headers.add(headerName, headerValue1)).build(); - assertThat(request.getHeaders().get(headerName)).containsExactly(headerValue1); request = request.mutate().headers(headers -> headers.add(headerName, headerValue2)).build(); - assertThat(request.getHeaders().get(headerName)).containsExactly(headerValue1, headerValue2); } @@ -151,19 +154,17 @@ public class ServerHttpRequestTests { String headerValue2 = "value2"; String headerValue3 = "value3"; - ServerHttpRequest request = createHttpRequest("/path"); + ServerHttpRequest request = createRequest("/path"); assertThat(request.getHeaders().get(headerName)).isNull(); request = request.mutate().header(headerName, headerValue1, headerValue2).build(); - assertThat(request.getHeaders().get(headerName)).containsExactly(headerValue1, headerValue2); request = request.mutate().header(headerName, headerValue3).build(); - assertThat(request.getHeaders().get(headerName)).containsExactly(headerValue3); } - private ServerHttpRequest createHttpRequest(String uriString) throws Exception { + private ServerHttpRequest createRequest(String uriString) throws Exception { URI uri = URI.create(uriString); MockHttpServletRequest request = new TestHttpServletRequest(uri); AsyncContext asyncContext = new MockAsyncContext(request, new MockHttpServletResponse()); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java index 45ad32585d..f3fe50ea75 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java @@ -31,6 +31,7 @@ import org.springframework.http.HttpRequest; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseCookie; import org.springframework.http.client.reactive.ClientHttpResponse; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; @@ -69,12 +70,17 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { private int statusCode = 200; - private final HttpHeaders headers; + @Nullable + private HttpHeaders headers; - private final MultiValueMap cookies; + @Nullable + private MultiValueMap cookies; private Flux body = Flux.empty(); + @Nullable + private ClientResponse originalResponse; + private HttpRequest request; @@ -91,14 +97,13 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { this.strategies = other.strategies(); this.statusCode = other.rawStatusCode(); if (mutate) { - this.headers = HttpHeaders.writableHttpHeaders(other.headers().asHttpHeaders()); this.body = other.bodyToFlux(DataBuffer.class); } else { this.headers = new HttpHeaders(); - headers(headers -> headers.addAll(other.headers().asHttpHeaders())); + this.headers.addAll(other.headers().asHttpHeaders()); } - this.cookies = new LinkedMultiValueMap<>(other.cookies()); + this.originalResponse = other; this.request = (other instanceof DefaultClientResponse ? ((DefaultClientResponse) other).request() : EMPTY_REQUEST); } @@ -119,31 +124,47 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { @Override public ClientResponse.Builder header(String headerName, String... headerValues) { for (String headerValue : headerValues) { - this.headers.add(headerName, headerValue); + getHeaders().add(headerName, headerValue); } return this; } @Override public ClientResponse.Builder headers(Consumer headersConsumer) { - headersConsumer.accept(this.headers); + headersConsumer.accept(getHeaders()); return this; } + @SuppressWarnings("ConstantConditions") + private HttpHeaders getHeaders() { + if (this.headers == null) { + this.headers = HttpHeaders.writableHttpHeaders(this.originalResponse.headers().asHttpHeaders()); + } + return this.headers; + } + @Override public DefaultClientResponseBuilder cookie(String name, String... values) { for (String value : values) { - this.cookies.add(name, ResponseCookie.from(name, value).build()); + getCookies().add(name, ResponseCookie.from(name, value).build()); } return this; } @Override public ClientResponse.Builder cookies(Consumer> cookiesConsumer) { - cookiesConsumer.accept(this.cookies); + cookiesConsumer.accept(getCookies()); return this; } + @SuppressWarnings("ConstantConditions") + private MultiValueMap getCookies() { + if (this.cookies == null) { + this.cookies = new LinkedMultiValueMap<>(this.originalResponse.cookies()); + } + return this.cookies; + } + @Override public ClientResponse.Builder body(Function, Flux> transformer) { this.body = transformer.apply(this.body); @@ -184,8 +205,8 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { @Override public ClientResponse build() { - ClientHttpResponse httpResponse = - new BuiltClientHttpResponse(this.statusCode, this.headers, this.cookies, this.body); + ClientHttpResponse httpResponse = new BuiltClientHttpResponse( + this.statusCode, this.headers, this.cookies, this.body, this.originalResponse); return new DefaultClientResponse( httpResponse, this.strategies, "", "", () -> this.request); @@ -196,19 +217,33 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { private final int statusCode; + @Nullable private final HttpHeaders headers; + @Nullable private final MultiValueMap cookies; private final Flux body; - public BuiltClientHttpResponse(int statusCode, HttpHeaders headers, - MultiValueMap cookies, Flux body) { + @Nullable + private final ClientResponse originalResponse; + + + public BuiltClientHttpResponse(int statusCode, @Nullable HttpHeaders headers, + @Nullable MultiValueMap cookies, Flux body, + @Nullable ClientResponse originalResponse) { + + Assert.isTrue(headers != null || originalResponse != null, + "Expected either headers or an original response with headers."); + + Assert.isTrue(cookies != null || originalResponse != null, + "Expected either cookies or an original response with cookies."); this.statusCode = statusCode; - this.headers = HttpHeaders.readOnlyHttpHeaders(headers); - this.cookies = CollectionUtils.unmodifiableMultiValueMap(cookies); + this.headers = (headers != null ? HttpHeaders.readOnlyHttpHeaders(headers) : null); + this.cookies = (cookies != null ? CollectionUtils.unmodifiableMultiValueMap(cookies) : null); this.body = body; + this.originalResponse = originalResponse; } @Override @@ -222,13 +257,15 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { } @Override + @SuppressWarnings("ConstantConditions") public HttpHeaders getHeaders() { - return this.headers; + return (this.headers != null ? this.headers : this.originalResponse.headers().asHttpHeaders()); } @Override + @SuppressWarnings("ConstantConditions") public MultiValueMap getCookies() { - return this.cookies; + return (this.cookies != null ? this.cookies : this.originalResponse.cookies()); } @Override