From b3a45670f9b8b193a34c022b63cc2b587feae6ef Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 17 Apr 2024 22:44:36 +0200 Subject: [PATCH] Do not set 0 Content-Length header in BufferingClientHttpRequestFactory 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 --- .../AbstractBufferingClientHttpRequest.java | 4 ++-- .../http/client/AbstractMockWebServerTests.java | 4 ++++ .../BufferingClientHttpRequestFactoryTests.java | 16 +++++++++++----- 3 files changed, 17 insertions(+), 7 deletions(-) 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 df0936a11e3..769ea1ebc81 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-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. @@ -42,7 +42,7 @@ abstract class AbstractBufferingClientHttpRequest extends AbstractClientHttpRequ @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); diff --git a/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java b/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java index c2237dd61d8..c8c88be4ec7 100644 --- a/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java @@ -106,6 +106,10 @@ public abstract class AbstractMockWebServerTests { 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) { diff --git a/spring-web/src/test/java/org/springframework/http/client/BufferingClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/BufferingClientHttpRequestFactoryTests.java index 4716e351af4..8fba0a71512 100644 --- a/spring-web/src/test/java/org/springframework/http/client/BufferingClientHttpRequestFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/BufferingClientHttpRequestFactoryTests.java @@ -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; @@ -49,12 +50,7 @@ class BufferingClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryT 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()); @@ -64,4 +60,14 @@ class BufferingClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryT } } + @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"); + } + } + }