From bd1f34e17403dd26f3cb7473bd3d49b2c5f6705f Mon Sep 17 00:00:00 2001 From: sokomishalov Date: Mon, 10 Jan 2022 11:51:55 +0300 Subject: [PATCH 1/2] Apply adapters to client request headers after committed See gh-27768 --- .../reactive/AbstractClientHttpRequest.java | 11 ++++-- .../HttpComponentsClientHttpRequest.java | 5 +++ .../HttpComponentsHeadersAdapter.java | 36 +++++++++---------- .../reactive/JettyClientHttpRequest.java | 4 +++ .../reactive/ReactorClientHttpRequest.java | 6 ++++ 5 files changed, 42 insertions(+), 20 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/AbstractClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/AbstractClientHttpRequest.java index 8734521f048..d1e67ae9c4c 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/AbstractClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/AbstractClientHttpRequest.java @@ -74,14 +74,13 @@ public abstract class AbstractClientHttpRequest implements ClientHttpRequest { this.cookies = new LinkedMultiValueMap<>(); } - @Override public HttpHeaders getHeaders() { if (this.readOnlyHeaders != null) { return this.readOnlyHeaders; } else if (State.COMMITTED.equals(this.state.get())) { - this.readOnlyHeaders = HttpHeaders.readOnlyHttpHeaders(this.headers); + this.readOnlyHeaders = initReadOnlyHeaders(); return this.readOnlyHeaders; } else { @@ -144,6 +143,14 @@ public abstract class AbstractClientHttpRequest implements ClientHttpRequest { return Flux.concat(actions).then(); } + /** + * Initialize read-only headers with underlying request headers. + * @return read-only headers + */ + protected HttpHeaders initReadOnlyHeaders() { + return HttpHeaders.readOnlyHttpHeaders(this.headers); + } + /** * Apply header changes from {@link #getHeaders()} to the underlying request. diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java index 440fc681860..a4317c817dc 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java @@ -118,6 +118,11 @@ class HttpComponentsClientHttpRequest extends AbstractClientHttpRequest { return doCommit(); } + @Override + protected HttpHeaders initReadOnlyHeaders() { + return HttpHeaders.readOnlyHttpHeaders(new HttpComponentsHeadersAdapter(this.httpRequest)); + } + @Override protected void applyHeaders() { HttpHeaders headers = getHeaders(); diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsHeadersAdapter.java b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsHeadersAdapter.java index f24d77cb86d..944703f8e16 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsHeadersAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsHeadersAdapter.java @@ -28,7 +28,7 @@ import java.util.Map; import java.util.Set; import org.apache.hc.core5.http.Header; -import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.HttpMessage; import org.springframework.http.HttpHeaders; import org.springframework.lang.Nullable; @@ -44,23 +44,23 @@ import org.springframework.util.MultiValueMap; */ class HttpComponentsHeadersAdapter implements MultiValueMap { - private final HttpResponse response; + private final HttpMessage message; - HttpComponentsHeadersAdapter(HttpResponse response) { - this.response = response; + HttpComponentsHeadersAdapter(HttpMessage message) { + this.message = message; } @Override public String getFirst(String key) { - Header header = this.response.getFirstHeader(key); + Header header = this.message.getFirstHeader(key); return (header != null ? header.getValue() : null); } @Override public void add(String key, @Nullable String value) { - this.response.addHeader(key, value); + this.message.addHeader(key, value); } @Override @@ -75,7 +75,7 @@ class HttpComponentsHeadersAdapter implements MultiValueMap { @Override public void set(String key, @Nullable String value) { - this.response.setHeader(key, value); + this.message.setHeader(key, value); } @Override @@ -86,29 +86,29 @@ class HttpComponentsHeadersAdapter implements MultiValueMap { @Override public Map toSingleValueMap() { Map map = CollectionUtils.newLinkedHashMap(size()); - this.response.headerIterator().forEachRemaining(h -> map.putIfAbsent(h.getName(), h.getValue())); + this.message.headerIterator().forEachRemaining(h -> map.putIfAbsent(h.getName(), h.getValue())); return map; } @Override public int size() { - return this.response.getHeaders().length; + return this.message.getHeaders().length; } @Override public boolean isEmpty() { - return (this.response.getHeaders().length == 0); + return (this.message.getHeaders().length == 0); } @Override public boolean containsKey(Object key) { - return (key instanceof String && this.response.containsHeader((String) key)); + return (key instanceof String && this.message.containsHeader((String) key)); } @Override public boolean containsValue(Object value) { return (value instanceof String && - Arrays.stream(this.response.getHeaders()).anyMatch(h -> h.getValue().equals(value))); + Arrays.stream(this.message.getHeaders()).anyMatch(h -> h.getValue().equals(value))); } @Nullable @@ -116,7 +116,7 @@ class HttpComponentsHeadersAdapter implements MultiValueMap { public List get(Object key) { List values = null; if (containsKey(key)) { - Header[] headers = this.response.getHeaders((String) key); + Header[] headers = this.message.getHeaders((String) key); values = new ArrayList<>(headers.length); for (Header header : headers) { values.add(header.getValue()); @@ -138,7 +138,7 @@ class HttpComponentsHeadersAdapter implements MultiValueMap { public List remove(Object key) { if (key instanceof String) { List oldValues = get(key); - this.response.removeHeaders((String) key); + this.message.removeHeaders((String) key); return oldValues; } return null; @@ -151,13 +151,13 @@ class HttpComponentsHeadersAdapter implements MultiValueMap { @Override public void clear() { - this.response.setHeaders(); + this.message.setHeaders(); } @Override public Set keySet() { Set keys = new LinkedHashSet<>(size()); - for (Header header : this.response.getHeaders()) { + for (Header header : this.message.getHeaders()) { keys.add(header.getName()); } return keys; @@ -166,7 +166,7 @@ class HttpComponentsHeadersAdapter implements MultiValueMap { @Override public Collection> values() { Collection> values = new ArrayList<>(size()); - for (Header header : this.response.getHeaders()) { + for (Header header : this.message.getHeaders()) { values.add(get(header.getName())); } return values; @@ -196,7 +196,7 @@ class HttpComponentsHeadersAdapter implements MultiValueMap { private class EntryIterator implements Iterator>> { - private Iterator
iterator = response.headerIterator(); + private final Iterator
iterator = message.headerIterator(); @Override public boolean hasNext() { diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java index 8aabf895792..b2bfded6051 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java @@ -126,6 +126,10 @@ class JettyClientHttpRequest extends AbstractClientHttpRequest { }); } + @Override + protected HttpHeaders initReadOnlyHeaders() { + return HttpHeaders.readOnlyHttpHeaders(new JettyHeadersAdapter(this.jettyRequest.getHeaders())); + } @Override protected void applyCookies() { diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java index f098f8594ba..836b4b7b17d 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java @@ -31,6 +31,7 @@ import reactor.netty.http.client.HttpClientRequest; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.NettyDataBufferFactory; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.ZeroCopyHttpOutputMessage; @@ -121,6 +122,11 @@ class ReactorClientHttpRequest extends AbstractClientHttpRequest implements Zero return doCommit(this.outbound::then); } + @Override + protected HttpHeaders initReadOnlyHeaders() { + return HttpHeaders.readOnlyHttpHeaders(new NettyHeadersAdapter(this.request.requestHeaders())); + } + @Override protected void applyHeaders() { getHeaders().forEach((key, value) -> this.request.requestHeaders().set(key, value)); From e8e7fbba94165df1572d48e62fbbc5b637a616ed Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 11 Jan 2022 17:07:26 +0000 Subject: [PATCH 2/2] Polishing contribution and fix failing test The failing test is for Apache HttpComponents where we cannot apply the Content-Length together with other headers, and must pass it instead explicitly to ReactiveEntityProducer when writing at which point it gets set in the native headers. Closes gh-27768 --- .../reactive/AbstractClientHttpRequest.java | 21 +++++++++++-------- .../HttpComponentsClientHttpRequest.java | 20 +++++++++++------- .../reactive/JettyClientHttpRequest.java | 13 ++++++------ .../reactive/ReactorClientHttpRequest.java | 12 +++++------ 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/AbstractClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/AbstractClientHttpRequest.java index d1e67ae9c4c..8a9e1a3f90b 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/AbstractClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/AbstractClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -74,6 +74,7 @@ public abstract class AbstractClientHttpRequest implements ClientHttpRequest { this.cookies = new LinkedMultiValueMap<>(); } + @Override public HttpHeaders getHeaders() { if (this.readOnlyHeaders != null) { @@ -88,6 +89,16 @@ public abstract class AbstractClientHttpRequest implements ClientHttpRequest { } } + /** + * Initialize the read-only headers after the request is committed. + *

By default, this method simply applies a read-only wrapper. + * Subclasses can do the same for headers from the native request. + * @since 5.3.15 + */ + protected HttpHeaders initReadOnlyHeaders() { + return HttpHeaders.readOnlyHttpHeaders(this.headers); + } + @Override public MultiValueMap getCookies() { if (State.COMMITTED.equals(this.state.get())) { @@ -143,14 +154,6 @@ public abstract class AbstractClientHttpRequest implements ClientHttpRequest { return Flux.concat(actions).then(); } - /** - * Initialize read-only headers with underlying request headers. - * @return read-only headers - */ - protected HttpHeaders initReadOnlyHeaders() { - return HttpHeaders.readOnlyHttpHeaders(this.headers); - } - /** * Apply header changes from {@link #getHeaders()} to the underlying request. diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java index a4317c817dc..fd48945d5fe 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/HttpComponentsClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -62,6 +62,8 @@ class HttpComponentsClientHttpRequest extends AbstractClientHttpRequest { @Nullable private Flux byteBufferFlux; + private transient long contentLength = -1; + public HttpComponentsClientHttpRequest(HttpMethod method, URI uri, HttpClientContext context, DataBufferFactory dataBufferFactory) { @@ -118,11 +120,6 @@ class HttpComponentsClientHttpRequest extends AbstractClientHttpRequest { return doCommit(); } - @Override - protected HttpHeaders initReadOnlyHeaders() { - return HttpHeaders.readOnlyHttpHeaders(new HttpComponentsHeadersAdapter(this.httpRequest)); - } - @Override protected void applyHeaders() { HttpHeaders headers = getHeaders(); @@ -135,6 +132,8 @@ class HttpComponentsClientHttpRequest extends AbstractClientHttpRequest { if (!this.httpRequest.containsHeader(HttpHeaders.ACCEPT)) { this.httpRequest.addHeader(HttpHeaders.ACCEPT, ALL_VALUE); } + + this.contentLength = headers.getContentLength(); } @Override @@ -156,6 +155,11 @@ class HttpComponentsClientHttpRequest extends AbstractClientHttpRequest { }); } + @Override + protected HttpHeaders initReadOnlyHeaders() { + return HttpHeaders.readOnlyHttpHeaders(new HttpComponentsHeadersAdapter(this.httpRequest)); + } + public AsyncRequestProducer toRequestProducer() { ReactiveEntityProducer reactiveEntityProducer = null; @@ -165,8 +169,8 @@ class HttpComponentsClientHttpRequest extends AbstractClientHttpRequest { if (getHeaders().getContentType() != null) { contentType = ContentType.parse(getHeaders().getContentType().toString()); } - reactiveEntityProducer = new ReactiveEntityProducer(this.byteBufferFlux, getHeaders().getContentLength(), - contentType, contentEncoding); + reactiveEntityProducer = new ReactiveEntityProducer( + this.byteBufferFlux, this.contentLength, contentType, contentEncoding); } return new BasicRequestProducer(this.httpRequest, reactiveEntityProducer); diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java index b2bfded6051..4e1df906eb2 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/JettyClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -126,11 +126,6 @@ class JettyClientHttpRequest extends AbstractClientHttpRequest { }); } - @Override - protected HttpHeaders initReadOnlyHeaders() { - return HttpHeaders.readOnlyHttpHeaders(new JettyHeadersAdapter(this.jettyRequest.getHeaders())); - } - @Override protected void applyCookies() { getCookies().values().stream().flatMap(Collection::stream) @@ -147,9 +142,13 @@ class JettyClientHttpRequest extends AbstractClientHttpRequest { } } + @Override + protected HttpHeaders initReadOnlyHeaders() { + return HttpHeaders.readOnlyHttpHeaders(new JettyHeadersAdapter(this.jettyRequest.getHeaders())); + } + public ReactiveRequest toReactiveRequest() { return this.builder.build(); } - } diff --git a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java index 836b4b7b17d..332183457e6 100644 --- a/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/reactive/ReactorClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -122,11 +122,6 @@ class ReactorClientHttpRequest extends AbstractClientHttpRequest implements Zero return doCommit(this.outbound::then); } - @Override - protected HttpHeaders initReadOnlyHeaders() { - return HttpHeaders.readOnlyHttpHeaders(new NettyHeadersAdapter(this.request.requestHeaders())); - } - @Override protected void applyHeaders() { getHeaders().forEach((key, value) -> this.request.requestHeaders().set(key, value)); @@ -139,4 +134,9 @@ class ReactorClientHttpRequest extends AbstractClientHttpRequest implements Zero .forEach(this.request::addCookie); } + @Override + protected HttpHeaders initReadOnlyHeaders() { + return HttpHeaders.readOnlyHttpHeaders(new NettyHeadersAdapter(this.request.requestHeaders())); + } + }