From e8e7fbba94165df1572d48e62fbbc5b637a616ed Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 11 Jan 2022 17:07:26 +0000 Subject: [PATCH] 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 d1e67ae9c4..8a9e1a3f90 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 a4317c817d..fd48945d5f 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 b2bfded605..4e1df906eb 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 836b4b7b17..332183457e 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())); + } + }