From 019ce4418cd57391ed61c7fb6812bbd527db35e6 Mon Sep 17 00:00:00 2001
From: Brian Clozel <brian.clozel@broadcom.com>
Date: Wed, 10 Apr 2024 16:41:16 +0200
Subject: [PATCH] BufferingClientHttpRequestWrapper should not write empty body

Prior to this commit, `BufferingClientHttpRequestWrapper` would always
write to the actual client request body, even if the buffered content
was empty (empty byte array).
This would cause issues with specific client request factories,
especially the OkHttp variant, that would consider empty byte arrays as
non-empty body and would reject such cases for GET requests with an
"IllegalArgumentException: method GET must not have a request body".

This commit only writes to the request if the buffered content is not
empty.

Fixes gh-32612
---
 .../BufferingClientHttpRequestWrapper.java    | 30 +++++++++---------
 ...ientHttpRequestFactoryWithOkHttpTests.java | 31 +++++++++++++++++++
 2 files changed, 47 insertions(+), 14 deletions(-)
 create mode 100644 spring-web/src/test/java/org/springframework/http/client/BufferingClientHttpRequestFactoryWithOkHttpTests.java

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 47adf2431db..9425cc35ef6 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
@@ -55,21 +55,23 @@ final class BufferingClientHttpRequestWrapper extends AbstractBufferingClientHtt
 	protected ClientHttpResponse executeInternal(HttpHeaders headers, byte[] bufferedOutput) throws IOException {
 		this.request.getHeaders().putAll(headers);
 
-		if (this.request instanceof StreamingHttpOutputMessage streamingHttpOutputMessage) {
-			streamingHttpOutputMessage.setBody(new StreamingHttpOutputMessage.Body() {
-				@Override
-				public void writeTo(OutputStream outputStream) throws IOException {
-					StreamUtils.copy(bufferedOutput, outputStream);
-				}
+		if (bufferedOutput.length > 0) {
+			if (this.request instanceof StreamingHttpOutputMessage streamingHttpOutputMessage) {
+				streamingHttpOutputMessage.setBody(new StreamingHttpOutputMessage.Body() {
+					@Override
+					public void writeTo(OutputStream outputStream) throws IOException {
+						StreamUtils.copy(bufferedOutput, outputStream);
+					}
 
-				@Override
-				public boolean repeatable() {
-					return true;
-				}
-			});
-		}
-		else {
-			StreamUtils.copy(bufferedOutput, this.request.getBody());
+					@Override
+					public boolean repeatable() {
+						return true;
+					}
+				});
+			}
+			else {
+				StreamUtils.copy(bufferedOutput, this.request.getBody());
+			}
 		}
 
 		ClientHttpResponse response = this.request.execute();
diff --git a/spring-web/src/test/java/org/springframework/http/client/BufferingClientHttpRequestFactoryWithOkHttpTests.java b/spring-web/src/test/java/org/springframework/http/client/BufferingClientHttpRequestFactoryWithOkHttpTests.java
new file mode 100644
index 00000000000..0d2436996ba
--- /dev/null
+++ b/spring-web/src/test/java/org/springframework/http/client/BufferingClientHttpRequestFactoryWithOkHttpTests.java
@@ -0,0 +1,31 @@
+/*
+ * 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.
+ * 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;
+
+/**
+ * Tests for {@link BufferingClientHttpRequestWrapper} for clients
+ * not supporting non-null, empty request bodies for GET requests.
+ */
+class BufferingClientHttpRequestFactoryWithOkHttpTests extends AbstractHttpRequestFactoryTests {
+
+	@Override
+	@SuppressWarnings("removal")
+	protected ClientHttpRequestFactory createRequestFactory() {
+		return new BufferingClientHttpRequestFactory(new OkHttp3ClientHttpRequestFactory());
+	}
+
+}