From 033bebf8cd4a2933788e1ea3540404aefdac2cf7 Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Wed, 10 May 2023 11:56:08 +0200 Subject: [PATCH] Remove buffering from ClientHttpRequest implementations This commit ensures that ClientHttpRequest implementations implement StreamingHttpOutputMessage, so that they do not expose an OutputStream, but store a handle capable of writing to a stream instead. Closes gh-30557 --- .../AbstractBufferingClientHttpRequest.java | 10 +- .../AbstractStreamingClientHttpRequest.java | 83 ++++++++ .../BufferingClientHttpRequestWrapper.java | 12 +- .../HttpComponentsClientHttpRequest.java | 106 ++++++++-- ...ttpComponentsClientHttpRequestFactory.java | 14 +- ...pComponentsStreamingClientHttpRequest.java | 183 ------------------ .../http/client/OkHttp3ClientHttpRequest.java | 77 +++++++- .../OkHttp3ClientHttpRequestFactory.java | 32 +-- ...uest.java => SimpleClientHttpRequest.java} | 47 +++-- .../SimpleClientHttpRequestFactory.java | 18 +- .../http/client/SimpleClientHttpResponse.java | 5 +- .../SimpleStreamingClientHttpRequest.java | 111 ----------- .../AbstractHttpRequestFactoryTests.java | 16 +- ...BufferedSimpleHttpRequestFactoryTests.java | 102 ---------- ...erceptingStreamingHttpComponentsTests.java | 4 +- ...BufferedSimpleHttpRequestFactoryTests.java | 29 --- ...treamingSimpleHttpRequestFactoryTests.java | 29 --- .../SimpleClientHttpRequestFactoryTests.java | 110 ++++++++++- ...mponentsClientHttpRequestFactoryTests.java | 41 ---- ...ngSimpleClientHttpRequestFactoryTests.java | 98 ---------- .../client/AbstractMockWebServerTests.java | 5 +- 21 files changed, 421 insertions(+), 711 deletions(-) create mode 100644 spring-web/src/main/java/org/springframework/http/client/AbstractStreamingClientHttpRequest.java delete mode 100644 spring-web/src/main/java/org/springframework/http/client/HttpComponentsStreamingClientHttpRequest.java rename spring-web/src/main/java/org/springframework/http/client/{SimpleBufferingClientHttpRequest.java => SimpleClientHttpRequest.java} (74%) delete mode 100644 spring-web/src/main/java/org/springframework/http/client/SimpleStreamingClientHttpRequest.java delete mode 100644 spring-web/src/test/java/org/springframework/http/client/BufferedSimpleHttpRequestFactoryTests.java delete mode 100644 spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingBufferedSimpleHttpRequestFactoryTests.java delete mode 100644 spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingStreamingSimpleHttpRequestFactoryTests.java delete mode 100644 spring-web/src/test/java/org/springframework/http/client/StreamingHttpComponentsClientHttpRequestFactoryTests.java delete mode 100644 spring-web/src/test/java/org/springframework/http/client/StreamingSimpleClientHttpRequestFactoryTests.java diff --git a/spring-web/src/main/java/org/springframework/http/client/AbstractBufferingClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/AbstractBufferingClientHttpRequest.java index abedb2c051c..df0936a11e3 100644 --- a/spring-web/src/main/java/org/springframework/http/client/AbstractBufferingClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/AbstractBufferingClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2023 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. @@ -16,11 +16,11 @@ package org.springframework.http.client; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; import org.springframework.http.HttpHeaders; +import org.springframework.util.FastByteArrayOutputStream; /** * Base implementation of {@link ClientHttpRequest} that buffers output @@ -31,7 +31,7 @@ import org.springframework.http.HttpHeaders; */ abstract class AbstractBufferingClientHttpRequest extends AbstractClientHttpRequest { - private ByteArrayOutputStream bufferedOutput = new ByteArrayOutputStream(1024); + private final FastByteArrayOutputStream bufferedOutput = new FastByteArrayOutputStream(1024); @Override @@ -41,12 +41,12 @@ abstract class AbstractBufferingClientHttpRequest extends AbstractClientHttpRequ @Override protected ClientHttpResponse executeInternal(HttpHeaders headers) throws IOException { - byte[] bytes = this.bufferedOutput.toByteArray(); + byte[] bytes = this.bufferedOutput.toByteArrayUnsafe(); if (headers.getContentLength() < 0) { headers.setContentLength(bytes.length); } ClientHttpResponse result = executeInternal(headers, bytes); - this.bufferedOutput = new ByteArrayOutputStream(0); + this.bufferedOutput.reset(); return result; } diff --git a/spring-web/src/main/java/org/springframework/http/client/AbstractStreamingClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/AbstractStreamingClientHttpRequest.java new file mode 100644 index 00000000000..fd0f4fcccf1 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/client/AbstractStreamingClientHttpRequest.java @@ -0,0 +1,83 @@ +/* + * Copyright 2002-2023 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 + * + * https://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.http.client; + +import java.io.IOException; +import java.io.OutputStream; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.StreamingHttpOutputMessage; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; +import org.springframework.util.FastByteArrayOutputStream; + +/** + * Abstract base for {@link ClientHttpRequest} that also implement + * {@link StreamingHttpOutputMessage}. Ensures that headers and + * body are not written multiple times. + * + * @author Arjen Poutsma + * @since 6.1 + */ +abstract class AbstractStreamingClientHttpRequest extends AbstractClientHttpRequest + implements StreamingHttpOutputMessage { + + @Nullable + private Body body; + + @Nullable + private FastByteArrayOutputStream bodyStream; + + + @Override + protected final OutputStream getBodyInternal(HttpHeaders headers) { + Assert.state(this.body == null, "Invoke either getBody or setBody; not both"); + + if (this.bodyStream == null) { + this.bodyStream = new FastByteArrayOutputStream(1024); + } + return this.bodyStream; + } + + @Override + public final void setBody(Body body) { + Assert.notNull(body, "Body must not be null"); + assertNotExecuted(); + Assert.state(this.bodyStream == null, "Invoke either getBody or setBody; not both"); + + this.body = body; + } + + @Override + protected final ClientHttpResponse executeInternal(HttpHeaders headers) throws IOException { + if (this.body == null && this.bodyStream != null) { + this.body = outputStream -> this.bodyStream.writeTo(outputStream); + } + return executeInternal(headers, this.body); + } + + + /** + * Abstract template method that writes the given headers and content to the HTTP request. + * @param headers the HTTP headers + * @param body the HTTP body, may be {@code null} if no body was {@linkplain #setBody(Body) set} + * @return the response object for the executed request + * @since 6.1 + */ + protected abstract ClientHttpResponse executeInternal(HttpHeaders headers, @Nullable Body body) throws IOException; + +} diff --git a/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpRequestWrapper.java b/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpRequestWrapper.java index 64903b154c2..680921ffc04 100644 --- a/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpRequestWrapper.java +++ b/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpRequestWrapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -21,6 +21,7 @@ import java.net.URI; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.StreamingHttpOutputMessage; import org.springframework.util.StreamUtils; /** @@ -52,7 +53,14 @@ final class BufferingClientHttpRequestWrapper extends AbstractBufferingClientHtt @Override protected ClientHttpResponse executeInternal(HttpHeaders headers, byte[] bufferedOutput) throws IOException { this.request.getHeaders().putAll(headers); - StreamUtils.copy(bufferedOutput, this.request.getBody()); + + if (this.request instanceof StreamingHttpOutputMessage streamingHttpOutputMessage) { + streamingHttpOutputMessage.setBody(outputStream -> StreamUtils.copy(bufferedOutput, outputStream)); + } + else { + StreamUtils.copy(bufferedOutput, this.request.getBody()); + } + ClientHttpResponse response = this.request.execute(); return new BufferingClientHttpResponseWrapper(response); } diff --git a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequest.java index 12c4958838f..ec1e6d33ee5 100644 --- a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -17,21 +17,24 @@ package org.springframework.http.client; import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.net.URI; import java.net.URISyntaxException; +import java.util.List; +import java.util.Set; import org.apache.hc.client5.http.classic.HttpClient; +import org.apache.hc.core5.function.Supplier; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; -import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpEntity; -import org.apache.hc.core5.http.HttpResponse; -import org.apache.hc.core5.http.io.entity.ByteArrayEntity; import org.apache.hc.core5.http.protocol.HttpContext; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; -import org.springframework.util.Assert; +import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; /** @@ -46,7 +49,7 @@ import org.springframework.util.StringUtils; * @since 3.1 * @see HttpComponentsClientHttpRequestFactory#createRequest(URI, HttpMethod) */ -final class HttpComponentsClientHttpRequest extends AbstractBufferingClientHttpRequest { +final class HttpComponentsClientHttpRequest extends AbstractStreamingClientHttpRequest { private final HttpClient httpClient; @@ -81,19 +84,16 @@ final class HttpComponentsClientHttpRequest extends AbstractBufferingClientHttpR return this.httpContext; } - - @SuppressWarnings("deprecation") // execute(ClassicHttpRequest, HttpContext) @Override - protected ClientHttpResponse executeInternal(HttpHeaders headers, byte[] bufferedOutput) throws IOException { + protected ClientHttpResponse executeInternal(HttpHeaders headers, @Nullable Body body) throws IOException { addHeaders(this.httpRequest, headers); - ContentType contentType = ContentType.parse(headers.getFirst(HttpHeaders.CONTENT_TYPE)); - HttpEntity requestEntity = new ByteArrayEntity(bufferedOutput, contentType); - this.httpRequest.setEntity(requestEntity); - HttpResponse httpResponse = this.httpClient.execute(this.httpRequest, this.httpContext); - Assert.isInstanceOf(ClassicHttpResponse.class, httpResponse, - "HttpResponse not an instance of ClassicHttpResponse"); - return new HttpComponentsClientHttpResponse((ClassicHttpResponse) httpResponse); + if (body != null) { + HttpEntity requestEntity = new BodyEntity(headers, body); + this.httpRequest.setEntity(requestEntity); + } + ClassicHttpResponse httpResponse = this.httpClient.executeOpen(null, this.httpRequest, this.httpContext); + return new HttpComponentsClientHttpResponse(httpResponse); } /** @@ -116,4 +116,78 @@ final class HttpComponentsClientHttpRequest extends AbstractBufferingClientHttpR }); } + + private static class BodyEntity implements HttpEntity { + + private final HttpHeaders headers; + + private final Body body; + + + public BodyEntity(HttpHeaders headers, Body body) { + this.headers = headers; + this.body = body; + } + + + @Override + public long getContentLength() { + return this.headers.getContentLength(); + } + + @Override + @Nullable + public String getContentType() { + return this.headers.getFirst(HttpHeaders.CONTENT_TYPE); + } + + @Override + public InputStream getContent() { + throw new UnsupportedOperationException(); + } + + @Override + public void writeTo(OutputStream outStream) throws IOException { + this.body.writeTo(outStream); + } + + @Override + public boolean isRepeatable() { + return false; + } + + @Override + public boolean isStreaming() { + return true; + } + + @Override + @Nullable + public Supplier> getTrailers() { + return null; + } + + @Override + @Nullable + public String getContentEncoding() { + return this.headers.getFirst(HttpHeaders.CONTENT_ENCODING); + } + + @Override + public boolean isChunked() { + return false; + } + + @Override + @Nullable + public Set getTrailerNames() { + return null; + } + + @Override + public void close() { + } + + } + } diff --git a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java index c3872a4ceb7..f7c9119c46e 100644 --- a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java +++ b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java @@ -65,8 +65,6 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest private HttpClient httpClient; - private boolean bufferRequestBody = true; - @Nullable private BiFunction httpContextFactory; @@ -146,9 +144,11 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest *

Default is {@code true}. When sending large amounts of data via POST or PUT, it is * recommended to change this property to {@code false}, so as not to run out of memory. * @since 4.0 + * @deprecated since 6.1 requests are never buffered, as if this property is {@code false} */ + @Deprecated(since = "6.1", forRemoval = true) public void setBufferRequestBody(boolean bufferRequestBody) { - this.bufferRequestBody = bufferRequestBody; + // no-op } /** @@ -190,13 +190,7 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest context.setAttribute(HttpClientContext.REQUEST_CONFIG, config); } } - - if (this.bufferRequestBody) { - return new HttpComponentsClientHttpRequest(client, httpRequest, context); - } - else { - return new HttpComponentsStreamingClientHttpRequest(client, httpRequest, context); - } + return new HttpComponentsClientHttpRequest(client, httpRequest, context); } diff --git a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsStreamingClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsStreamingClientHttpRequest.java deleted file mode 100644 index 9c12d187b4b..00000000000 --- a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsStreamingClientHttpRequest.java +++ /dev/null @@ -1,183 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://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.http.client; - -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.net.URI; -import java.net.URISyntaxException; -import java.util.List; -import java.util.Set; - -import org.apache.hc.client5.http.classic.HttpClient; -import org.apache.hc.core5.function.Supplier; -import org.apache.hc.core5.http.ClassicHttpRequest; -import org.apache.hc.core5.http.ClassicHttpResponse; -import org.apache.hc.core5.http.Header; -import org.apache.hc.core5.http.HttpEntity; -import org.apache.hc.core5.http.HttpResponse; -import org.apache.hc.core5.http.protocol.HttpContext; - -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpMethod; -import org.springframework.http.StreamingHttpOutputMessage; -import org.springframework.lang.Nullable; -import org.springframework.util.Assert; - -/** - * {@link ClientHttpRequest} implementation based on - * Apache HttpComponents HttpClient in streaming mode. - * - *

Created via the {@link HttpComponentsClientHttpRequestFactory}. - * - * @author Arjen Poutsma - * @since 4.0 - * @see HttpComponentsClientHttpRequestFactory#createRequest(java.net.URI, org.springframework.http.HttpMethod) - */ -final class HttpComponentsStreamingClientHttpRequest extends AbstractClientHttpRequest - implements StreamingHttpOutputMessage { - - private final HttpClient httpClient; - - private final ClassicHttpRequest httpRequest; - - private final HttpContext httpContext; - - @Nullable - private Body body; - - - HttpComponentsStreamingClientHttpRequest(HttpClient client, ClassicHttpRequest request, HttpContext context) { - this.httpClient = client; - this.httpRequest = request; - this.httpContext = context; - } - - @Override - public HttpMethod getMethod() { - return HttpMethod.valueOf(this.httpRequest.getMethod()); - } - - @Override - public URI getURI() { - try { - return this.httpRequest.getUri(); - } - catch (URISyntaxException ex) { - throw new IllegalStateException(ex.getMessage(), ex); - } - } - - @Override - public void setBody(Body body) { - assertNotExecuted(); - this.body = body; - } - - @Override - protected OutputStream getBodyInternal(HttpHeaders headers) { - throw new UnsupportedOperationException("getBody not supported"); - } - - @SuppressWarnings("deprecation") // execute(ClassicHttpRequest, HttpContext) - @Override - protected ClientHttpResponse executeInternal(HttpHeaders headers) throws IOException { - HttpComponentsClientHttpRequest.addHeaders(this.httpRequest, headers); - - if (this.body != null) { - HttpEntity requestEntity = new StreamingHttpEntity(getHeaders(), this.body); - this.httpRequest.setEntity(requestEntity); - } - HttpResponse httpResponse = this.httpClient.execute(this.httpRequest, this.httpContext); - Assert.isInstanceOf(ClassicHttpResponse.class, httpResponse, - "HttpResponse not an instance of ClassicHttpResponse"); - return new HttpComponentsClientHttpResponse((ClassicHttpResponse) httpResponse); - } - - - private static class StreamingHttpEntity implements HttpEntity { - - private final HttpHeaders headers; - - private final StreamingHttpOutputMessage.Body body; - - public StreamingHttpEntity(HttpHeaders headers, StreamingHttpOutputMessage.Body body) { - this.headers = headers; - this.body = body; - } - - @Override - public boolean isRepeatable() { - return false; - } - - @Override - public boolean isChunked() { - return false; - } - - @Override - public long getContentLength() { - return this.headers.getContentLength(); - } - - @Override - @Nullable - public String getContentType() { - return this.headers.getFirst(HttpHeaders.CONTENT_TYPE); - } - - @Override - @Nullable - public String getContentEncoding() { - return this.headers.getFirst(HttpHeaders.CONTENT_ENCODING); - } - - @Override - public InputStream getContent() throws IOException, IllegalStateException { - throw new IllegalStateException("No content available"); - } - - @Override - public void writeTo(OutputStream outputStream) throws IOException { - this.body.writeTo(outputStream); - } - - @Override - public boolean isStreaming() { - return true; - } - - @Override - @Nullable - public Supplier> getTrailers() { - return null; - } - - @Override - @Nullable - public Set getTrailerNames() { - return null; - } - - @Override - public void close() throws IOException { - } - } - -} diff --git a/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequest.java index 55602f8b8aa..29cb9817afe 100644 --- a/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -19,11 +19,16 @@ package org.springframework.http.client; import java.io.IOException; import java.net.URI; +import okhttp3.MediaType; import okhttp3.OkHttpClient; import okhttp3.Request; +import okhttp3.RequestBody; +import okio.BufferedSink; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.lang.Nullable; +import org.springframework.util.StringUtils; /** * {@link ClientHttpRequest} implementation based on OkHttp 3.x. @@ -35,7 +40,7 @@ import org.springframework.http.HttpMethod; * @author Roy Clarkson * @since 4.3 */ -class OkHttp3ClientHttpRequest extends AbstractBufferingClientHttpRequest { +class OkHttp3ClientHttpRequest extends AbstractStreamingClientHttpRequest { private final OkHttpClient client; @@ -61,11 +66,73 @@ class OkHttp3ClientHttpRequest extends AbstractBufferingClientHttpRequest { return this.uri; } - @Override - protected ClientHttpResponse executeInternal(HttpHeaders headers, byte[] content) throws IOException { - Request request = OkHttp3ClientHttpRequestFactory.buildRequest(headers, content, this.uri, this.method); + protected ClientHttpResponse executeInternal(HttpHeaders headers, @Nullable Body body) throws IOException { + + RequestBody requestBody; + if (body != null) { + requestBody = new BodyRequestBody(headers, body); + } + else if (okhttp3.internal.http.HttpMethod.requiresRequestBody(getMethod().name())) { + String header = headers.getFirst(HttpHeaders.CONTENT_TYPE); + MediaType contentType = (header != null) ? MediaType.parse(header) : null; + requestBody = RequestBody.create(contentType, new byte[0]); + } + else { + requestBody = null; + } + Request.Builder builder = new Request.Builder() + .url(this.uri.toURL()); + builder.method(this.method.name(), requestBody); + headers.forEach((headerName, headerValues) -> { + for (String headerValue : headerValues) { + builder.addHeader(headerName, headerValue); + } + }); + Request request = builder.build(); return new OkHttp3ClientHttpResponse(this.client.newCall(request).execute()); } + + private static class BodyRequestBody extends RequestBody { + + private final HttpHeaders headers; + + private final Body body; + + + public BodyRequestBody(HttpHeaders headers, Body body) { + this.headers = headers; + this.body = body; + } + + @Override + public long contentLength() { + return this.headers.getContentLength(); + } + + @Nullable + @Override + public MediaType contentType() { + String contentType = this.headers.getFirst(HttpHeaders.CONTENT_TYPE); + if (StringUtils.hasText(contentType)) { + return MediaType.parse(contentType); + } + else { + return null; + } + } + + @Override + public void writeTo(BufferedSink sink) throws IOException { + this.body.writeTo(sink.outputStream()); + } + + @Override + public boolean isOneShot() { + return true; + } + } + + } diff --git a/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java index e81bbe61a4c..93232719d81 100644 --- a/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java +++ b/spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 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. @@ -17,21 +17,15 @@ package org.springframework.http.client; import java.io.IOException; -import java.net.MalformedURLException; import java.net.URI; import java.util.concurrent.TimeUnit; import okhttp3.Cache; import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.RequestBody; import org.springframework.beans.factory.DisposableBean; -import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; -import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.StringUtils; /** * {@link ClientHttpRequestFactory} implementation that uses @@ -118,28 +112,4 @@ public class OkHttp3ClientHttpRequestFactory implements ClientHttpRequestFactory } } - - static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method) - throws MalformedURLException { - - okhttp3.MediaType contentType = getContentType(headers); - RequestBody body = (content.length > 0 || - okhttp3.internal.http.HttpMethod.requiresRequestBody(method.name()) ? - RequestBody.create(contentType, content) : null); - - Request.Builder builder = new Request.Builder().url(uri.toURL()).method(method.name(), body); - headers.forEach((headerName, headerValues) -> { - for (String headerValue : headerValues) { - builder.addHeader(headerName, headerValue); - } - }); - return builder.build(); - } - - @Nullable - private static okhttp3.MediaType getContentType(HttpHeaders headers) { - String rawContentType = headers.getFirst(HttpHeaders.CONTENT_TYPE); - return (StringUtils.hasText(rawContentType) ? okhttp3.MediaType.parse(rawContentType) : null); - } - } diff --git a/spring-web/src/main/java/org/springframework/http/client/SimpleBufferingClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpRequest.java similarity index 74% rename from spring-web/src/main/java/org/springframework/http/client/SimpleBufferingClientHttpRequest.java rename to spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpRequest.java index d11663138b4..9c6932e95c1 100644 --- a/spring-web/src/main/java/org/springframework/http/client/SimpleBufferingClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -17,34 +17,34 @@ package org.springframework.http.client; import java.io.IOException; +import java.io.OutputStream; import java.net.HttpURLConnection; import java.net.URI; import java.net.URISyntaxException; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; -import org.springframework.util.FileCopyUtils; +import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; /** * {@link ClientHttpRequest} implementation that uses standard JDK facilities to - * execute buffered requests. Created via the {@link SimpleClientHttpRequestFactory}. + * execute streaming requests. Created via the {@link SimpleClientHttpRequestFactory}. * * @author Arjen Poutsma - * @author Juergen Hoeller - * @since 3.0 - * @see SimpleClientHttpRequestFactory#createRequest(java.net.URI, HttpMethod) + * @since 6.1 + * @see SimpleClientHttpRequestFactory#createRequest(URI, HttpMethod) */ -final class SimpleBufferingClientHttpRequest extends AbstractBufferingClientHttpRequest { +final class SimpleClientHttpRequest extends AbstractStreamingClientHttpRequest { private final HttpURLConnection connection; - private final boolean outputStreaming; + private final int chunkSize; - SimpleBufferingClientHttpRequest(HttpURLConnection connection, boolean outputStreaming) { + SimpleClientHttpRequest(HttpURLConnection connection, int chunkSize) { this.connection = connection; - this.outputStreaming = outputStreaming; + this.chunkSize = chunkSize; } @Override @@ -63,18 +63,25 @@ final class SimpleBufferingClientHttpRequest extends AbstractBufferingClientHttp } @Override - protected ClientHttpResponse executeInternal(HttpHeaders headers, byte[] bufferedOutput) throws IOException { - addHeaders(this.connection, headers); - // JDK <1.8 doesn't support getOutputStream with HTTP DELETE - if (getMethod() == HttpMethod.DELETE && bufferedOutput.length == 0) { - this.connection.setDoOutput(false); - } - if (this.connection.getDoOutput() && this.outputStreaming) { - this.connection.setFixedLengthStreamingMode(bufferedOutput.length); + protected ClientHttpResponse executeInternal(HttpHeaders headers, @Nullable Body body) throws IOException { + if (this.connection.getDoOutput()) { + long contentLength = headers.getContentLength(); + if (contentLength >= 0) { + this.connection.setFixedLengthStreamingMode(contentLength); + } + else { + this.connection.setChunkedStreamingMode(this.chunkSize); + } } + + addHeaders(this.connection, headers); + this.connection.connect(); - if (this.connection.getDoOutput()) { - FileCopyUtils.copy(bufferedOutput, this.connection.getOutputStream()); + + if (this.connection.getDoOutput() && body != null) { + try (OutputStream os = this.connection.getOutputStream()) { + body.writeTo(os); + } } else { // Immediately trigger the request in a no-output scenario as well diff --git a/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java index 8af176ef10a..cc3ea82152d 100644 --- a/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java +++ b/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpRequestFactory.java @@ -43,16 +43,12 @@ public class SimpleClientHttpRequestFactory implements ClientHttpRequestFactory @Nullable private Proxy proxy; - private boolean bufferRequestBody = true; - private int chunkSize = DEFAULT_CHUNK_SIZE; private int connectTimeout = -1; private int readTimeout = -1; - private boolean outputStreaming = true; - /** * Set the {@link Proxy} to use for this request factory. @@ -73,9 +69,10 @@ public class SimpleClientHttpRequestFactory implements ClientHttpRequestFactory * (if the {@code Content-Length} is not known in advance). * @see #setChunkSize(int) * @see HttpURLConnection#setFixedLengthStreamingMode(int) + * @deprecated since 6.1 requests are never buffered, as if this property is {@code false} */ + @Deprecated(since = "6.1", forRemoval = true) public void setBufferRequestBody(boolean bufferRequestBody) { - this.bufferRequestBody = bufferRequestBody; } /** @@ -119,9 +116,11 @@ public class SimpleClientHttpRequestFactory implements ClientHttpRequestFactory * {@link HttpURLConnection#setChunkedStreamingMode} methods of the underlying connection will never * be called. * @param outputStreaming if output streaming is enabled + * @deprecated as of 6.1, this property is ignored with no direct replacement. + * @deprecated since 6.1 requests are always streamed, as if this property is {@code true} */ + @Deprecated(since = "6.1", forRemoval = true) public void setOutputStreaming(boolean outputStreaming) { - this.outputStreaming = outputStreaming; } @@ -130,12 +129,7 @@ public class SimpleClientHttpRequestFactory implements ClientHttpRequestFactory HttpURLConnection connection = openConnection(uri.toURL(), this.proxy); prepareConnection(connection, httpMethod.name()); - if (this.bufferRequestBody) { - return new SimpleBufferingClientHttpRequest(connection, this.outputStreaming); - } - else { - return new SimpleStreamingClientHttpRequest(connection, this.chunkSize, this.outputStreaming); - } + return new SimpleClientHttpRequest(connection, this.chunkSize); } /** diff --git a/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpResponse.java index be507db47a7..f4cf612bc4a 100644 --- a/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/SimpleClientHttpResponse.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2023 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. @@ -28,8 +28,7 @@ import org.springframework.util.StringUtils; /** * {@link ClientHttpResponse} implementation that uses standard JDK facilities. - * Obtained via {@link SimpleBufferingClientHttpRequest#execute()} and - * {@link SimpleStreamingClientHttpRequest#execute()}. + * Obtained via {@link SimpleClientHttpRequest#execute()}. * * @author Arjen Poutsma * @author Brian Clozel diff --git a/spring-web/src/main/java/org/springframework/http/client/SimpleStreamingClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/SimpleStreamingClientHttpRequest.java deleted file mode 100644 index d7f9097c4c3..00000000000 --- a/spring-web/src/main/java/org/springframework/http/client/SimpleStreamingClientHttpRequest.java +++ /dev/null @@ -1,111 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://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.http.client; - -import java.io.IOException; -import java.io.OutputStream; -import java.net.HttpURLConnection; -import java.net.URI; -import java.net.URISyntaxException; - -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpMethod; -import org.springframework.lang.Nullable; -import org.springframework.util.StreamUtils; - -/** - * {@link ClientHttpRequest} implementation that uses standard JDK facilities to - * execute streaming requests. Created via the {@link SimpleClientHttpRequestFactory}. - * - * @author Arjen Poutsma - * @since 3.0 - * @see SimpleClientHttpRequestFactory#createRequest(java.net.URI, HttpMethod) - * @see org.springframework.http.client.support.HttpAccessor - * @see org.springframework.web.client.RestTemplate - */ -final class SimpleStreamingClientHttpRequest extends AbstractClientHttpRequest { - - private final HttpURLConnection connection; - - private final int chunkSize; - - @Nullable - private OutputStream body; - - private final boolean outputStreaming; - - - SimpleStreamingClientHttpRequest(HttpURLConnection connection, int chunkSize, boolean outputStreaming) { - this.connection = connection; - this.chunkSize = chunkSize; - this.outputStreaming = outputStreaming; - } - - @Override - public HttpMethod getMethod() { - return HttpMethod.valueOf(this.connection.getRequestMethod()); - } - - @Override - public URI getURI() { - try { - return this.connection.getURL().toURI(); - } - catch (URISyntaxException ex) { - throw new IllegalStateException("Could not get HttpURLConnection URI: " + ex.getMessage(), ex); - } - } - - @Override - protected OutputStream getBodyInternal(HttpHeaders headers) throws IOException { - if (this.body == null) { - if (this.outputStreaming) { - long contentLength = headers.getContentLength(); - if (contentLength >= 0) { - this.connection.setFixedLengthStreamingMode(contentLength); - } - else { - this.connection.setChunkedStreamingMode(this.chunkSize); - } - } - SimpleBufferingClientHttpRequest.addHeaders(this.connection, headers); - this.connection.connect(); - this.body = this.connection.getOutputStream(); - } - return StreamUtils.nonClosing(this.body); - } - - @Override - protected ClientHttpResponse executeInternal(HttpHeaders headers) throws IOException { - try { - if (this.body != null) { - this.body.close(); - } - else { - SimpleBufferingClientHttpRequest.addHeaders(this.connection, headers); - this.connection.connect(); - // Immediately trigger the request in a no-output scenario as well - this.connection.getResponseCode(); - } - } - catch (IOException ex) { - // ignore - } - return new SimpleClientHttpResponse(this.connection); - } - -} diff --git a/spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTests.java index b48ca6aa725..5ea5d5849bf 100644 --- a/spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTests.java @@ -134,7 +134,12 @@ abstract class AbstractHttpRequestFactoryTests extends AbstractMockWebServerTest request.getHeaders().add("MyHeader", "value"); byte[] body = "Hello World".getBytes(StandardCharsets.UTF_8); assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> { - FileCopyUtils.copy(body, request.getBody()); + if (request instanceof StreamingHttpOutputMessage streamingRequest) { + streamingRequest.setBody(outputStream -> FileCopyUtils.copy(body, outputStream)); + } + else { + FileCopyUtils.copy(body, request.getBody()); + } try (ClientHttpResponse response = request.execute()) { assertThat(response).isNotNull(); request.getHeaders().add("MyHeader", "value"); @@ -155,12 +160,11 @@ abstract class AbstractHttpRequestFactoryTests extends AbstractMockWebServerTest protected void assertHttpMethod(String path, HttpMethod method) throws Exception { ClientHttpRequest request = factory.createRequest(URI.create(baseUrl + "/methods/" + path), method); if (method == HttpMethod.POST || method == HttpMethod.PUT || method == HttpMethod.PATCH) { - // requires a body - try { - request.getBody().write(32); + if (request instanceof StreamingHttpOutputMessage streamingRequest) { + streamingRequest.setBody(outputStream -> outputStream.write(32)); } - catch (UnsupportedOperationException ex) { - // probably a streaming request - let's simply ignore it + else { + request.getBody().write(32); } } diff --git a/spring-web/src/test/java/org/springframework/http/client/BufferedSimpleHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/BufferedSimpleHttpRequestFactoryTests.java deleted file mode 100644 index dbea824c20c..00000000000 --- a/spring-web/src/test/java/org/springframework/http/client/BufferedSimpleHttpRequestFactoryTests.java +++ /dev/null @@ -1,102 +0,0 @@ -/* - * Copyright 2002-2019 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 - * - * https://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.http.client; - -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.net.HttpURLConnection; -import java.net.ProtocolException; -import java.net.URL; - -import org.junit.jupiter.api.Test; - -import org.springframework.http.HttpMethod; - -import static org.assertj.core.api.Assertions.assertThat; - -public class BufferedSimpleHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { - - @Override - protected ClientHttpRequestFactory createRequestFactory() { - return new SimpleClientHttpRequestFactory(); - } - - @Override - @Test - public void httpMethods() throws Exception { - try { - assertHttpMethod("patch", HttpMethod.PATCH); - } - catch (ProtocolException ex) { - // Currently HttpURLConnection does not support HTTP PATCH - } - } - - @Test - public void prepareConnectionWithRequestBody() throws Exception { - URL uri = new URL("https://example.com"); - testRequestBodyAllowed(uri, "GET", false); - testRequestBodyAllowed(uri, "HEAD", false); - testRequestBodyAllowed(uri, "OPTIONS", false); - testRequestBodyAllowed(uri, "TRACE", false); - testRequestBodyAllowed(uri, "PUT", true); - testRequestBodyAllowed(uri, "POST", true); - testRequestBodyAllowed(uri, "DELETE", true); - } - - @Test - public void deleteWithoutBodyDoesNotRaiseException() throws Exception { - HttpURLConnection connection = new TestHttpURLConnection(new URL("https://example.com")); - ((SimpleClientHttpRequestFactory) this.factory).prepareConnection(connection, "DELETE"); - SimpleBufferingClientHttpRequest request = new SimpleBufferingClientHttpRequest(connection, false); - request.execute(); - } - - private void testRequestBodyAllowed(URL uri, String httpMethod, boolean allowed) throws IOException { - HttpURLConnection connection = new TestHttpURLConnection(uri); - ((SimpleClientHttpRequestFactory) this.factory).prepareConnection(connection, httpMethod); - assertThat(connection.getDoOutput()).isEqualTo(allowed); - } - - - private static class TestHttpURLConnection extends HttpURLConnection { - - public TestHttpURLConnection(URL uri) { - super(uri); - } - - @Override - public void connect() throws IOException { - } - - @Override - public void disconnect() { - } - - @Override - public boolean usingProxy() { - return false; - } - - @Override - public InputStream getInputStream() throws IOException { - return new ByteArrayInputStream(new byte[0]); - } - } - -} diff --git a/spring-web/src/test/java/org/springframework/http/client/InterceptingStreamingHttpComponentsTests.java b/spring-web/src/test/java/org/springframework/http/client/InterceptingStreamingHttpComponentsTests.java index 18b0cdf649b..d84bbb74006 100644 --- a/spring-web/src/test/java/org/springframework/http/client/InterceptingStreamingHttpComponentsTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/InterceptingStreamingHttpComponentsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2023 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. @@ -28,13 +28,13 @@ public class InterceptingStreamingHttpComponentsTests extends AbstractHttpReques @Override protected ClientHttpRequestFactory createRequestFactory() { HttpComponentsClientHttpRequestFactory requestFactory = new HttpComponentsClientHttpRequestFactory(); - requestFactory.setBufferRequestBody(false); return new InterceptingClientHttpRequestFactory(requestFactory, null); } @Override @Test public void httpMethods() throws Exception { + super.httpMethods(); assertHttpMethod("patch", HttpMethod.PATCH); } diff --git a/spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingBufferedSimpleHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingBufferedSimpleHttpRequestFactoryTests.java deleted file mode 100644 index af7de98d61e..00000000000 --- a/spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingBufferedSimpleHttpRequestFactoryTests.java +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright 2002-2013 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 - * - * https://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.http.client; - - -public class NoOutputStreamingBufferedSimpleHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { - - @Override - protected ClientHttpRequestFactory createRequestFactory() { - SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory(); - factory.setOutputStreaming(false); - return factory; - } - -} diff --git a/spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingStreamingSimpleHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingStreamingSimpleHttpRequestFactoryTests.java deleted file mode 100644 index fba97743533..00000000000 --- a/spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingStreamingSimpleHttpRequestFactoryTests.java +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright 2002-2013 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 - * - * https://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.http.client; - - -public class NoOutputStreamingStreamingSimpleHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { - - @Override - protected ClientHttpRequestFactory createRequestFactory() { - SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory(); - factory.setBufferRequestBody(false); - factory.setOutputStreaming(false); - return factory; - } -} diff --git a/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java index fc4755db016..558c61a7888 100644 --- a/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java @@ -16,21 +16,94 @@ package org.springframework.http.client; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.net.HttpURLConnection; +import java.net.ProtocolException; +import java.net.URI; +import java.net.URL; +import java.util.Collections; import org.junit.jupiter.api.Test; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -/** - * @author Stephane Nicoll - */ -public class SimpleClientHttpRequestFactoryTests { +public class SimpleClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { + + @Override + protected ClientHttpRequestFactory createRequestFactory() { + return new SimpleClientHttpRequestFactory(); + } + + @Override + @Test + public void httpMethods() throws Exception { + super.httpMethods(); + assertThatExceptionOfType(ProtocolException.class).isThrownBy(() -> + assertHttpMethod("patch", HttpMethod.PATCH)); + } + + @Test + public void prepareConnectionWithRequestBody() throws Exception { + URI uri = new URI("https://example.com"); + testRequestBodyAllowed(uri, "GET", false); + testRequestBodyAllowed(uri, "HEAD", false); + testRequestBodyAllowed(uri, "OPTIONS", false); + testRequestBodyAllowed(uri, "TRACE", false); + testRequestBodyAllowed(uri, "PUT", true); + testRequestBodyAllowed(uri, "POST", true); + testRequestBodyAllowed(uri, "DELETE", true); + } + + private void testRequestBodyAllowed(URI uri, String httpMethod, boolean allowed) throws IOException { + HttpURLConnection connection = new TestHttpURLConnection(uri.toURL()); + ((SimpleClientHttpRequestFactory) this.factory).prepareConnection(connection, httpMethod); + assertThat(connection.getDoOutput()).isEqualTo(allowed); + } + + @Test + public void deleteWithoutBodyDoesNotRaiseException() throws Exception { + HttpURLConnection connection = new TestHttpURLConnection(new URL("https://example.com")); + ((SimpleClientHttpRequestFactory) this.factory).prepareConnection(connection, "DELETE"); + SimpleClientHttpRequest request = new SimpleClientHttpRequest(connection, 4096); + request.execute(); + } + + @Test // SPR-8809 + public void interceptor() throws Exception { + final String headerName = "MyHeader"; + final String headerValue = "MyValue"; + ClientHttpRequestInterceptor interceptor = (request, body, execution) -> { + request.getHeaders().add(headerName, headerValue); + return execution.execute(request, body); + }; + InterceptingClientHttpRequestFactory factory = new InterceptingClientHttpRequestFactory( + createRequestFactory(), Collections.singletonList(interceptor)); + + ClientHttpResponse response = null; + try { + ClientHttpRequest request = factory.createRequest(URI.create(baseUrl + "/echo"), HttpMethod.GET); + response = request.execute(); + assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK); + HttpHeaders responseHeaders = response.getHeaders(); + assertThat(responseHeaders.getFirst(headerName)).as("Custom header invalid").isEqualTo(headerValue); + } + finally { + if (response != null) { + response.close(); + } + } + } @Test // SPR-13225 @@ -39,8 +112,35 @@ public class SimpleClientHttpRequestFactoryTests { given(urlConnection.getRequestMethod()).willReturn("GET"); HttpHeaders headers = new HttpHeaders(); headers.set("foo", null); - SimpleBufferingClientHttpRequest.addHeaders(urlConnection, headers); + SimpleClientHttpRequest.addHeaders(urlConnection, headers); verify(urlConnection, times(1)).addRequestProperty("foo", ""); } + + + private static class TestHttpURLConnection extends HttpURLConnection { + + public TestHttpURLConnection(URL uri) { + super(uri); + } + + @Override + public void connect() throws IOException { + } + + @Override + public void disconnect() { + } + + @Override + public boolean usingProxy() { + return false; + } + + @Override + public InputStream getInputStream() throws IOException { + return new ByteArrayInputStream(new byte[0]); + } + } + } diff --git a/spring-web/src/test/java/org/springframework/http/client/StreamingHttpComponentsClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/StreamingHttpComponentsClientHttpRequestFactoryTests.java deleted file mode 100644 index 50576aa0268..00000000000 --- a/spring-web/src/test/java/org/springframework/http/client/StreamingHttpComponentsClientHttpRequestFactoryTests.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2002-2018 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 - * - * https://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.http.client; - -import org.junit.jupiter.api.Test; - -import org.springframework.http.HttpMethod; - -/** - * @author Arjen Poutsma - */ -public class StreamingHttpComponentsClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { - - @Override - protected ClientHttpRequestFactory createRequestFactory() { - HttpComponentsClientHttpRequestFactory requestFactory = new HttpComponentsClientHttpRequestFactory(); - requestFactory.setBufferRequestBody(false); - return requestFactory; - } - - @Override - @Test - public void httpMethods() throws Exception { - assertHttpMethod("patch", HttpMethod.PATCH); - } - -} diff --git a/spring-web/src/test/java/org/springframework/http/client/StreamingSimpleClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/StreamingSimpleClientHttpRequestFactoryTests.java deleted file mode 100644 index 00350e33735..00000000000 --- a/spring-web/src/test/java/org/springframework/http/client/StreamingSimpleClientHttpRequestFactoryTests.java +++ /dev/null @@ -1,98 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://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.http.client; - -import java.io.OutputStream; -import java.net.URI; -import java.util.Collections; -import java.util.Random; - -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; - -import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpMethod; -import org.springframework.http.HttpStatus; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - * @author Arjen Poutsma - */ -public class StreamingSimpleClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { - - @Override - protected ClientHttpRequestFactory createRequestFactory() { - SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory(); - factory.setBufferRequestBody(false); - return factory; - } - - @Test // SPR-8809 - public void interceptor() throws Exception { - final String headerName = "MyHeader"; - final String headerValue = "MyValue"; - ClientHttpRequestInterceptor interceptor = (request, body, execution) -> { - request.getHeaders().add(headerName, headerValue); - return execution.execute(request, body); - }; - InterceptingClientHttpRequestFactory factory = new InterceptingClientHttpRequestFactory( - createRequestFactory(), Collections.singletonList(interceptor)); - - ClientHttpResponse response = null; - try { - ClientHttpRequest request = factory.createRequest(URI.create(baseUrl + "/echo"), HttpMethod.GET); - response = request.execute(); - assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK); - HttpHeaders responseHeaders = response.getHeaders(); - assertThat(responseHeaders.getFirst(headerName)).as("Custom header invalid").isEqualTo(headerValue); - } - finally { - if (response != null) { - response.close(); - } - } - } - - @Test - @Disabled - public void largeFileUpload() throws Exception { - Random rnd = new Random(); - ClientHttpResponse response = null; - try { - ClientHttpRequest request = factory.createRequest(URI.create(baseUrl + "/methods/post"), HttpMethod.POST); - final int BUF_SIZE = 4096; - final int ITERATIONS = Integer.MAX_VALUE / BUF_SIZE; - // final int contentLength = ITERATIONS * BUF_SIZE; - // request.getHeaders().setContentLength(contentLength); - OutputStream body = request.getBody(); - for (int i = 0; i < ITERATIONS; i++) { - byte[] buffer = new byte[BUF_SIZE]; - rnd.nextBytes(buffer); - body.write(buffer); - } - response = request.execute(); - assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK); - } - finally { - if (response != null) { - response.close(); - } - } - } - -} diff --git a/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTests.java b/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTests.java index abbd5a7144b..a92c05d2723 100644 --- a/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTests.java +++ b/spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTests.java @@ -113,7 +113,10 @@ abstract class AbstractMockWebServerTests { private MockResponse jsonPostRequest(RecordedRequest request, String location, String contentType) { if (request.getBodySize() > 0) { - assertThat(Integer.parseInt(request.getHeader(CONTENT_LENGTH))).as("Invalid request content-length").isGreaterThan(0); + String contentLength = request.getHeader(CONTENT_LENGTH); + if (contentLength != null) { + assertThat(Integer.parseInt(contentLength)).as("Invalid request content-length").isGreaterThan(0); + } assertThat(request.getHeader(CONTENT_TYPE)).as("No content-type").isNotNull(); } return new MockResponse()