trumpetinc · Jan 30, 2025 · Jan 30, 2025 · Jan 30, 2025 · Jan 30, 2025 · Jan 31, 2025
diff --git a/annotation-error-decoder/src/main/java/feign/error/ExceptionGenerator.java b/annotation-error-decoder/src/main/java/feign/error/ExceptionGenerator.java
@@ -17,6 +17,8 @@
 
 import static feign.Util.checkState;
 
+import feign.HttpBodyFactory;
+import feign.HttpBodyFactory.HttpBody;
 import feign.Request;
 import feign.Response;
 import feign.Types;
@@ -43,11 +45,11 @@ class ExceptionGenerator {
     TEST_RESPONSE =
         Response.builder()
             .status(500)
-            .body((Response.Body) null)
+            .body((HttpBody) null)
             .headers(testHeaders)
             .request(
                 Request.create(
-                    Request.HttpMethod.GET, "http://test", testHeaders, Request.Body.empty(), null))
+                    Request.HttpMethod.GET, "http://test", testHeaders, HttpBodyFactory.empty(), null))
             .build();
   }
 

diff --git a/annotation-error-decoder/src/test/java/feign/error/AbstractAnnotationErrorDecoderTest.java b/annotation-error-decoder/src/test/java/feign/error/AbstractAnnotationErrorDecoderTest.java
@@ -17,6 +17,7 @@
 
 import static feign.Feign.configKey;
 
+import feign.HttpBodyFactory;
 import feign.Request;
 import feign.Response;
 import java.nio.charset.StandardCharsets;
@@ -47,7 +48,7 @@ Response testResponse(int status, String body, Map<String, Collection<String>> h
         .headers(headers)
         .request(
             Request.create(
-                Request.HttpMethod.GET, "http://test", headers, Request.Body.empty(), null))
+                Request.HttpMethod.GET, "http://test", headers, HttpBodyFactory.empty(), null))
         .build();
   }
 }
diff --git a/...or-decoder/src/test/java/feign/error/AnnotationErrorDecoderExceptionConstructorsTest.java b/...or-decoder/src/test/java/feign/error/AnnotationErrorDecoderExceptionConstructorsTest.java
@@ -17,6 +17,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import feign.HttpBodyFactory;
 import feign.Request;
 import feign.codec.Decoder;
 import feign.error.AnnotationErrorDecoderExceptionConstructorsTest.TestClientInterfaceWithDifferentExceptionConstructors;
@@ -59,7 +60,7 @@ public class AnnotationErrorDecoderExceptionConstructorsTest
           feign.Request.HttpMethod.GET,
           "http://test",
           Collections.emptyMap(),
-          Request.Body.empty(),
+          HttpBodyFactory.empty(),
           null);
   private static final feign.Request NO_REQUEST = null;
   private static final Map<String, Collection<String>> NON_NULL_HEADERS = new HashMap<>();

diff --git a/core/src/main/java/feign/Client.java b/core/src/main/java/feign/Client.java
@@ -26,7 +26,6 @@
 import static java.lang.String.CASE_INSENSITIVE_ORDER;
 import static java.lang.String.format;
 
-import feign.Request.Options;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -38,28 +37,49 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.TreeMap;
 import java.util.zip.DeflaterOutputStream;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.GZIPOutputStream;
 import java.util.zip.InflaterInputStream;
+
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLSocketFactory;
 
+import feign.HttpBodyFactory.HttpBody;
+import feign.Request.Options;
+
 /** Submits HTTP {@link Request requests}. Implementations are expected to be thread-safe. */
 public interface Client {
 
   /**
    * Executes a request against its {@link Request#url() url} and returns a response.
-   *
+   * 
    * @param request safe to replay.
    * @param options options to apply to this request.
-   * @return connected response, {@link Response.Body} is absent or unread.
+   * @return connected response, {@link HttpBody} is absent or unread.
    * @throws IOException on a network error connecting to {@link Request#url()}.
    */
   Response execute(Request request, Options options) throws IOException;
 
+  class ForceRequestCloseDecoratorClient implements Client {
+	  private final Client delegate;
+
+	  public ForceRequestCloseDecoratorClient(Client delegate) {
+		this.delegate = delegate;
+	}
+
+	  // TODO: KD - get rid of null check if we can agree to not allow null bodies...
+	@Override
+	public Response execute(Request request, Options options) throws IOException {
+		Response resp = delegate.execute(request, options);
+	    Optional.ofNullable(request.httpBody()).ifPresent(HttpBody::close);
+	    return resp;
+	}
+  }
+
   class Default implements Client {
 
     private final SSLSocketFactory sslContextFactory;
@@ -129,10 +149,6 @@ Response convertResponse(HttpURLConnection connection, Request request) throws I
         }
       }
 
-      Integer length = connection.getContentLength();
-      if (length == -1) {
-        length = null;
-      }
       InputStream stream;
       if (status >= 400) {
         stream = connection.getErrorStream();
@@ -149,7 +165,7 @@ Response convertResponse(HttpURLConnection connection, Request request) throws I
           .reason(reason)
           .headers(headers)
           .request(request)
-          .body(stream, length)
+          .body(stream, connection.getContentLengthLong())
           .build();
     }
 
@@ -175,20 +191,22 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce
       connection.setInstanceFollowRedirects(options.isFollowRedirects());
       connection.setRequestMethod(request.httpMethod().name());
 
