Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove buffering from ClientHttpRequest implementations

Browse Source 
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
This commit is contained in:
Arjen Poutsma
2023-05-10 11:56:08 +02:00
parent 1ce22bdcc1
commit 033bebf8cd
21 changed files with 422 additions and 712 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTests.java
View File

@@ -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);
}
}

102
spring-web/src/test/java/org/springframework/http/client/BufferedSimpleHttpRequestFactoryTests.java
View File

@@ -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]);
}
}
}

4
spring-web/src/test/java/org/springframework/http/client/InterceptingStreamingHttpComponentsTests.java
View File

@@ -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);
}

29
spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingBufferedSimpleHttpRequestFactoryTests.java
View File

@@ -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;
}
}

29
spring-web/src/test/java/org/springframework/http/client/NoOutputStreamingStreamingSimpleHttpRequestFactoryTests.java
View File

@@ -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;
}
}

110
spring-web/src/test/java/org/springframework/http/client/SimpleClientHttpRequestFactoryTests.java
View File

@@ -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]);
}
}
}

41
spring-web/src/test/java/org/springframework/http/client/StreamingHttpComponentsClientHttpRequestFactoryTests.java
View File

@@ -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);
}
}

98
spring-web/src/test/java/org/springframework/http/client/StreamingSimpleClientHttpRequestFactoryTests.java
View File

@@ -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();
}
}
}
}

5
spring-web/src/test/java/org/springframework/web/client/AbstractMockWebServerTests.java
View File

@@ -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()