Skip to content

Commit

Permalink
Do not set 0 Content-Length header in BufferingClientHttpRequestFactory
Browse files Browse the repository at this point in the history
Prior to this commit, the `BufferingClientHttpRequestFactory`, through
the `AbstractBufferingClientHttpRequest`, would set a "Content-Length"
header value, even if the buffered body was empty.

This behavior is invalid since no request body would be set by the
client code in the first place.

This commit ensures that this header is only set if a request body has
been buffered and is about to be written to the request.

Fixes gh-32650
  • Loading branch information
bclozel committed Apr 17, 2024
1 parent 8ec2743 commit b3a4567
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 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.
Expand Down Expand Up @@ -42,7 +42,7 @@ protected OutputStream getBodyInternal(HttpHeaders headers) throws IOException {
@Override
protected ClientHttpResponse executeInternal(HttpHeaders headers) throws IOException {
byte[] bytes = this.bufferedOutput.toByteArrayUnsafe();
if (headers.getContentLength() < 0) {
if (bytes.length > 0 && headers.getContentLength() < 0) {
headers.setContentLength(bytes.length);
}
ClientHttpResponse result = executeInternal(headers, bytes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ else if(request.getPath().startsWith("/methods/")) {
assertThat(request.getMethod()).isEqualTo(expectedMethod);
return new MockResponse();
}
else if(request.getPath().startsWith("/header/")) {
String headerName = request.getPath().replace("/header/","");
return new MockResponse().setBody(headerName + ":" + request.getHeader(headerName)).setResponseCode(200);
}
return new MockResponse().setResponseCode(404);
}
catch (Throwable exc) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.springframework.http.client;

import java.io.InputStreamReader;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
Expand Down Expand Up @@ -49,12 +50,7 @@ void repeatableRead() throws Exception {
FileCopyUtils.copy(body, request.getBody());
try (ClientHttpResponse response = request.execute()) {
assertThat(response.getStatusCode()).as("Invalid status code").isEqualTo(HttpStatus.OK);
assertThat(response.getStatusCode()).as("Invalid status code").isEqualTo(HttpStatus.OK);

assertThat(response.getHeaders().containsKey(headerName)).as("Header not found").isTrue();
assertThat(response.getHeaders().containsKey(headerName)).as("Header not found").isTrue();

assertThat(response.getHeaders().get(headerName)).as("Header value not found").isEqualTo(Arrays.asList(headerValue1, headerValue2));
assertThat(response.getHeaders().get(headerName)).as("Header value not found").isEqualTo(Arrays.asList(headerValue1, headerValue2));

byte[] result = FileCopyUtils.copyToByteArray(response.getBody());
Expand All @@ -64,4 +60,14 @@ void repeatableRead() throws Exception {
}
}

@Test
void shouldNotSetContentLengthWhenEmptyBody() throws Exception {
ClientHttpRequest request = factory.createRequest(URI.create(baseUrl + "/header/Content-Length"), HttpMethod.POST);
try (ClientHttpResponse response = request.execute()) {
assertThat(response.getStatusCode()).as("Invalid status code").isEqualTo(HttpStatus.OK);
String result = FileCopyUtils.copyToString(new InputStreamReader(response.getBody()));
assertThat(result).as("Invalid body").isEqualTo("Content-Length:null");
}
}

}

0 comments on commit b3a4567

Please sign in to comment.