From 1061bcdba2d2953df0a7b07ca177354babfcb645 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 22 Sep 2020 07:33:03 +0100 Subject: [PATCH] Set sameSite in ClientHttpResponse implementations Closes gh-25785 --- .../HttpComponentsClientHttpResponse.java | 16 +++++---- .../reactive/JettyClientHttpResponse.java | 31 ++++++++++++----- .../reactive/ReactorClientHttpResponse.java | 33 ++++++++++++++----- .../client/WebClientIntegrationTests.java | 9 +++-- 4 files changed, 61 insertions(+), 28 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpResponse.java index df39dd1d40..c284834e15 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpResponse.java @@ -81,13 +81,15 @@ class HttpComponentsClientHttpResponse implements ClientHttpResponse { public MultiValueMap getCookies() { LinkedMultiValueMap result = new LinkedMultiValueMap<>(); this.context.getCookieStore().getCookies().forEach(cookie -> - result.add(cookie.getName(), ResponseCookie.fromClientResponse(cookie.getName(), cookie.getValue()) - .domain(cookie.getDomain()) - .path(cookie.getPath()) - .maxAge(getMaxAgeSeconds(cookie)) - .secure(cookie.isSecure()) - .httpOnly(cookie.containsAttribute("httponly")) - .build())); + result.add(cookie.getName(), + ResponseCookie.fromClientResponse(cookie.getName(), cookie.getValue()) + .domain(cookie.getDomain()) + .path(cookie.getPath()) + .maxAge(getMaxAgeSeconds(cookie)) + .secure(cookie.isSecure()) + .httpOnly(cookie.containsAttribute("httponly")) + .sameSite(cookie.getAttribute("samesite")) + .build())); return result; } diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java index 29683498e8..80a1260bb6 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpResponse.java @@ -18,6 +18,8 @@ package org.springframework.http.client.reactive; import java.net.HttpCookie; import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.eclipse.jetty.reactive.client.ReactiveResponse; import org.reactivestreams.Publisher; @@ -27,6 +29,7 @@ import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseCookie; +import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -41,6 +44,9 @@ import org.springframework.util.MultiValueMap; */ class JettyClientHttpResponse implements ClientHttpResponse { + private static final Pattern SAMESITE_PATTERN = Pattern.compile("(?i).*SameSite=(Strict|Lax|None).*"); + + private final ReactiveResponse reactiveResponse; private final Flux content; @@ -72,19 +78,28 @@ class JettyClientHttpResponse implements ClientHttpResponse { MultiValueMap result = new LinkedMultiValueMap<>(); List cookieHeader = getHeaders().get(HttpHeaders.SET_COOKIE); if (cookieHeader != null) { - cookieHeader.forEach(header -> HttpCookie.parse(header) - .forEach(c -> result.add(c.getName(), ResponseCookie.fromClientResponse(c.getName(), c.getValue()) - .domain(c.getDomain()) - .path(c.getPath()) - .maxAge(c.getMaxAge()) - .secure(c.getSecure()) - .httpOnly(c.isHttpOnly()) - .build())) + cookieHeader.forEach(header -> + HttpCookie.parse(header).forEach(cookie -> result.add(cookie.getName(), + ResponseCookie.fromClientResponse(cookie.getName(), cookie.getValue()) + .domain(cookie.getDomain()) + .path(cookie.getPath()) + .maxAge(cookie.getMaxAge()) + .secure(cookie.getSecure()) + .httpOnly(cookie.isHttpOnly()) + .sameSite(parseSameSite(header)) + .build())) ); } return CollectionUtils.unmodifiableMultiValueMap(result); } + @Nullable + private static String parseSameSite(String headerValue) { + Matcher matcher = SAMESITE_PATTERN.matcher(headerValue); + return (matcher.matches() ? matcher.group(1) : null); + } + + @Override public Flux getBody() { return this.content; diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java index b5a24f8e1f..1a39b7c5cd 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpResponse.java @@ -21,6 +21,8 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiFunction; import io.netty.buffer.ByteBufAllocator; +import io.netty.handler.codec.http.cookie.Cookie; +import io.netty.handler.codec.http.cookie.DefaultCookie; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Flux; @@ -34,6 +36,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseCookie; +import org.springframework.lang.Nullable; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; @@ -129,17 +132,31 @@ class ReactorClientHttpResponse implements ClientHttpResponse { @Override public MultiValueMap getCookies() { MultiValueMap result = new LinkedMultiValueMap<>(); - this.response.cookies().values().stream().flatMap(Collection::stream) - .forEach(c -> result.add(c.name(), ResponseCookie.fromClientResponse(c.name(), c.value()) - .domain(c.domain()) - .path(c.path()) - .maxAge(c.maxAge()) - .secure(c.isSecure()) - .httpOnly(c.isHttpOnly()) - .build())); + this.response.cookies().values().stream() + .flatMap(Collection::stream) + .forEach(cookie -> result.add(cookie.name(), + ResponseCookie.fromClientResponse(cookie.name(), cookie.value()) + .domain(cookie.domain()) + .path(cookie.path()) + .maxAge(cookie.maxAge()) + .secure(cookie.isSecure()) + .httpOnly(cookie.isHttpOnly()) + .sameSite(getSameSite(cookie)) + .build())); return CollectionUtils.unmodifiableMultiValueMap(result); } + @Nullable + private static String getSameSite(Cookie cookie) { + if (cookie instanceof DefaultCookie) { + DefaultCookie defaultCookie = (DefaultCookie) cookie; + if (defaultCookie.sameSite() != null) { + return defaultCookie.sameSite().name(); + } + } + return null; + } + /** * Called by {@link ReactorClientHttpConnector} when a cancellation is detected * but the content has not been subscribed to. If the subscription never diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java index c3f07ec6a9..50a9b9f65b 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java @@ -120,10 +120,8 @@ class WebClientIntegrationTests { void retrieve(ClientHttpConnector connector) { startServer(connector); - prepareResponse(response -> response.setHeader("Content-Type", "text/plain") - .addHeader("Set-Cookie", "testkey1=testvalue1;") - .addHeader("Set-Cookie", "testkey2=testvalue2; Max-Age=42; HttpOnly; Secure") - .setBody("Hello Spring!")); + prepareResponse(response -> + response.setHeader("Content-Type", "text/plain").setBody("Hello Spring!")); Mono result = this.webClient.get() .uri("/greeting") @@ -1102,7 +1100,7 @@ class WebClientIntegrationTests { prepareResponse(response -> response .setHeader("Content-Type", "text/plain") .addHeader("Set-Cookie", "testkey1=testvalue1;") - .addHeader("Set-Cookie", "testkey2=testvalue2; Max-Age=42; HttpOnly; Secure") + .addHeader("Set-Cookie", "testkey2=testvalue2; Max-Age=42; HttpOnly; SameSite=Lax; Secure") .setBody("test")); Mono result = this.webClient.get() @@ -1123,6 +1121,7 @@ class WebClientIntegrationTests { assertThat(cookie2.getValue()).isEqualTo("testvalue2"); assertThat(cookie2.isSecure()).isTrue(); assertThat(cookie2.isHttpOnly()).isTrue(); + assertThat(cookie2.getSameSite()).isEqualTo("Lax"); assertThat(cookie2.getMaxAge().getSeconds()).isEqualTo(42); }) .expectComplete()