+      HttpBody body = request.httpBody();
+
       Collection<String> contentEncodingValues = request.headers().get(CONTENT_ENCODING);
       boolean gzipEncodedRequest = this.isGzip(contentEncodingValues);
       boolean deflateEncodedRequest = this.isDeflate(contentEncodingValues);
 
       boolean hasAcceptHeader = false;
-      Integer contentLength = null;
+      Long contentLength = null;
       for (String field : request.headers().keySet()) {
         if (field.equalsIgnoreCase("Accept")) {
           hasAcceptHeader = true;
         }
         for (String value : request.headers().get(field)) {
           if (field.equals(CONTENT_LENGTH)) {
             if (!gzipEncodedRequest && !deflateEncodedRequest) {
-              contentLength = Integer.valueOf(value);
+              contentLength = Long.valueOf(value);
               connection.addRequestProperty(field, value);
             }
           }
@@ -206,9 +224,7 @@ else if (field.equals(ACCEPT_ENCODING)) {
         connection.addRequestProperty("Accept", "*/*");
       }
 
-      byte[] body = request.body();
-
-      if (body != null) {
+      if (body != null && body.getLength() != 0) {
         /*
          * Ignore disableRequestBuffering flag if the empty body was set, to ensure that internal
          * retry logic applies to such requests.
@@ -228,7 +244,7 @@ else if (field.equals(ACCEPT_ENCODING)) {
           out = new DeflaterOutputStream(out);
         }
         try {
-          out.write(body);
+          Util.copy(body.asInputStream(), out);
         } finally {
           try {
             out.close();

diff --git a/core/src/main/java/feign/Feign.java b/core/src/main/java/feign/Feign.java
@@ -108,7 +108,7 @@ public Builder contract(Contract contract) {
     }
 
     public Builder client(Client client) {
-      this.client = client;
+      this.client = new Client.ForceRequestCloseDecoratorClient(client);
 
       return this;
     }

diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java
@@ -42,6 +42,8 @@ public class FeignException extends RuntimeException {
       "request should not be null";
   private static final long serialVersionUID = 0;
   private final int status;
+
+  // TODO: KD - it would probably be a lot cleaner to store HttpBody here - but there are a *ton* of places that assume byte[]...
   private byte[] responseBody;
   private Map<String, Collection<String>> responseHeaders;
   private final Request request;
@@ -177,14 +179,25 @@ public String contentUTF8() {
   }
 
   static FeignException errorReading(Request request, Response response, IOException cause) {
+	byte[] body = request.httpBody() == null ? null : new byte[0];
+	try {
+		body = Util.toByteArray(request.httpBody().peek().asInputStream());
+	} catch (IOException e) {
+		// TODO: KD - Any way to properly log this?
+	} 
+
     return new FeignException(
         response.status(),
         format("%s reading %s %s", cause.getMessage(), request.httpMethod(), request.url()),
         request,
         cause,
-        request.body(),
+        body,
         request.headers());
   }
+
+  static FeignException bodyException(String message, IOException cause) {
+	  return new FeignException(-1, message, cause);
+  }
 
   public static FeignException errorStatus(String methodKey, Response response) {
     return errorStatus(methodKey, response, null, null);
@@ -437,6 +450,7 @@ public GatewayTimeout(
     }
   }
 
+  // TODO: KD - consider passing HttpBody instead of byte[]
   private static class FeignExceptionMessageBuilder {
 
     private static final int MAX_BODY_BYTES_LENGTH = 400;
@@ -499,10 +513,7 @@ public String build() {
     }
 
     private String getBodyAsString(byte[] body, Map<String, Collection<String>> headers) {
-      Charset charset = getResponseCharset(headers);
-      if (charset == null) {
-        charset = Util.UTF_8;
-      }
+      Charset charset = Response.charsetFromHeaders(headers);
       return getResponseBody(body, charset);
     }
 
@@ -527,28 +538,29 @@ private String getResponseBodyPreview(byte[] body, Charset charset) {
       }
     }
 
-    private static Charset getResponseCharset(Map<String, Collection<String>> headers) {
-
-      Collection<String> strings = headers.get("content-type");
-      if (strings == null || strings.isEmpty()) {
-        return null;
-      }
-
-      Pattern pattern = Pattern.compile(".*charset=\"?([^\\s|^;|^\"]+).*", CASE_INSENSITIVE);
-      Matcher matcher = pattern.matcher(strings.iterator().next());
-      if (!matcher.lookingAt()) {
-        return null;
-      }
-
-      String group = matcher.group(1);
-      try {
-        if (!Charset.isSupported(group)) {
-          return null;
-        }
-      } catch (IllegalCharsetNameException ex) {
-        return null;
-      }
-      return Charset.forName(group);
-    }
+// KD - replaced with call to Response.charsetFromHeaders()...    
+//    private static Charset getResponseCharset(Map<String, Collection<String>> headers) {
+//
+//      Collection<String> strings = headers.get(Util.CONTENT_TYPE);
+//      if (strings == null || strings.isEmpty()) {
+//        return null;
+//      }
+//
+//      Pattern pattern = Pattern.compile(".*charset=\"?([^\\s|^;|^\"]+).*", CASE_INSENSITIVE);
+//      Matcher matcher = pattern.matcher(strings.iterator().next());
+//      if (!matcher.lookingAt()) {
+//        return null;
+//      }
+//
+//      String group = matcher.group(1);
+//      try {
+//        if (!Charset.isSupported(group)) {
+//          return null;
+//        }
+//      } catch (IllegalCharsetNameException ex) {
+//        return null;
+//      }
+//      return Charset.forName(group);
+//    }
   }
 }