From 1aaaa7b6c6501f48de6933d0e4506ca77e0a6a1a Mon Sep 17 00:00:00 2001 From: kevin Date: Tue, 4 Feb 2025 13:03:48 -0700 Subject: [PATCH 01/11] Centralize content-type parsing and make header name not case-sensitive. --- core/src/main/java/feign/FeignException.java | 23 ++----- core/src/main/java/feign/Response.java | 18 +----- .../java/feign/utils/ContentTypeParser.java | 62 +++++++++++++++++++ 3 files changed, 69 insertions(+), 34 deletions(-) create mode 100644 core/src/main/java/feign/utils/ContentTypeParser.java diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java index b7ea794cae..2787e88a1d 100644 --- a/core/src/main/java/feign/FeignException.java +++ b/core/src/main/java/feign/FeignException.java @@ -35,6 +35,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import feign.utils.ContentTypeParser; + /** Origin exception type for all Http Apis. */ public class FeignException extends RuntimeException { @@ -529,26 +531,9 @@ private String getResponseBodyPreview(byte[] body, Charset charset) { private static Charset getResponseCharset(Map> headers) { - Collection 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; - } + return ContentTypeParser.parseContentTypeFromHeaders(headers).getCharset().orElse(Util.UTF_8); - String group = matcher.group(1); - try { - if (!Charset.isSupported(group)) { - return null; - } - } catch (IllegalCharsetNameException ex) { - return null; - } - return Charset.forName(group); } + } } diff --git a/core/src/main/java/feign/Response.java b/core/src/main/java/feign/Response.java index 2bf64d56e2..d44c0d1755 100644 --- a/core/src/main/java/feign/Response.java +++ b/core/src/main/java/feign/Response.java @@ -18,6 +18,8 @@ import static feign.Util.*; import feign.Request.ProtocolVersion; +import feign.utils.ContentTypeParser; + import java.io.*; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; @@ -210,22 +212,8 @@ public ProtocolVersion protocolVersion() { */ public Charset charset() { - Collection contentTypeHeaders = headers().get("Content-Type"); - - if (contentTypeHeaders != null) { - for (String contentTypeHeader : contentTypeHeaders) { - String[] contentTypeParmeters = contentTypeHeader.split(";"); - if (contentTypeParmeters.length > 1) { - String[] charsetParts = contentTypeParmeters[1].split("="); - if (charsetParts.length == 2 && "charset".equalsIgnoreCase(charsetParts[0].trim())) { - String charsetString = charsetParts[1].replaceAll("\"", ""); - return Charset.forName(charsetString); - } - } - } - } + return ContentTypeParser.parseContentTypeFromHeaders(headers()).getCharset().orElse(Util.UTF_8); - return Util.UTF_8; } @Override diff --git a/core/src/main/java/feign/utils/ContentTypeParser.java b/core/src/main/java/feign/utils/ContentTypeParser.java new file mode 100644 index 0000000000..c4e008247a --- /dev/null +++ b/core/src/main/java/feign/utils/ContentTypeParser.java @@ -0,0 +1,62 @@ +package feign.utils; + +import java.nio.charset.Charset; +import java.util.Collection; +import java.util.Map; +import java.util.Optional; + +import feign.Util; + +public final class ContentTypeParser { + + private ContentTypeParser() { + } + + public static ContentTypeResult parseContentTypeFromHeaders(Map> headers) { + for(Map.Entry> entry : headers.entrySet()) { + if (entry.getKey().equalsIgnoreCase("content-type")) { + for (String val : entry.getValue()) { + return parseContentTypeHeader(val); + } + } + } + + return new ContentTypeResult("", null); + } + + public static ContentTypeResult parseContentTypeHeader(String contentTypeHeader) { + + String[] contentTypeParmeters = contentTypeHeader.split(";"); + String contentType = contentTypeParmeters[0]; + String charsetString = ""; + if (contentTypeParmeters.length > 1) { + String[] charsetParts = contentTypeParmeters[1].split("="); + if (charsetParts.length == 2 && "charset".equalsIgnoreCase(charsetParts[0].trim())) { + // TODO: KD - this doesn't really implement the full parser definition for the content-type header (esp related to quoted strings, etc...) - see https://www.w3.org/Protocols/rfc1341/4_Content-Type.html + charsetString = charsetParts[1].trim(); + if (charsetString.length() > 1 && charsetString.startsWith("\"") && charsetString.endsWith("\"")) + charsetString = charsetString.substring(1, charsetString.length()-2); + } + } + + return new ContentTypeResult(contentType, Charset.forName(charsetString, null)); + } + + public static class ContentTypeResult{ + private String contentType; + private Optional charset; + + public ContentTypeResult(String contentType, Charset charset) { + this.contentType = contentType; + this.charset = Optional.ofNullable(charset); + } + + public String getContentType() { + return contentType; + } + + public Optional getCharset() { + return charset; + } + } +} From 2496187b8303309cc2afbf870646660676769d2b Mon Sep 17 00:00:00 2001 From: kevin Date: Tue, 4 Feb 2025 13:14:58 -0700 Subject: [PATCH 02/11] Take advantage of case-insensitivity of header map --- core/src/main/java/feign/FeignException.java | 2 +- core/src/main/java/feign/RequestTemplate.java | 3 +-- core/src/main/java/feign/Response.java | 2 +- .../java/feign/utils/ContentTypeParser.java | 18 ++++++++---------- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java index 2787e88a1d..fa976779e6 100644 --- a/core/src/main/java/feign/FeignException.java +++ b/core/src/main/java/feign/FeignException.java @@ -531,7 +531,7 @@ private String getResponseBodyPreview(byte[] body, Charset charset) { private static Charset getResponseCharset(Map> headers) { - return ContentTypeParser.parseContentTypeFromHeaders(headers).getCharset().orElse(Util.UTF_8); + return ContentTypeParser.parseContentTypeFromHeaders(headers, "").getCharset().orElse(Util.UTF_8); } diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java index 5ac205e6dd..17f4dc867c 100644 --- a/core/src/main/java/feign/RequestTemplate.java +++ b/core/src/main/java/feign/RequestTemplate.java @@ -1,5 +1,4 @@ /* - * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -801,7 +800,7 @@ private RequestTemplate appendHeader(String name, Iterable values, boole this.headers.remove(name); return this; } - if (name.equals("Content-Type")) { + if (name.equalsIgnoreCase("Content-Type")) { // headers are case-insensitive // a client can only produce content of one single type, so always override Content-Type and // only add a single type this.headers.remove(name); diff --git a/core/src/main/java/feign/Response.java b/core/src/main/java/feign/Response.java index d44c0d1755..9f1367f56a 100644 --- a/core/src/main/java/feign/Response.java +++ b/core/src/main/java/feign/Response.java @@ -212,7 +212,7 @@ public ProtocolVersion protocolVersion() { */ public Charset charset() { - return ContentTypeParser.parseContentTypeFromHeaders(headers()).getCharset().orElse(Util.UTF_8); + return ContentTypeParser.parseContentTypeFromHeaders(headers(), "").getCharset().orElse(Util.UTF_8); } diff --git a/core/src/main/java/feign/utils/ContentTypeParser.java b/core/src/main/java/feign/utils/ContentTypeParser.java index c4e008247a..ecf3ac8bf4 100644 --- a/core/src/main/java/feign/utils/ContentTypeParser.java +++ b/core/src/main/java/feign/utils/ContentTypeParser.java @@ -2,26 +2,22 @@ import java.nio.charset.Charset; import java.util.Collection; +import java.util.Collections; import java.util.Map; import java.util.Optional; -import feign.Util; - public final class ContentTypeParser { private ContentTypeParser() { } - public static ContentTypeResult parseContentTypeFromHeaders(Map> headers) { - for(Map.Entry> entry : headers.entrySet()) { - if (entry.getKey().equalsIgnoreCase("content-type")) { - for (String val : entry.getValue()) { - return parseContentTypeHeader(val); - } - } + public static ContentTypeResult parseContentTypeFromHeaders(Map> headers, String ifMissing) { + // The header map *should* be a case insensitive treemap + for (String val : headers.getOrDefault("content-type", Collections.emptyList())) { + return parseContentTypeHeader(val); } - return new ContentTypeResult("", null); + return new ContentTypeResult(ifMissing, null); } public static ContentTypeResult parseContentTypeHeader(String contentTypeHeader) { @@ -43,6 +39,8 @@ public static ContentTypeResult parseContentTypeHeader(String contentTypeHeader) } public static class ContentTypeResult{ + public static final ContentTypeResult MISSING = new ContentTypeResult("", null); + private String contentType; private Optional charset; From bcc960ae56a2b76ee99c58a6b3331cbc429e147e Mon Sep 17 00:00:00 2001 From: kevin Date: Tue, 4 Feb 2025 14:12:53 -0700 Subject: [PATCH 03/11] Response body preview now based on streaming --- core/src/main/java/feign/FeignException.java | 4 +- core/src/main/java/feign/Response.java | 45 +++++++++++++++---- .../java/feign/utils/ContentTypeParser.java | 2 +- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java index fa976779e6..09d774c001 100644 --- a/core/src/main/java/feign/FeignException.java +++ b/core/src/main/java/feign/FeignException.java @@ -502,9 +502,7 @@ public String build() { private String getBodyAsString(byte[] body, Map> headers) { Charset charset = getResponseCharset(headers); - if (charset == null) { - charset = Util.UTF_8; - } + return getResponseBody(body, charset); } diff --git a/core/src/main/java/feign/Response.java b/core/src/main/java/feign/Response.java index 9f1367f56a..ab7a693ea1 100644 --- a/core/src/main/java/feign/Response.java +++ b/core/src/main/java/feign/Response.java @@ -210,6 +210,7 @@ public ProtocolVersion protocolVersion() { * See rfc7231 - Media * Type */ + // TODO: KD - RFC 7231 spec says default charset if not specified is ISO-8859-1 (at least for HTTP/1.1). Seems dangerous to assume UTF_8 here... public Charset charset() { return ContentTypeParser.parseContentTypeFromHeaders(headers(), "").getCharset().orElse(Util.UTF_8); @@ -227,9 +228,32 @@ public String toString() { builder.append(field).append(": ").append(value).append('\n'); } } - if (body != null) builder.append('\n').append(body); + if (body != null) builder.append('\n').append(bodyPreview()); return builder.toString(); } + + private String bodyPreview(){ + final int MAX_CHARS = 1024; + + try { + + char[] preview = new char[MAX_CHARS]; + Reader reader = new InputStreamReader(body.asInputStream(), charset()); + int count = reader.read(preview); + + if (count == -1) return ""; + + boolean fullBody = count < preview.length; + + String bodyPreview = new String(preview, 0, count); + + if (!fullBody) bodyPreview = bodyPreview + "... (" + body.length() + " bytes)"; + + return bodyPreview; + } catch (IOException e) { + return e + " , failed to parse response body preview"; + } + } @Override public void close() { @@ -265,15 +289,18 @@ default Reader asReader() throws IOException { /** It is the responsibility of the caller to close the stream. */ Reader asReader(Charset charset) throws IOException; + } private static final class InputStreamBody implements Response.Body { + private final int REWIND_LIMIT = 8092; private final InputStream inputStream; private final Integer length; private InputStreamBody(InputStream inputStream, Integer length) { - this.inputStream = inputStream; + this.inputStream = inputStream.markSupported() ? inputStream : new BufferedInputStream(inputStream, REWIND_LIMIT); + this.inputStream.mark(0); this.length = length; } @@ -291,30 +318,32 @@ public Integer length() { @Override public boolean isRepeatable() { - return false; + return true; } @Override - public InputStream asInputStream() { + public InputStream asInputStream() throws IOException { + inputStream.reset(); return inputStream; } @SuppressWarnings("deprecation") @Override - public Reader asReader() { - return new InputStreamReader(inputStream, UTF_8); + public Reader asReader() throws IOException { + return new InputStreamReader(asInputStream(), UTF_8); } @Override - public Reader asReader(Charset charset) { + public Reader asReader(Charset charset) throws IOException { checkNotNull(charset, "charset should not be null"); - return new InputStreamReader(inputStream, charset); + return new InputStreamReader(asInputStream(), charset); } @Override public void close() throws IOException { inputStream.close(); } + } private static final class ByteArrayBody implements Response.Body { diff --git a/core/src/main/java/feign/utils/ContentTypeParser.java b/core/src/main/java/feign/utils/ContentTypeParser.java index ecf3ac8bf4..3e87b33680 100644 --- a/core/src/main/java/feign/utils/ContentTypeParser.java +++ b/core/src/main/java/feign/utils/ContentTypeParser.java @@ -31,7 +31,7 @@ public static ContentTypeResult parseContentTypeHeader(String contentTypeHeader) // TODO: KD - this doesn't really implement the full parser definition for the content-type header (esp related to quoted strings, etc...) - see https://www.w3.org/Protocols/rfc1341/4_Content-Type.html charsetString = charsetParts[1].trim(); if (charsetString.length() > 1 && charsetString.startsWith("\"") && charsetString.endsWith("\"")) - charsetString = charsetString.substring(1, charsetString.length()-2); + charsetString = charsetString.substring(1, charsetString.length()-1); } } From 9f62a6ffca7b0e3ae5b0d38dcaf3722cd652119f Mon Sep 17 00:00:00 2001 From: kevin Date: Tue, 4 Feb 2025 14:32:57 -0700 Subject: [PATCH 04/11] Response streaming is now working --- .../main/java/feign/InvocationContext.java | 36 +++--- .../stream/InputStreamAndReaderDecoder.java | 34 +++++ .../src/test/java/feign/FeignBuilderTest.java | 2 +- core/src/test/java/feign/FeignTest.java | 118 +++++++++++++++--- 4 files changed, 149 insertions(+), 41 deletions(-) create mode 100644 core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java diff --git a/core/src/main/java/feign/InvocationContext.java b/core/src/main/java/feign/InvocationContext.java index 26f50ba897..57e4d46e24 100755 --- a/core/src/main/java/feign/InvocationContext.java +++ b/core/src/main/java/feign/InvocationContext.java @@ -18,14 +18,15 @@ import static feign.FeignException.errorReading; import static feign.Util.ensureClosed; +import java.io.Closeable; +import java.io.IOException; +import java.lang.reflect.Type; + import feign.codec.DecodeException; import feign.codec.Decoder; import feign.codec.ErrorDecoder; -import java.io.IOException; -import java.lang.reflect.Type; public class InvocationContext { - private static final long MAX_RESPONSE_BUFFER_SIZE = 8192L; private final String configKey; private final Decoder decoder; private final ErrorDecoder errorDecoder; @@ -68,9 +69,11 @@ public Response response() { public Object proceed() throws Exception { if (returnType == Response.class) { - return disconnectResponseBodyIfNeeded(response); + return response; } + boolean noClose = false; + try { final boolean shouldDecodeResponseBody = (response.status() >= 200 && response.status() < 300) @@ -86,6 +89,12 @@ public Object proceed() throws Exception { } Class rawType = Types.getRawType(returnType); + + // if the return type is closable, then it is the callers responsibility to close. + if (Closeable.class.isAssignableFrom(rawType)) { + noClose = true; + } + if (TypedResponse.class.isAssignableFrom(rawType)) { Type bodyType = Types.resolveLastTypeParameter(returnType, TypedResponse.class); return TypedResponse.builder(response).body(decode(response, bodyType)).build(); @@ -93,29 +102,12 @@ public Object proceed() throws Exception { return decode(response, returnType); } finally { - if (closeAfterDecode) { + if (closeAfterDecode && !noClose) { ensureClosed(response.body()); } } } - private static Response disconnectResponseBodyIfNeeded(Response response) throws IOException { - final boolean shouldDisconnectResponseBody = - response.body() != null - && response.body().length() != null - && response.body().length() <= MAX_RESPONSE_BUFFER_SIZE; - if (!shouldDisconnectResponseBody) { - return response; - } - - try { - final byte[] bodyData = Util.toByteArray(response.body().asInputStream()); - return response.toBuilder().body(bodyData).build(); - } finally { - ensureClosed(response.body()); - } - } - private Object decode(Response response, Type returnType) { try { return decoder.decode(response, returnType); diff --git a/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java b/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java new file mode 100644 index 0000000000..dad5555f35 --- /dev/null +++ b/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java @@ -0,0 +1,34 @@ +package feign.stream; + +import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; +import java.lang.reflect.Type; + +import feign.FeignException; +import feign.Response; +import feign.codec.DecodeException; +import feign.codec.Decoder; + +public class InputStreamAndReaderDecoder implements Decoder{ + private final Decoder delegateDecoder; + + public InputStreamAndReaderDecoder(Decoder delegate) { + this.delegateDecoder = delegate; + } + + @Override + public Object decode(Response response, Type type) throws IOException, DecodeException, FeignException { + + if (InputStream.class.equals(type)) + return response.body().asInputStream(); + + if (Reader.class.equals(type)) + return response.body().asReader(); + + if (delegateDecoder == null) return null; + + return delegateDecoder.decode(response, type); + } + +} diff --git a/core/src/test/java/feign/FeignBuilderTest.java b/core/src/test/java/feign/FeignBuilderTest.java index 18393f6fea..abf3f479ef 100644 --- a/core/src/test/java/feign/FeignBuilderTest.java +++ b/core/src/test/java/feign/FeignBuilderTest.java @@ -431,7 +431,7 @@ public InputStream asInputStream() throws IOException { @SuppressWarnings("deprecation") @Override public Reader asReader() throws IOException { - return original.body().asReader(Util.UTF_8); + return original.body().asReader(); } @Override diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java index 15d744d214..c00776bb0a 100755 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -31,21 +31,9 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import com.google.gson.Gson; -import com.google.gson.reflect.TypeToken; -import feign.Feign.ResponseMappingDecoder; -import feign.QueryMap.MapEncoder; -import feign.Request.HttpMethod; -import feign.Target.HardCodedTarget; -import feign.codec.DecodeException; -import feign.codec.Decoder; -import feign.codec.EncodeException; -import feign.codec.Encoder; -import feign.codec.ErrorDecoder; -import feign.codec.StringDecoder; -import feign.querymap.BeanQueryMapEncoder; -import feign.querymap.FieldQueryMapEncoder; import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; import java.lang.reflect.Type; import java.net.URI; import java.time.Clock; @@ -60,17 +48,36 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; +import java.util.Random; import java.util.concurrent.atomic.AtomicReference; -import okhttp3.mockwebserver.MockResponse; -import okhttp3.mockwebserver.MockWebServer; -import okhttp3.mockwebserver.SocketPolicy; -import okio.Buffer; + import org.assertj.core.data.MapEntry; import org.assertj.core.util.Maps; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentMatchers; +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; + +import feign.Feign.ResponseMappingDecoder; +import feign.QueryMap.MapEncoder; +import feign.Request.HttpMethod; +import feign.Target.HardCodedTarget; +import feign.codec.DecodeException; +import feign.codec.Decoder; +import feign.codec.EncodeException; +import feign.codec.Encoder; +import feign.codec.ErrorDecoder; +import feign.codec.StringDecoder; +import feign.querymap.BeanQueryMapEncoder; +import feign.querymap.FieldQueryMapEncoder; +import feign.stream.InputStreamAndReaderDecoder; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.SocketPolicy; +import okio.Buffer; + @SuppressWarnings("deprecation") public class FeignTest { @@ -1220,6 +1227,81 @@ void callIgnoredMethod() throws Exception { } } + @Test + void streamingResponse() throws Exception{ + byte[] expectedResponse = new byte[16184]; + new Random().nextBytes(expectedResponse); + server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse))); + + LargeStreamTestInterface api = Feign.builder() + .decoder(new InputStreamAndReaderDecoder(null)) + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try(InputStream is = api.getLargeStream()){ + byte[] out = is.readAllBytes(); + assertThat(out.length).isEqualTo(expectedResponse.length); + assertThat(out).isEqualTo(expectedResponse); + } + } + + @Test + void streamingReaderResponse() throws Exception{ + String expectedResponse = new Random().ints(1, 1500 + 1) + .limit(16184) + .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) + .toString(); + + server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))).addHeader("content-type", "text/plan; charset=utf-8")); + + LargeStreamTestInterface api = Feign.builder() + .decoder(new InputStreamAndReaderDecoder(null)) + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try(Reader r = api.getLargeReader()){ + String out = Util.toString(r); + assertThat(out.length()).isEqualTo(expectedResponse.length()); + assertThat(out).isEqualTo(expectedResponse); + } + } + + @Test + void streamingReaderResponseWithNoCharset() throws Exception{ + String expectedResponse = new Random().ints(1, 1500 + 1) + .limit(16184) + .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) + .toString(); + + server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))).addHeader("content-type", "text/plan")); + + LargeStreamTestInterface api = Feign.builder() + .decoder(new InputStreamAndReaderDecoder(null)) + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try(Reader r = api.getLargeReader()){ + String out = Util.toString(r); + assertThat(out.length()).isEqualTo(expectedResponse.length()); + assertThat(out).isEqualTo(expectedResponse); + } + } + + interface LargeStreamTestInterface { + +// @RequestLine("POST /") +// String postLargeStream(InputStream stream); +// +// @RequestLine("POST /") +// void postLargeFile(File file); + + @RequestLine("GET /") + InputStream getLargeStream(); + + @RequestLine("GET /") + Reader getLargeReader(); + + } + + + interface TestInterface { @RequestLine("POST /") From c49f15fa6a82b59de3a0178c1f2dd53150756476 Mon Sep 17 00:00:00 2001 From: kevin Date: Tue, 4 Feb 2025 14:37:05 -0700 Subject: [PATCH 05/11] Add comment about FeignException not streaming for now --- core/src/main/java/feign/FeignException.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java index 09d774c001..c79aff8582 100644 --- a/core/src/main/java/feign/FeignException.java +++ b/core/src/main/java/feign/FeignException.java @@ -38,6 +38,8 @@ import feign.utils.ContentTypeParser; /** Origin exception type for all Http Apis. */ +// TODO: KD - FeignException does not currently support streaming bodies. Usually, error responses are short enough that this isn't an issue, but we may want to eventually replace byte[] responseBody with Response.Body responseBody; +// TODO: KD - for that matter, why aren't we just capturing the response itself instead of the headers and body as separate parameters? errorReading() captures the response... public class FeignException extends RuntimeException { private static final String EXCEPTION_MESSAGE_TEMPLATE_NULL_REQUEST = From c1ad499012e7b9317a6a3c4b24ba36d56c0b46ab Mon Sep 17 00:00:00 2001 From: kevin Date: Tue, 4 Feb 2025 14:43:56 -0700 Subject: [PATCH 06/11] Remove unused imports --- core/src/main/java/feign/FeignException.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java index c79aff8582..f52c610728 100644 --- a/core/src/main/java/feign/FeignException.java +++ b/core/src/main/java/feign/FeignException.java @@ -15,9 +15,10 @@ */ package feign; -import static feign.Util.*; +import static feign.Util.UTF_8; +import static feign.Util.caseInsensitiveCopyOf; +import static feign.Util.checkNotNull; import static java.lang.String.format; -import static java.util.regex.Pattern.CASE_INSENSITIVE; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -27,13 +28,10 @@ import java.nio.ByteBuffer; import java.nio.CharBuffer; import java.nio.charset.Charset; -import java.nio.charset.IllegalCharsetNameException; import java.util.Collection; import java.util.Collections; import java.util.Map; import java.util.Optional; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import feign.utils.ContentTypeParser; From 215c06c9e6d45f0653e718e64e35ff1dbb178525 Mon Sep 17 00:00:00 2001 From: kevin Date: Fri, 7 Feb 2025 17:07:05 -0700 Subject: [PATCH 07/11] Request streaming is now functional --- core/src/main/java/feign/Client.java | 70 +++--- core/src/main/java/feign/Logger.java | 12 +- core/src/main/java/feign/Request.java | 206 ++++++++++-------- core/src/main/java/feign/RequestTemplate.java | 128 ++++++++--- core/src/main/java/feign/Util.java | 2 +- .../stream/InputStreamAndFileEncoder.java | 71 ++++++ core/src/test/java/feign/ClientTest.java | 6 +- core/src/test/java/feign/FeignTest.java | 82 +------ .../feign/assertj/RequestTemplateAssert.java | 4 +- .../stream/InputStreamAndFileEncoderTest.java | 151 +++++++++++++ .../InputStreamAndReaderDecoderTest.java | 88 ++++++++ 11 files changed, 573 insertions(+), 247 deletions(-) create mode 100644 core/src/main/java/feign/stream/InputStreamAndFileEncoder.java create mode 100644 core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java create mode 100644 core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java diff --git a/core/src/main/java/feign/Client.java b/core/src/main/java/feign/Client.java index 3c3d048c29..9211f51e3d 100644 --- a/core/src/main/java/feign/Client.java +++ b/core/src/main/java/feign/Client.java @@ -180,7 +180,7 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce 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; @@ -188,7 +188,7 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce 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); } } @@ -201,47 +201,47 @@ else if (field.equals(ACCEPT_ENCODING)) { } } } + // Some servers choke on the default accept string. if (!hasAcceptHeader) { connection.addRequestProperty("Accept", "*/*"); } - byte[] body = request.body(); - - if (body != null) { - /* - * Ignore disableRequestBuffering flag if the empty body was set, to ensure that internal - * retry logic applies to such requests. - */ - if (disableRequestBuffering) { - if (contentLength != null) { - connection.setFixedLengthStreamingMode(contentLength); - } else { - connection.setChunkedStreamingMode(8196); - } - } - connection.setDoOutput(true); - OutputStream out = connection.getOutputStream(); - if (gzipEncodedRequest) { - out = new GZIPOutputStream(out); - } else if (deflateEncodedRequest) { - out = new DeflaterOutputStream(out); - } - try { - out.write(body); - } finally { - try { - out.close(); - } catch (IOException suppressed) { // NOPMD + if (request.hasBody()) { + /* + * Ignore disableRequestBuffering flag if the empty body was set, to ensure that internal + * retry logic applies to such requests. + */ + if (disableRequestBuffering) { + if (contentLength != null) { + connection.setFixedLengthStreamingMode(contentLength); + } else { + connection.setChunkedStreamingMode(8196); + } + } + connection.setDoOutput(true); + OutputStream out = connection.getOutputStream(); + if (gzipEncodedRequest) { + out = new GZIPOutputStream(out); + } else if (deflateEncodedRequest) { + out = new DeflaterOutputStream(out); + } + try { + request.sendBodyToOutputStream(out); + } finally { + try { + out.close(); + } catch (IOException suppressed) { // NOPMD + } + } + } else { + if (request.httpMethod().isWithBody()) { + // To use this Header, set 'sun.net.http.allowRestrictedHeaders' property true. + connection.addRequestProperty("Content-Length", "0"); } - } - } - - if (body == null && request.httpMethod().isWithBody()) { - // To use this Header, set 'sun.net.http.allowRestrictedHeaders' property true. - connection.addRequestProperty("Content-Length", "0"); } + return connection; } diff --git a/core/src/main/java/feign/Logger.java b/core/src/main/java/feign/Logger.java index 4dcb5f0d48..cf66e4ea48 100644 --- a/core/src/main/java/feign/Logger.java +++ b/core/src/main/java/feign/Logger.java @@ -78,17 +78,15 @@ protected void logRequest(String configKey, Level logLevel, Request request) { } } - int bodyLength = 0; - if (request.body() != null) { - bodyLength = request.length(); + if (request.hasBody()) { if (logLevel.ordinal() >= Level.FULL.ordinal()) { - String bodyText = - request.charset() != null ? new String(request.body(), request.charset()) : null; log(configKey, ""); // CRLF - log(configKey, "%s", bodyText != null ? bodyText : "Binary data"); + log(configKey, request.bodyAsString()); } + log(configKey, "---> END HTTP"); + } else { + log(configKey, "---> END HTTP (no body)"); } - log(configKey, "---> END HTTP (%s-byte body)", bodyLength); } } diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index a3627a164d..f3d3c3ffc8 100644 --- a/core/src/main/java/feign/Request.java +++ b/core/src/main/java/feign/Request.java @@ -15,11 +15,18 @@ */ package feign; +import static feign.Util.CONTENT_LENGTH; import static feign.Util.checkNotNull; import static feign.Util.getThreadIdentifier; import static feign.Util.valuesOrEmpty; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.io.OutputStreamWriter; import java.io.Serializable; +import java.io.StringWriter; +import java.io.Writer; import java.net.HttpURLConnection; import java.nio.charset.Charset; import java.time.Duration; @@ -28,10 +35,15 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; +import feign.Request.OutputStreamSender; +import feign.Request.WriterSender; +import feign.utils.ContentTypeParser; + /** An immutable request to an http server. */ public final class Request implements Serializable { @@ -83,24 +95,6 @@ public String toString() { } } - /** - * No parameters can be null except {@code body} and {@code charset}. All parameters must be - * effectively immutable, via safe copies, not mutating or otherwise. - * - * @deprecated {@link #create(HttpMethod, String, Map, byte[], Charset)} - */ - @Deprecated - public static Request create( - String method, - String url, - Map> headers, - byte[] body, - Charset charset) { - checkNotNull(method, "httpMethod of %s", method); - final HttpMethod httpMethod = HttpMethod.valueOf(method.toUpperCase()); - return create(httpMethod, url, headers, body, charset, null); - } - /** * Builds a Request. All parameters must be effectively immutable, via safe copies. * @@ -116,13 +110,14 @@ public static Request create( HttpMethod httpMethod, String url, Map> headers, - byte[] body, + byte[] bodyBytes, Charset charset) { - return create(httpMethod, url, headers, Body.create(body, charset), null); + return create(httpMethod, url, headers, Body.create(bodyBytes), (RequestTemplate)null); } - + /** * Builds a Request. All parameters must be effectively immutable, via safe copies. + * Used for unit testing only * * @param httpMethod for the request. * @param url for the request. @@ -132,14 +127,14 @@ public static Request create( * @return a Request */ public static Request create( - HttpMethod httpMethod, - String url, - Map> headers, - byte[] body, - Charset charset, - RequestTemplate requestTemplate) { - return create(httpMethod, url, headers, Body.create(body, charset), requestTemplate); - } + HttpMethod httpMethod, + String url, + Map> headers, + byte[] bodyBytes, + Charset charset, + RequestTemplate requestTemplate) { + return create(httpMethod, url, headers, Body.create(bodyBytes), requestTemplate); + } /** * Builds a Request. All parameters must be effectively immutable, via safe copies. @@ -158,7 +153,7 @@ public static Request create( RequestTemplate requestTemplate) { return new Request(httpMethod, url, headers, body, requestTemplate); } - + private final HttpMethod httpMethod; private final String url; private final Map> headers; @@ -253,30 +248,55 @@ public void header(String key, Collection values) { * @return the current character set for the request, may be {@literal null} for binary data. */ public Charset charset() { - return body.encoding; + return ContentTypeParser.parseContentTypeFromHeaders(headers, "Unknown/Unknown").getCharset().orElse(null); } + public boolean hasBody() { + return body != null; + } + + /** + * @return A string representation of the body as would be written to logs (this is NOT guaranteed to be an exact replica of the request body) + */ + public String bodyAsString() { + return body.asString(); + } + /** * If present, this is the replayable body to send to the server. In some cases, this may be * interpretable as text. * * @see #charset() + * @deprecated use {@link #sendBodyToOutputStream(OutputStream)} */ + @Deprecated public byte[] body() { - return body.data; + System.err.println("Deprecated method called"); + if (body == null) return null; + try { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + body.sendToOutputStream(baos); + return baos.toByteArray(); + } catch (IOException e) { + throw new FeignException(-1, e.getMessage(), e); + } } - public boolean isBinary() { - return body.isBinary(); + public void sendBodyToOutputStream(OutputStream os) throws IOException { + body.sendToOutputStream(os); } /** * Request Length. * - * @return size of the request body. + * @return size of the request body (using the Content-Length header value) if specified, or -1 if unknown */ public int length() { - return this.body.length(); + for (String value : headers().get(CONTENT_LENGTH)) { + return Integer.valueOf(value); + } + + return -1; } /** @@ -507,6 +527,14 @@ public RequestTemplate requestTemplate() { return this.requestTemplate; } + public static interface OutputStreamSender{ + public void sendToOutputStream(OutputStream os) throws IOException; + } + + public static interface WriterSender{ + public void sendToWriter(Writer w) throws IOException; + } + /** * Request Body * @@ -515,76 +543,66 @@ public RequestTemplate requestTemplate() { @Experimental public static class Body implements Serializable { - private transient Charset encoding; - - private byte[] data; - - private Body() { - super(); - } - - private Body(byte[] data) { - this.data = data; - } - - private Body(byte[] data, Charset encoding) { - this.data = data; - this.encoding = encoding; + private static final Body EMPTY = new Body(os -> {}, "-- EMPTY BODY --"); + + private final OutputStreamSender streamSender; + + /** + * The string representation of this body as it will be displayed in log output or toString() methods + */ + private final String stringRepresentation; + + protected Body(OutputStreamSender streamSender, String stringRepresentation) { + this.streamSender = streamSender; + this.stringRepresentation = stringRepresentation; } - public Optional getEncoding() { - return Optional.ofNullable(this.encoding); - } - - public int length() { - /* calculate the content length based on the data provided */ - return data != null ? data.length : 0; - } - - public byte[] asBytes() { - return data; + public void sendToOutputStream(OutputStream os) throws IOException{ + streamSender.sendToOutputStream(os); } public String asString() { - return !isBinary() ? new String(data, encoding) : "Binary data"; - } - - public boolean isBinary() { - return encoding == null || data == null; - } - - public static Body create(String data) { - return new Body(data.getBytes()); + return stringRepresentation; } public static Body create(String data, Charset charset) { - return new Body(data.getBytes(charset), charset); + Objects.requireNonNull(data); + Objects.requireNonNull(charset); + + return createForWriterSender(w -> w.write(data), charset, data); } public static Body create(byte[] data) { - return new Body(data); - } - - public static Body create(byte[] data, Charset charset) { - return new Body(data, charset); - } - - /** - * Creates a new Request Body with charset encoded data. - * - * @param data to be encoded. - * @param charset to encode the data with. if {@literal null}, then data will be considered - * binary and will not be encoded. - * @return a new Request.Body instance with the encoded data. - * @deprecated please use {@link Request.Body#create(byte[], Charset)} - */ - @Deprecated - public static Body encoded(byte[] data, Charset charset) { - return create(data, charset); - } - + // Compatibility with a zillion unit tests that use a null byte[] to indicate no body + if (data == null) return null; + + return createForOutputStreamSender( + os -> os.write(data), + "Binary data (" + data.length + " bytes)" + ); + } + + public static Body createForWriterSender(WriterSender sender, Charset charset, String stringRep) { + Objects.requireNonNull(charset); + + OutputStreamSender osSender = os -> { + try(Writer osw = new OutputStreamWriter(os, charset)){ + sender.sendToWriter( osw ); + } + }; + + return createForOutputStreamSender( + osSender, + stringRep + ); + } + + public static Body createForOutputStreamSender(OutputStreamSender sender, String stringRep) { + return new Request.Body(sender, stringRep); + } + public static Body empty() { - return new Body(); + return EMPTY; } } } diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java index 17f4dc867c..011522a383 100644 --- a/core/src/main/java/feign/RequestTemplate.java +++ b/core/src/main/java/feign/RequestTemplate.java @@ -17,13 +17,10 @@ import static feign.Util.CONTENT_LENGTH; import static feign.Util.checkNotNull; -import feign.Request.HttpMethod; -import feign.template.BodyTemplate; -import feign.template.HeaderTemplate; -import feign.template.QueryTemplate; -import feign.template.UriTemplate; -import feign.template.UriUtils; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.Serializable; +import java.io.StringWriter; import java.net.URI; import java.nio.charset.Charset; import java.util.AbstractMap.SimpleImmutableEntry; @@ -36,12 +33,23 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Map.Entry; import java.util.TreeMap; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import feign.Request.HttpMethod; +import feign.Request.OutputStreamSender; +import feign.Request.WriterSender; +import feign.template.BodyTemplate; +import feign.template.HeaderTemplate; +import feign.template.QueryTemplate; +import feign.template.UriTemplate; +import feign.template.UriUtils; +import feign.utils.ContentTypeParser; + /** * Request Builder for an HTTP Target. * @@ -61,7 +69,7 @@ public final class RequestTemplate implements Serializable { private BodyTemplate bodyTemplate; private HttpMethod method; private transient Charset charset = Util.UTF_8; - private Request.Body body = Request.Body.empty(); + private Request.Body body; private boolean decodeSlash = true; private CollectionFormat collectionFormat = CollectionFormat.EXPLODED; private MethodMetadata methodMetadata; @@ -99,6 +107,8 @@ private RequestTemplate( CollectionFormat collectionFormat, MethodMetadata methodMetadata, Target feignTarget) { + + this(); this.target = target; this.fragment = fragment; this.uriTemplate = uriTemplate; @@ -287,6 +297,7 @@ public Request request() { if (!this.resolved) { throw new IllegalStateException("template has not been resolved."); } + return Request.create(this.method, this.url(), this.headers(), this.body, this); } @@ -864,44 +875,87 @@ public Map> headers() { * Sets the Body and Charset for this request. * * @param data to send, can be null. - * @param charset of the encoded data. + * @param charset of the encoded data. Ignored (see deprecation note) * @return a RequestTemplate for chaining. + * @deprecated Charset always comes from the content-type header. Use {@link #body(byte[])} */ + @Deprecated public RequestTemplate body(byte[] data, Charset charset) { - this.body(Request.Body.create(data, charset)); - return this; + return body(data); } /** - * Set the Body for this request. Charset is assumed to be UTF_8. Data must be encoded. + * Sets the body output to a fixed byte[] + * @param data the data for the body + * @return a RequestTemplate for chaining. + */ + public RequestTemplate body(byte[] data) { + this.body(Request.Body.create(data)); + this.header(CONTENT_LENGTH, Integer.toString(data.length) ); + return this; + } + + /** + * Set the Body for this request. The content length header will be set to the length of the encoded string. + * Encoding is done using the {@link #requestCharset()} * * @param bodyText to send. * @return a RequestTemplate for chaining. */ public RequestTemplate body(String bodyText) { - this.body(Request.Body.create(bodyText.getBytes(this.charset), this.charset)); + + this.body(Request.Body.create(bodyText, requestCharset()) ); + this.header(CONTENT_LENGTH, Integer.toString(bodyText.getBytes(requestCharset()).length) ); return this; } /** - * Set the Body for this request. + * Sets the sender for the body content using the request template's content-type charset encoding (or utf-8 if charset is not specified) + * @param sender the sender that will generate the body content + * @param stringRepresentation the string that will be used for logging and toString methods. If null, the request will be read from the sender, which will cause downstream failures if the sender cannot be re-used, so be sure to set this for streaming requests. + * @return a RequestTemplate for chaining. + */ + public RequestTemplate bodyWriterSender(WriterSender sender, String stringRepresentation) { + + if (stringRepresentation == null) { + try(StringWriter writer = new StringWriter()) { + sender.sendToWriter(writer); + stringRepresentation = sender.toString(); + } catch (IOException e) { + stringRepresentation = "-- Unable to determine body: " + e.getMessage() + " --"; + } + + } + + return body( + Request.Body.createForWriterSender(sender, requestCharset(), stringRepresentation) + ); + + } + + public RequestTemplate bodyOutputStreamSender(OutputStreamSender sender, String stringRepresentation) { + return body( + Request.Body.createForOutputStreamSender(sender, stringRepresentation) + ); + } + + /** + * Set the Body for this request and clear the content-length header. Callers can set the content-length after calling this if they are able to compute the length. * * @param body to send. * @return a RequestTemplate for chaining. - * @deprecated use {@link #body(byte[], Charset)} instead. + * @deprecated this method will eventually be changed to private. Use the various public body(...) methods instead. */ @Deprecated public RequestTemplate body(Request.Body body) { this.body = body; - /* body template must be cleared to prevent double processing */ + // Enforce invariant: bodyTemplate or body must be set, but not both this.bodyTemplate = null; - header(CONTENT_LENGTH, Collections.emptyList()); - if (body.length() > 0) { - header(CONTENT_LENGTH, String.valueOf(body.length())); - } - + // remove the content length header to indicate streaming is required, callers can set it again if they are able to compute the length + this.header(CONTENT_LENGTH); + return this; } @@ -910,20 +964,35 @@ public RequestTemplate body(Request.Body body) { * * @return the currently applied Charset. */ + + public Charset requestCharset() { - if (this.body != null) { - return this.body.getEncoding().orElse(this.charset); - } - return this.charset; + + // KD - Note that this is the charset of the request body - *not* the 'charset' field of this RequestTemplate object. + // As near as I can tell, the RequestTemplate#charset field isn't actually necessary except for feeding UriTemplate and QueryTemplate + // and neither of those benefit from an externally supplied charset. At the end of the day, all of these calls terminate at UriUtils#encodeChunk, + // which encodes, processes bytes, then immediately decodes. Any encoding would work. + + return ContentTypeParser.parseContentTypeFromHeaders(headers(), "Unknown/Unknown").getCharset().orElse(Util.UTF_8); + } /** * The Request Body. * - * @return the request body. + * @implNote This should not be called unless the request body is resettable (e.g. the body was set using {@link #body(byte[])} or {@link #body(String)} ) + * + * @return the request body as a byte array */ public byte[] body() { - return body.asBytes(); + if (this.body == null) return null; + try { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + this.body.sendToOutputStream(baos); + return baos.toByteArray(); + } catch (IOException e) { + throw new FeignException(-1, e.getMessage(), e); + } } /** @@ -944,8 +1013,7 @@ public Request.Body requestBody() { * @return a RequestTemplate for chaining. */ public RequestTemplate bodyTemplate(String bodyTemplate) { - this.bodyTemplate = BodyTemplate.create(bodyTemplate, this.charset); - return this; + return bodyTemplate(bodyTemplate, this.charset); } /** @@ -955,8 +1023,10 @@ public RequestTemplate bodyTemplate(String bodyTemplate) { * @return a RequestTemplate for chaining. */ public RequestTemplate bodyTemplate(String bodyTemplate, Charset charset) { + + // Enforce invariant: bodyTemplate or body must be set, but not both this.bodyTemplate = BodyTemplate.create(bodyTemplate, charset); - this.charset = charset; + this.body = null; return this; } diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java index 99b56683df..11d2ab0cc5 100644 --- a/core/src/main/java/feign/Util.java +++ b/core/src/main/java/feign/Util.java @@ -265,7 +265,7 @@ public static byte[] toByteArray(InputStream in) throws IOException { } /** Adapted from {@code com.google.common.io.ByteStreams.copy()}. */ - private static long copy(InputStream from, OutputStream to) throws IOException { + public static long copy(InputStream from, OutputStream to) throws IOException { checkNotNull(from, "from"); checkNotNull(to, "to"); byte[] buf = new byte[BUF_SIZE]; diff --git a/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java b/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java new file mode 100644 index 0000000000..9eb550abcb --- /dev/null +++ b/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java @@ -0,0 +1,71 @@ +package feign.stream; + +import static java.lang.String.format; + +import java.io.BufferedInputStream; +import java.io.File; +import java.io.InputStream; +import java.lang.reflect.Type; +import java.nio.file.Files; + +import feign.Request.OutputStreamSender; +import feign.RequestTemplate; +import feign.Util; +import feign.codec.EncodeException; +import feign.codec.Encoder; + +public class InputStreamAndFileEncoder implements Encoder { + + private final Encoder delegateEncoder; + + public InputStreamAndFileEncoder(Encoder delegateEncoder) { + this.delegateEncoder = delegateEncoder; + } + + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + if (bodyType instanceof Class) { + Class bodyClass = (Class)bodyType; + if (InputStream.class.isAssignableFrom(bodyClass)) { + + // Support some degree of retry - if the stream is too long, then any retry will throw + final int BUFSIZE = 8092; + + InputStream is = (InputStream)object; + InputStream streamToSend = is.markSupported() ? is : new BufferedInputStream(is, BUFSIZE); + streamToSend.mark(BUFSIZE); + + OutputStreamSender sender = os -> { + streamToSend.reset(); + Util.copy(streamToSend, os); + }; + + template.bodyOutputStreamSender(sender, "-- Binary Data (Unknown Length) --"); + + return; + } + + if (File.class.isAssignableFrom(bodyClass)) { + File file = (File)object; + + if (!file.isFile()) + throw new EncodeException(format("Unable to encode missing file - %s", file)); + + template.bodyOutputStreamSender(os -> Files.copy(file.toPath(), os), "-- Content of " + file + " (" + file.length() + " bytes) --"); + template.header(Util.CONTENT_LENGTH, Long.toString(file.length())); + + return; + } + + } + + + if (delegateEncoder != null) { + delegateEncoder.encode(object, bodyType, template); + return; + } + + throw new EncodeException(format("%s is not a type supported by this encoder.", object.getClass())); + + } +} diff --git a/core/src/test/java/feign/ClientTest.java b/core/src/test/java/feign/ClientTest.java index 8f6a072b70..8fd9900f70 100644 --- a/core/src/test/java/feign/ClientTest.java +++ b/core/src/test/java/feign/ClientTest.java @@ -61,9 +61,8 @@ void testConvertAndSendWithAcceptEncoding() throws IOException { headers.put(Util.ACCEPT_ENCODING, acceptEncoding); RequestTemplate requestTemplate = mock(RequestTemplate.class); - Request.Body body = mock(Request.Body.class); + Request.Body body = null; Request.Options options = mock(Request.Options.class); - Client client = mock(Client.class); Request request = Request.create( @@ -82,9 +81,8 @@ void testConvertAndSendWithContentLength() throws IOException { headers.put(Util.CONTENT_LENGTH, Collections.singletonList("100")); RequestTemplate requestTemplate = mock(RequestTemplate.class); - Request.Body body = mock(Request.Body.class); + Request.Body body = null; Request.Options options = mock(Request.Options.class); - Client client = mock(Client.class); Request request = Request.create( diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java index c00776bb0a..c18b8b184e 100755 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -31,11 +31,16 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import java.io.BufferedInputStream; +import java.io.ByteArrayInputStream; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.Reader; import java.lang.reflect.Type; import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; import java.time.Clock; import java.time.Instant; import java.time.ZoneId; @@ -55,6 +60,7 @@ import org.assertj.core.util.Maps; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.mockito.ArgumentMatchers; import com.google.gson.Gson; @@ -72,6 +78,7 @@ import feign.codec.StringDecoder; import feign.querymap.BeanQueryMapEncoder; import feign.querymap.FieldQueryMapEncoder; +import feign.stream.InputStreamAndFileEncoder; import feign.stream.InputStreamAndReaderDecoder; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; @@ -1227,81 +1234,6 @@ void callIgnoredMethod() throws Exception { } } - @Test - void streamingResponse() throws Exception{ - byte[] expectedResponse = new byte[16184]; - new Random().nextBytes(expectedResponse); - server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse))); - - LargeStreamTestInterface api = Feign.builder() - .decoder(new InputStreamAndReaderDecoder(null)) - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - try(InputStream is = api.getLargeStream()){ - byte[] out = is.readAllBytes(); - assertThat(out.length).isEqualTo(expectedResponse.length); - assertThat(out).isEqualTo(expectedResponse); - } - } - - @Test - void streamingReaderResponse() throws Exception{ - String expectedResponse = new Random().ints(1, 1500 + 1) - .limit(16184) - .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) - .toString(); - - server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))).addHeader("content-type", "text/plan; charset=utf-8")); - - LargeStreamTestInterface api = Feign.builder() - .decoder(new InputStreamAndReaderDecoder(null)) - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - try(Reader r = api.getLargeReader()){ - String out = Util.toString(r); - assertThat(out.length()).isEqualTo(expectedResponse.length()); - assertThat(out).isEqualTo(expectedResponse); - } - } - - @Test - void streamingReaderResponseWithNoCharset() throws Exception{ - String expectedResponse = new Random().ints(1, 1500 + 1) - .limit(16184) - .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) - .toString(); - - server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))).addHeader("content-type", "text/plan")); - - LargeStreamTestInterface api = Feign.builder() - .decoder(new InputStreamAndReaderDecoder(null)) - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - try(Reader r = api.getLargeReader()){ - String out = Util.toString(r); - assertThat(out.length()).isEqualTo(expectedResponse.length()); - assertThat(out).isEqualTo(expectedResponse); - } - } - - interface LargeStreamTestInterface { - -// @RequestLine("POST /") -// String postLargeStream(InputStream stream); -// -// @RequestLine("POST /") -// void postLargeFile(File file); - - @RequestLine("GET /") - InputStream getLargeStream(); - - @RequestLine("GET /") - Reader getLargeReader(); - - } - - - interface TestInterface { @RequestLine("POST /") diff --git a/core/src/test/java/feign/assertj/RequestTemplateAssert.java b/core/src/test/java/feign/assertj/RequestTemplateAssert.java index 0a571ebb36..6f8a6e151c 100644 --- a/core/src/test/java/feign/assertj/RequestTemplateAssert.java +++ b/core/src/test/java/feign/assertj/RequestTemplateAssert.java @@ -73,8 +73,8 @@ public RequestTemplateAssert hasBody(byte[] expected) { public RequestTemplateAssert hasBodyTemplate(String expected) { isNotNull(); - if (actual.body() != null) { - failWithMessage("\nExpecting body to be null, but was:<%s>", actual.bodyTemplate()); + if (actual.requestBody() != null) { + failWithMessage("\nExpecting requestBody to be null, but was:<%s>", actual.requestBody()); } objects.assertEqual(info, actual.bodyTemplate(), expected); return this; diff --git a/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java b/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java new file mode 100644 index 0000000000..df9ec97ec6 --- /dev/null +++ b/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java @@ -0,0 +1,151 @@ +package feign.stream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.BufferedInputStream; +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Type; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Random; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import feign.Feign; +import feign.FeignException; +import feign.Request.HttpMethod; +import feign.RequestLine; +import feign.Response; +import feign.RetryableException; +import feign.codec.StringDecoder; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; + +public class InputStreamAndFileEncoderTest { + public final MockWebServer server = new MockWebServer(); + private static final Long NON_RETRYABLE = null; + + interface LargeStreamTestInterface { + + @RequestLine("POST /") + String postLargeStream(InputStream stream); + + @RequestLine("POST /") + void postLargeFile(File file); + + } + + @Test + void streamingRequest() throws Exception{ + server.enqueue(new MockResponse()); + + byte[] expectedResponse = new byte[16184]; + new Random().nextBytes(expectedResponse); + + + LargeStreamTestInterface api = Feign.builder() + .encoder(new InputStreamAndFileEncoder(null)) + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try(InputStream is = new ByteArrayInputStream(expectedResponse)){ + api.postLargeStream(is); + } + + byte[] result = server.takeRequest().getBody().readByteArray(); + + assertThat(result.length).isEqualByComparingTo(expectedResponse.length); + assertThat(result).isEqualTo(expectedResponse); + } + + Feign.Builder createRetryableFeignBuilder(){ + return Feign.builder() + .encoder(new InputStreamAndFileEncoder(null)) + .decoder( + new StringDecoder() { + @Override + public Object decode(Response response, Type type) throws IOException { + String string = super.decode(response, type).toString(); + if ("retry!".equals(string)) { + throw new RetryableException( + response.status(), + string, + HttpMethod.POST, + NON_RETRYABLE, + response.request()); + } + return string; + } + }); + } + + @Test + void streamingRequestCanRetry() throws Exception { + + server.enqueue(new MockResponse().setBody("retry!")); + server.enqueue(new MockResponse().setBody("success!")); + + LargeStreamTestInterface api = createRetryableFeignBuilder() + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + byte[] requestData = new byte[16184]; + new Random().nextBytes(requestData); + + // if we use a plain ByteArrayInputStream, then it is infinitely resettable, so the following will retry + // if instead we wrapped this in a reset limited inputstream (like a BufferedInputStream with size set to 1024), retry would fail. + // As of now, I don't see a way to tell the server to read *some* of the request and then fail, so we aren't getting good test coverage of the partial request reset scenario + String rslt = api.postLargeStream(new ByteArrayInputStream(requestData)); + byte[] dataReceivedByServer = server.takeRequest().getBody().readByteArray(); + + assertThat(rslt).isEqualTo("success!"); + assertThat(server.getRequestCount()).isEqualTo(2); + assertThat(dataReceivedByServer).isEqualTo(requestData); + } + + @Test + void streamingRequestRetryFailsIfTooMuchDataRead() throws Exception { + + server.enqueue(new MockResponse().setBody("retry!")); + server.enqueue(new MockResponse().setBody("success!")); + + LargeStreamTestInterface api = createRetryableFeignBuilder() + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + byte[] requestData = new byte[16184]; + new Random().nextBytes(requestData); + + // if we wrap the inputstream in a reset limited inputstream like a BufferedInputStream, retry will fail + // As of now, I don't see a way to tell the server to read *some* of the request and then fail, so we aren't getting good test coverage of the partial request reset scenario + FeignException e = assertThrows(FeignException.class, () -> api.postLargeStream(new BufferedInputStream(new ByteArrayInputStream(requestData), 1024)) ); + + assertThat(e).hasCauseInstanceOf(IOException.class); + } + + @Test + void streamingFileRequest(@TempDir Path tempPath) throws Exception{ + server.enqueue(new MockResponse()); + + byte[] expectedResponse = new byte[16184]; + new Random().nextBytes(expectedResponse); + + Path fileToSend = tempPath.resolve("temp.dat"); + + Files.write(fileToSend, expectedResponse); + + LargeStreamTestInterface api = Feign.builder() + .encoder(new InputStreamAndFileEncoder(null)) + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + api.postLargeFile(fileToSend.toFile()); + + byte[] result = server.takeRequest().getBody().readByteArray(); + + assertThat(result.length).isEqualByComparingTo(expectedResponse.length); + assertThat(result).isEqualTo(expectedResponse); + } + +} diff --git a/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java b/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java new file mode 100644 index 0000000000..7fb291e802 --- /dev/null +++ b/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java @@ -0,0 +1,88 @@ +package feign.stream; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.InputStream; +import java.io.Reader; +import java.util.Random; + +import org.junit.jupiter.api.Test; + +import feign.Feign; +import feign.RequestLine; +import feign.Util; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okio.Buffer; + +public class InputStreamAndReaderDecoderTest { + public final MockWebServer server = new MockWebServer(); + + interface LargeStreamTestInterface { + + @RequestLine("GET /") + InputStream getLargeStream(); + + @RequestLine("GET /") + Reader getLargeReader(); + + } + + @Test + void streamingResponse() throws Exception{ + byte[] expectedResponse = new byte[16184]; + new Random().nextBytes(expectedResponse); + server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse))); + + LargeStreamTestInterface api = Feign.builder() + .decoder(new InputStreamAndReaderDecoder(null)) + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try(InputStream is = api.getLargeStream()){ + byte[] out = is.readAllBytes(); + assertThat(out.length).isEqualTo(expectedResponse.length); + assertThat(out).isEqualTo(expectedResponse); + } + } + + @Test + void streamingReaderResponse() throws Exception{ + String expectedResponse = new Random().ints(1, 1500 + 1) + .limit(16184) + .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) + .toString(); + + server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))).addHeader("content-type", "text/plan; charset=utf-8")); + + LargeStreamTestInterface api = Feign.builder() + .decoder(new InputStreamAndReaderDecoder(null)) + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try(Reader r = api.getLargeReader()){ + String out = Util.toString(r); + assertThat(out.length()).isEqualTo(expectedResponse.length()); + assertThat(out).isEqualTo(expectedResponse); + } + } + + @Test + void streamingReaderResponseWithNoCharset() throws Exception{ + String expectedResponse = new Random().ints(1, 1500 + 1) + .limit(16184) + .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) + .toString(); + + server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))).addHeader("content-type", "text/plan")); + + LargeStreamTestInterface api = Feign.builder() + .decoder(new InputStreamAndReaderDecoder(null)) + .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try(Reader r = api.getLargeReader()){ + String out = Util.toString(r); + assertThat(out.length()).isEqualTo(expectedResponse.length()); + assertThat(out).isEqualTo(expectedResponse); + } + } + +} From 456a0fda39744e3641a713b8af8a7b557698461c Mon Sep 17 00:00:00 2001 From: kevin Date: Fri, 7 Feb 2025 17:17:30 -0700 Subject: [PATCH 08/11] Remove syserr output note --- core/src/main/java/feign/Request.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index f3d3c3ffc8..5e49248816 100644 --- a/core/src/main/java/feign/Request.java +++ b/core/src/main/java/feign/Request.java @@ -271,10 +271,10 @@ public String bodyAsString() { */ @Deprecated public byte[] body() { - System.err.println("Deprecated method called"); + // Anyone calling this method is circumventing the streaming capabilities + //System.err.println("Deprecated method called"); if (body == null) return null; - try { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try(ByteArrayOutputStream baos = new ByteArrayOutputStream()) { body.sendToOutputStream(baos); return baos.toByteArray(); } catch (IOException e) { From 1c49c0a1a7ae2a4e7127ab83fdfe31aa04efe8bd Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Sun, 9 Feb 2025 10:37:45 -0300 Subject: [PATCH 09/11] Fix code format --- core/src/main/java/feign/Client.java | 63 ++-- core/src/main/java/feign/FeignException.java | 16 +- .../main/java/feign/InvocationContext.java | 17 +- core/src/main/java/feign/Logger.java | 2 +- core/src/main/java/feign/Request.java | 161 +++++----- core/src/main/java/feign/RequestTemplate.java | 166 +++++----- core/src/main/java/feign/Response.java | 64 ++-- .../stream/InputStreamAndFileEncoder.java | 127 ++++---- .../stream/InputStreamAndReaderDecoder.java | 54 ++-- .../java/feign/utils/ContentTypeParser.java | 115 ++++--- core/src/test/java/feign/FeignTest.java | 50 ++- .../stream/InputStreamAndFileEncoderTest.java | 288 ++++++++++-------- .../InputStreamAndReaderDecoderTest.java | 174 ++++++----- 13 files changed, 698 insertions(+), 599 deletions(-) diff --git a/core/src/main/java/feign/Client.java b/core/src/main/java/feign/Client.java index 9211f51e3d..76887f2af0 100644 --- a/core/src/main/java/feign/Client.java +++ b/core/src/main/java/feign/Client.java @@ -201,47 +201,46 @@ else if (field.equals(ACCEPT_ENCODING)) { } } } - + // Some servers choke on the default accept string. if (!hasAcceptHeader) { connection.addRequestProperty("Accept", "*/*"); } if (request.hasBody()) { - /* - * Ignore disableRequestBuffering flag if the empty body was set, to ensure that internal - * retry logic applies to such requests. - */ - if (disableRequestBuffering) { - if (contentLength != null) { - connection.setFixedLengthStreamingMode(contentLength); - } else { - connection.setChunkedStreamingMode(8196); - } - } - connection.setDoOutput(true); - OutputStream out = connection.getOutputStream(); - if (gzipEncodedRequest) { - out = new GZIPOutputStream(out); - } else if (deflateEncodedRequest) { - out = new DeflaterOutputStream(out); - } - try { - request.sendBodyToOutputStream(out); - } finally { - try { - out.close(); - } catch (IOException suppressed) { // NOPMD - } - } - } else { - if (request.httpMethod().isWithBody()) { - // To use this Header, set 'sun.net.http.allowRestrictedHeaders' property true. - connection.addRequestProperty("Content-Length", "0"); + /* + * Ignore disableRequestBuffering flag if the empty body was set, to ensure that internal + * retry logic applies to such requests. + */ + if (disableRequestBuffering) { + if (contentLength != null) { + connection.setFixedLengthStreamingMode(contentLength); + } else { + connection.setChunkedStreamingMode(8196); } + } + connection.setDoOutput(true); + OutputStream out = connection.getOutputStream(); + if (gzipEncodedRequest) { + out = new GZIPOutputStream(out); + } else if (deflateEncodedRequest) { + out = new DeflaterOutputStream(out); + } + try { + request.sendBodyToOutputStream(out); + } finally { + try { + out.close(); + } catch (IOException suppressed) { // NOPMD + } + } + } else { + if (request.httpMethod().isWithBody()) { + // To use this Header, set 'sun.net.http.allowRestrictedHeaders' property true. + connection.addRequestProperty("Content-Length", "0"); + } } - return connection; } diff --git a/core/src/main/java/feign/FeignException.java b/core/src/main/java/feign/FeignException.java index f52c610728..74d70fb309 100644 --- a/core/src/main/java/feign/FeignException.java +++ b/core/src/main/java/feign/FeignException.java @@ -20,6 +20,7 @@ import static feign.Util.checkNotNull; import static java.lang.String.format; +import feign.utils.ContentTypeParser; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStreamReader; @@ -33,11 +34,12 @@ import java.util.Map; import java.util.Optional; -import feign.utils.ContentTypeParser; - /** Origin exception type for all Http Apis. */ -// TODO: KD - FeignException does not currently support streaming bodies. Usually, error responses are short enough that this isn't an issue, but we may want to eventually replace byte[] responseBody with Response.Body responseBody; -// TODO: KD - for that matter, why aren't we just capturing the response itself instead of the headers and body as separate parameters? errorReading() captures the response... +// TODO: KD - FeignException does not currently support streaming bodies. Usually, error responses +// are short enough that this isn't an issue, but we may want to eventually replace byte[] +// responseBody with Response.Body responseBody; +// TODO: KD - for that matter, why aren't we just capturing the response itself instead of the +// headers and body as separate parameters? errorReading() captures the response... public class FeignException extends RuntimeException { private static final String EXCEPTION_MESSAGE_TEMPLATE_NULL_REQUEST = @@ -529,9 +531,9 @@ private String getResponseBodyPreview(byte[] body, Charset charset) { private static Charset getResponseCharset(Map> headers) { - return ContentTypeParser.parseContentTypeFromHeaders(headers, "").getCharset().orElse(Util.UTF_8); - + return ContentTypeParser.parseContentTypeFromHeaders(headers, "") + .getCharset() + .orElse(Util.UTF_8); } - } } diff --git a/core/src/main/java/feign/InvocationContext.java b/core/src/main/java/feign/InvocationContext.java index 57e4d46e24..7511025817 100755 --- a/core/src/main/java/feign/InvocationContext.java +++ b/core/src/main/java/feign/InvocationContext.java @@ -18,13 +18,12 @@ import static feign.FeignException.errorReading; import static feign.Util.ensureClosed; -import java.io.Closeable; -import java.io.IOException; -import java.lang.reflect.Type; - import feign.codec.DecodeException; import feign.codec.Decoder; import feign.codec.ErrorDecoder; +import java.io.Closeable; +import java.io.IOException; +import java.lang.reflect.Type; public class InvocationContext { private final String configKey; @@ -73,7 +72,7 @@ public Object proceed() throws Exception { } boolean noClose = false; - + try { final boolean shouldDecodeResponseBody = (response.status() >= 200 && response.status() < 300) @@ -89,12 +88,12 @@ public Object proceed() throws Exception { } Class rawType = Types.getRawType(returnType); - + // if the return type is closable, then it is the callers responsibility to close. - if (Closeable.class.isAssignableFrom(rawType)) { - noClose = true; + if (Closeable.class.isAssignableFrom(rawType)) { + noClose = true; } - + if (TypedResponse.class.isAssignableFrom(rawType)) { Type bodyType = Types.resolveLastTypeParameter(returnType, TypedResponse.class); return TypedResponse.builder(response).body(decode(response, bodyType)).build(); diff --git a/core/src/main/java/feign/Logger.java b/core/src/main/java/feign/Logger.java index cf66e4ea48..220544663d 100644 --- a/core/src/main/java/feign/Logger.java +++ b/core/src/main/java/feign/Logger.java @@ -85,7 +85,7 @@ protected void logRequest(String configKey, Level logLevel, Request request) { } log(configKey, "---> END HTTP"); } else { - log(configKey, "---> END HTTP (no body)"); + log(configKey, "---> END HTTP (no body)"); } } } diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index 5e49248816..c7e83f54bd 100644 --- a/core/src/main/java/feign/Request.java +++ b/core/src/main/java/feign/Request.java @@ -20,12 +20,14 @@ import static feign.Util.getThreadIdentifier; import static feign.Util.valuesOrEmpty; +import feign.Request.OutputStreamSender; +import feign.Request.WriterSender; +import feign.utils.ContentTypeParser; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.Serializable; -import java.io.StringWriter; import java.io.Writer; import java.net.HttpURLConnection; import java.nio.charset.Charset; @@ -36,14 +38,9 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; -import feign.Request.OutputStreamSender; -import feign.Request.WriterSender; -import feign.utils.ContentTypeParser; - /** An immutable request to an http server. */ public final class Request implements Serializable { @@ -112,12 +109,12 @@ public static Request create( Map> headers, byte[] bodyBytes, Charset charset) { - return create(httpMethod, url, headers, Body.create(bodyBytes), (RequestTemplate)null); + return create(httpMethod, url, headers, Body.create(bodyBytes), (RequestTemplate) null); } - + /** - * Builds a Request. All parameters must be effectively immutable, via safe copies. - * Used for unit testing only + * Builds a Request. All parameters must be effectively immutable, via safe copies. Used for unit + * testing only * * @param httpMethod for the request. * @param url for the request. @@ -127,14 +124,14 @@ public static Request create( * @return a Request */ public static Request create( - HttpMethod httpMethod, - String url, - Map> headers, - byte[] bodyBytes, - Charset charset, - RequestTemplate requestTemplate) { - return create(httpMethod, url, headers, Body.create(bodyBytes), requestTemplate); - } + HttpMethod httpMethod, + String url, + Map> headers, + byte[] bodyBytes, + Charset charset, + RequestTemplate requestTemplate) { + return create(httpMethod, url, headers, Body.create(bodyBytes), requestTemplate); + } /** * Builds a Request. All parameters must be effectively immutable, via safe copies. @@ -153,7 +150,7 @@ public static Request create( RequestTemplate requestTemplate) { return new Request(httpMethod, url, headers, body, requestTemplate); } - + private final HttpMethod httpMethod; private final String url; private final Map> headers; @@ -248,20 +245,23 @@ public void header(String key, Collection values) { * @return the current character set for the request, may be {@literal null} for binary data. */ public Charset charset() { - return ContentTypeParser.parseContentTypeFromHeaders(headers, "Unknown/Unknown").getCharset().orElse(null); + return ContentTypeParser.parseContentTypeFromHeaders(headers, "Unknown/Unknown") + .getCharset() + .orElse(null); } public boolean hasBody() { - return body != null; + return body != null; } - + /** - * @return A string representation of the body as would be written to logs (this is NOT guaranteed to be an exact replica of the request body) + * @return A string representation of the body as would be written to logs (this is NOT guaranteed + * to be an exact replica of the request body) */ public String bodyAsString() { - return body.asString(); + return body.asString(); } - + /** * If present, this is the replayable body to send to the server. In some cases, this may be * interpretable as text. @@ -271,31 +271,32 @@ public String bodyAsString() { */ @Deprecated public byte[] body() { - // Anyone calling this method is circumventing the streaming capabilities - //System.err.println("Deprecated method called"); - if (body == null) return null; - try(ByteArrayOutputStream baos = new ByteArrayOutputStream()) { - body.sendToOutputStream(baos); - return baos.toByteArray(); - } catch (IOException e) { - throw new FeignException(-1, e.getMessage(), e); - } + // Anyone calling this method is circumventing the streaming capabilities + // System.err.println("Deprecated method called"); + if (body == null) return null; + try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) { + body.sendToOutputStream(baos); + return baos.toByteArray(); + } catch (IOException e) { + throw new FeignException(-1, e.getMessage(), e); + } } public void sendBodyToOutputStream(OutputStream os) throws IOException { - body.sendToOutputStream(os); + body.sendToOutputStream(os); } /** * Request Length. * - * @return size of the request body (using the Content-Length header value) if specified, or -1 if unknown + * @return size of the request body (using the Content-Length header value) if specified, or -1 if + * unknown */ public int length() { for (String value : headers().get(CONTENT_LENGTH)) { - return Integer.valueOf(value); + return Integer.valueOf(value); } - + return -1; } @@ -527,14 +528,14 @@ public RequestTemplate requestTemplate() { return this.requestTemplate; } - public static interface OutputStreamSender{ - public void sendToOutputStream(OutputStream os) throws IOException; + public static interface OutputStreamSender { + public void sendToOutputStream(OutputStream os) throws IOException; } - - public static interface WriterSender{ - public void sendToWriter(Writer w) throws IOException; + + public static interface WriterSender { + public void sendToWriter(Writer w) throws IOException; } - + /** * Request Body * @@ -543,21 +544,22 @@ public static interface WriterSender{ @Experimental public static class Body implements Serializable { - private static final Body EMPTY = new Body(os -> {}, "-- EMPTY BODY --"); - - private final OutputStreamSender streamSender; - - /** - * The string representation of this body as it will be displayed in log output or toString() methods - */ - private final String stringRepresentation; - + private static final Body EMPTY = new Body(os -> {}, "-- EMPTY BODY --"); + + private final OutputStreamSender streamSender; + + /** + * The string representation of this body as it will be displayed in log output or toString() + * methods + */ + private final String stringRepresentation; + protected Body(OutputStreamSender streamSender, String stringRepresentation) { this.streamSender = streamSender; this.stringRepresentation = stringRepresentation; } - public void sendToOutputStream(OutputStream os) throws IOException{ + public void sendToOutputStream(OutputStream os) throws IOException { streamSender.sendToOutputStream(os); } @@ -566,41 +568,38 @@ public String asString() { } public static Body create(String data, Charset charset) { - Objects.requireNonNull(data); - Objects.requireNonNull(charset); - - return createForWriterSender(w -> w.write(data), charset, data); + Objects.requireNonNull(data); + Objects.requireNonNull(charset); + + return createForWriterSender(w -> w.write(data), charset, data); } public static Body create(byte[] data) { - // Compatibility with a zillion unit tests that use a null byte[] to indicate no body - if (data == null) return null; - + // Compatibility with a zillion unit tests that use a null byte[] to indicate no body + if (data == null) return null; + return createForOutputStreamSender( - os -> os.write(data), - "Binary data (" + data.length + " bytes)" - ); - } - - public static Body createForWriterSender(WriterSender sender, Charset charset, String stringRep) { - Objects.requireNonNull(charset); - - OutputStreamSender osSender = os -> { - try(Writer osw = new OutputStreamWriter(os, charset)){ - sender.sendToWriter( osw ); - } - }; - - return createForOutputStreamSender( - osSender, - stringRep - ); + os -> os.write(data), "Binary data (" + data.length + " bytes)"); + } + + public static Body createForWriterSender( + WriterSender sender, Charset charset, String stringRep) { + Objects.requireNonNull(charset); + + OutputStreamSender osSender = + os -> { + try (Writer osw = new OutputStreamWriter(os, charset)) { + sender.sendToWriter(osw); + } + }; + + return createForOutputStreamSender(osSender, stringRep); } public static Body createForOutputStreamSender(OutputStreamSender sender, String stringRep) { - return new Request.Body(sender, stringRep); + return new Request.Body(sender, stringRep); } - + public static Body empty() { return EMPTY; } diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java index 011522a383..31e782f0de 100644 --- a/core/src/main/java/feign/RequestTemplate.java +++ b/core/src/main/java/feign/RequestTemplate.java @@ -1,3 +1,18 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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 + * + * http://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. + */ /* * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -17,6 +32,15 @@ import static feign.Util.CONTENT_LENGTH; import static feign.Util.checkNotNull; +import feign.Request.HttpMethod; +import feign.Request.OutputStreamSender; +import feign.Request.WriterSender; +import feign.template.BodyTemplate; +import feign.template.HeaderTemplate; +import feign.template.QueryTemplate; +import feign.template.UriTemplate; +import feign.template.UriUtils; +import feign.utils.ContentTypeParser; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.Serializable; @@ -33,23 +57,12 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Map.Entry; import java.util.TreeMap; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; -import feign.Request.HttpMethod; -import feign.Request.OutputStreamSender; -import feign.Request.WriterSender; -import feign.template.BodyTemplate; -import feign.template.HeaderTemplate; -import feign.template.QueryTemplate; -import feign.template.UriTemplate; -import feign.template.UriUtils; -import feign.utils.ContentTypeParser; - /** * Request Builder for an HTTP Target. * @@ -107,8 +120,8 @@ private RequestTemplate( CollectionFormat collectionFormat, MethodMetadata methodMetadata, Target feignTarget) { - - this(); + + this(); this.target = target; this.fragment = fragment; this.uriTemplate = uriTemplate; @@ -297,7 +310,7 @@ public Request request() { if (!this.resolved) { throw new IllegalStateException("template has not been resolved."); } - + return Request.create(this.method, this.url(), this.headers(), this.body, this); } @@ -875,7 +888,7 @@ public Map> headers() { * Sets the Body and Charset for this request. * * @param data to send, can be null. - * @param charset of the encoded data. Ignored (see deprecation note) + * @param charset of the encoded data. Ignored (see deprecation note) * @return a RequestTemplate for chaining. * @deprecated Charset always comes from the content-type header. Use {@link #body(byte[])} */ @@ -886,76 +899,79 @@ public RequestTemplate body(byte[] data, Charset charset) { /** * Sets the body output to a fixed byte[] + * * @param data the data for the body * @return a RequestTemplate for chaining. */ public RequestTemplate body(byte[] data) { - this.body(Request.Body.create(data)); - this.header(CONTENT_LENGTH, Integer.toString(data.length) ); - return this; + this.body(Request.Body.create(data)); + this.header(CONTENT_LENGTH, Integer.toString(data.length)); + return this; } - + /** - * Set the Body for this request. The content length header will be set to the length of the encoded string. - * Encoding is done using the {@link #requestCharset()} + * Set the Body for this request. The content length header will be set to the length of the + * encoded string. Encoding is done using the {@link #requestCharset()} * * @param bodyText to send. * @return a RequestTemplate for chaining. */ public RequestTemplate body(String bodyText) { - this.body(Request.Body.create(bodyText, requestCharset()) ); - this.header(CONTENT_LENGTH, Integer.toString(bodyText.getBytes(requestCharset()).length) ); + this.body(Request.Body.create(bodyText, requestCharset())); + this.header(CONTENT_LENGTH, Integer.toString(bodyText.getBytes(requestCharset()).length)); return this; } /** - * Sets the sender for the body content using the request template's content-type charset encoding (or utf-8 if charset is not specified) + * Sets the sender for the body content using the request template's content-type charset encoding + * (or utf-8 if charset is not specified) + * * @param sender the sender that will generate the body content - * @param stringRepresentation the string that will be used for logging and toString methods. If null, the request will be read from the sender, which will cause downstream failures if the sender cannot be re-used, so be sure to set this for streaming requests. + * @param stringRepresentation the string that will be used for logging and toString methods. If + * null, the request will be read from the sender, which will cause downstream failures if the + * sender cannot be re-used, so be sure to set this for streaming requests. * @return a RequestTemplate for chaining. */ public RequestTemplate bodyWriterSender(WriterSender sender, String stringRepresentation) { - - if (stringRepresentation == null) { - try(StringWriter writer = new StringWriter()) { - sender.sendToWriter(writer); - stringRepresentation = sender.toString(); - } catch (IOException e) { - stringRepresentation = "-- Unable to determine body: " + e.getMessage() + " --"; - } - - } - - return body( - Request.Body.createForWriterSender(sender, requestCharset(), stringRepresentation) - ); - + + if (stringRepresentation == null) { + try (StringWriter writer = new StringWriter()) { + sender.sendToWriter(writer); + stringRepresentation = sender.toString(); + } catch (IOException e) { + stringRepresentation = "-- Unable to determine body: " + e.getMessage() + " --"; + } + } + + return body(Request.Body.createForWriterSender(sender, requestCharset(), stringRepresentation)); } - - public RequestTemplate bodyOutputStreamSender(OutputStreamSender sender, String stringRepresentation) { - return body( - Request.Body.createForOutputStreamSender(sender, stringRepresentation) - ); + + public RequestTemplate bodyOutputStreamSender( + OutputStreamSender sender, String stringRepresentation) { + return body(Request.Body.createForOutputStreamSender(sender, stringRepresentation)); } - + /** - * Set the Body for this request and clear the content-length header. Callers can set the content-length after calling this if they are able to compute the length. + * Set the Body for this request and clear the content-length header. Callers can set the + * content-length after calling this if they are able to compute the length. * * @param body to send. * @return a RequestTemplate for chaining. - * @deprecated this method will eventually be changed to private. Use the various public body(...) methods instead. + * @deprecated this method will eventually be changed to private. Use the various public body(...) + * methods instead. */ @Deprecated public RequestTemplate body(Request.Body body) { this.body = body; - // Enforce invariant: bodyTemplate or body must be set, but not both + // Enforce invariant: bodyTemplate or body must be set, but not both this.bodyTemplate = null; - // remove the content length header to indicate streaming is required, callers can set it again if they are able to compute the length - this.header(CONTENT_LENGTH); - + // remove the content length header to indicate streaming is required, callers can set it again + // if they are able to compute the length + this.header(CONTENT_LENGTH); + return this; } @@ -964,35 +980,37 @@ public RequestTemplate body(Request.Body body) { * * @return the currently applied Charset. */ - - public Charset requestCharset() { - - // KD - Note that this is the charset of the request body - *not* the 'charset' field of this RequestTemplate object. - // As near as I can tell, the RequestTemplate#charset field isn't actually necessary except for feeding UriTemplate and QueryTemplate - // and neither of those benefit from an externally supplied charset. At the end of the day, all of these calls terminate at UriUtils#encodeChunk, - // which encodes, processes bytes, then immediately decodes. Any encoding would work. - - return ContentTypeParser.parseContentTypeFromHeaders(headers(), "Unknown/Unknown").getCharset().orElse(Util.UTF_8); - + + // KD - Note that this is the charset of the request body - *not* the 'charset' field of this + // RequestTemplate object. + // As near as I can tell, the RequestTemplate#charset field isn't actually necessary except + // for feeding UriTemplate and QueryTemplate + // and neither of those benefit from an externally supplied charset. At the end of the day, + // all of these calls terminate at UriUtils#encodeChunk, + // which encodes, processes bytes, then immediately decodes. Any encoding would work. + + return ContentTypeParser.parseContentTypeFromHeaders(headers(), "Unknown/Unknown") + .getCharset() + .orElse(Util.UTF_8); } /** * The Request Body. * - * @implNote This should not be called unless the request body is resettable (e.g. the body was set using {@link #body(byte[])} or {@link #body(String)} ) - * + * @implNote This should not be called unless the request body is resettable (e.g. the body was + * set using {@link #body(byte[])} or {@link #body(String)} ) * @return the request body as a byte array */ public byte[] body() { - if (this.body == null) return null; - try { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - this.body.sendToOutputStream(baos); - return baos.toByteArray(); - } catch (IOException e) { - throw new FeignException(-1, e.getMessage(), e); - } + if (this.body == null) return null; + try { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + this.body.sendToOutputStream(baos); + return baos.toByteArray(); + } catch (IOException e) { + throw new FeignException(-1, e.getMessage(), e); + } } /** @@ -1023,8 +1041,8 @@ public RequestTemplate bodyTemplate(String bodyTemplate) { * @return a RequestTemplate for chaining. */ public RequestTemplate bodyTemplate(String bodyTemplate, Charset charset) { - - // Enforce invariant: bodyTemplate or body must be set, but not both + + // Enforce invariant: bodyTemplate or body must be set, but not both this.bodyTemplate = BodyTemplate.create(bodyTemplate, charset); this.body = null; return this; diff --git a/core/src/main/java/feign/Response.java b/core/src/main/java/feign/Response.java index ab7a693ea1..5781dd17d8 100644 --- a/core/src/main/java/feign/Response.java +++ b/core/src/main/java/feign/Response.java @@ -19,7 +19,6 @@ import feign.Request.ProtocolVersion; import feign.utils.ContentTypeParser; - import java.io.*; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; @@ -210,11 +209,13 @@ public ProtocolVersion protocolVersion() { * See rfc7231 - Media * Type */ - // TODO: KD - RFC 7231 spec says default charset if not specified is ISO-8859-1 (at least for HTTP/1.1). Seems dangerous to assume UTF_8 here... + // TODO: KD - RFC 7231 spec says default charset if not specified is ISO-8859-1 (at least for + // HTTP/1.1). Seems dangerous to assume UTF_8 here... public Charset charset() { - return ContentTypeParser.parseContentTypeFromHeaders(headers(), "").getCharset().orElse(Util.UTF_8); - + return ContentTypeParser.parseContentTypeFromHeaders(headers(), "") + .getCharset() + .orElse(Util.UTF_8); } @Override @@ -231,29 +232,29 @@ public String toString() { if (body != null) builder.append('\n').append(bodyPreview()); return builder.toString(); } - - private String bodyPreview(){ - final int MAX_CHARS = 1024; - - try { - - char[] preview = new char[MAX_CHARS]; - Reader reader = new InputStreamReader(body.asInputStream(), charset()); - int count = reader.read(preview); - - if (count == -1) return ""; - - boolean fullBody = count < preview.length; - - String bodyPreview = new String(preview, 0, count); - - if (!fullBody) bodyPreview = bodyPreview + "... (" + body.length() + " bytes)"; - - return bodyPreview; - } catch (IOException e) { - return e + " , failed to parse response body preview"; - } - } + + private String bodyPreview() { + final int MAX_CHARS = 1024; + + try { + + char[] preview = new char[MAX_CHARS]; + Reader reader = new InputStreamReader(body.asInputStream(), charset()); + int count = reader.read(preview); + + if (count == -1) return ""; + + boolean fullBody = count < preview.length; + + String bodyPreview = new String(preview, 0, count); + + if (!fullBody) bodyPreview = bodyPreview + "... (" + body.length() + " bytes)"; + + return bodyPreview; + } catch (IOException e) { + return e + " , failed to parse response body preview"; + } + } @Override public void close() { @@ -289,17 +290,19 @@ default Reader asReader() throws IOException { /** It is the responsibility of the caller to close the stream. */ Reader asReader(Charset charset) throws IOException; - } private static final class InputStreamBody implements Response.Body { - private final int REWIND_LIMIT = 8092; + private final int REWIND_LIMIT = 8092; private final InputStream inputStream; private final Integer length; private InputStreamBody(InputStream inputStream, Integer length) { - this.inputStream = inputStream.markSupported() ? inputStream : new BufferedInputStream(inputStream, REWIND_LIMIT); + this.inputStream = + inputStream.markSupported() + ? inputStream + : new BufferedInputStream(inputStream, REWIND_LIMIT); this.inputStream.mark(0); this.length = length; } @@ -343,7 +346,6 @@ public Reader asReader(Charset charset) throws IOException { public void close() throws IOException { inputStream.close(); } - } private static final class ByteArrayBody implements Response.Body { diff --git a/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java b/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java index 9eb550abcb..7eecf8f6dd 100644 --- a/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java +++ b/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java @@ -1,71 +1,86 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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 + * + * http://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 feign.stream; import static java.lang.String.format; -import java.io.BufferedInputStream; -import java.io.File; -import java.io.InputStream; -import java.lang.reflect.Type; -import java.nio.file.Files; - import feign.Request.OutputStreamSender; import feign.RequestTemplate; import feign.Util; import feign.codec.EncodeException; import feign.codec.Encoder; +import java.io.BufferedInputStream; +import java.io.File; +import java.io.InputStream; +import java.lang.reflect.Type; +import java.nio.file.Files; public class InputStreamAndFileEncoder implements Encoder { - private final Encoder delegateEncoder; - - public InputStreamAndFileEncoder(Encoder delegateEncoder) { - this.delegateEncoder = delegateEncoder; - } - - @Override - public void encode(Object object, Type bodyType, RequestTemplate template) { - if (bodyType instanceof Class) { - Class bodyClass = (Class)bodyType; - if (InputStream.class.isAssignableFrom(bodyClass)) { - - // Support some degree of retry - if the stream is too long, then any retry will throw - final int BUFSIZE = 8092; - - InputStream is = (InputStream)object; - InputStream streamToSend = is.markSupported() ? is : new BufferedInputStream(is, BUFSIZE); - streamToSend.mark(BUFSIZE); - - OutputStreamSender sender = os -> { - streamToSend.reset(); - Util.copy(streamToSend, os); - }; - - template.bodyOutputStreamSender(sender, "-- Binary Data (Unknown Length) --"); - - return; - } - - if (File.class.isAssignableFrom(bodyClass)) { - File file = (File)object; - - if (!file.isFile()) - throw new EncodeException(format("Unable to encode missing file - %s", file)); - - template.bodyOutputStreamSender(os -> Files.copy(file.toPath(), os), "-- Content of " + file + " (" + file.length() + " bytes) --"); - template.header(Util.CONTENT_LENGTH, Long.toString(file.length())); - - return; - } - - } - - - if (delegateEncoder != null) { - delegateEncoder.encode(object, bodyType, template); - return; - } - - throw new EncodeException(format("%s is not a type supported by this encoder.", object.getClass())); + private final Encoder delegateEncoder; + + public InputStreamAndFileEncoder(Encoder delegateEncoder) { + this.delegateEncoder = delegateEncoder; + } + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + if (bodyType instanceof Class) { + Class bodyClass = (Class) bodyType; + if (InputStream.class.isAssignableFrom(bodyClass)) { + + // Support some degree of retry - if the stream is too long, then any retry will throw + final int BUFSIZE = 8092; + + InputStream is = (InputStream) object; + InputStream streamToSend = is.markSupported() ? is : new BufferedInputStream(is, BUFSIZE); + streamToSend.mark(BUFSIZE); + + OutputStreamSender sender = + os -> { + streamToSend.reset(); + Util.copy(streamToSend, os); + }; + + template.bodyOutputStreamSender(sender, "-- Binary Data (Unknown Length) --"); + + return; + } + + if (File.class.isAssignableFrom(bodyClass)) { + File file = (File) object; + + if (!file.isFile()) + throw new EncodeException(format("Unable to encode missing file - %s", file)); + + template.bodyOutputStreamSender( + os -> Files.copy(file.toPath(), os), + "-- Content of " + file + " (" + file.length() + " bytes) --"); + template.header(Util.CONTENT_LENGTH, Long.toString(file.length())); + + return; + } } + + if (delegateEncoder != null) { + delegateEncoder.encode(object, bodyType, template); + return; + } + + throw new EncodeException( + format("%s is not a type supported by this encoder.", object.getClass())); + } } diff --git a/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java b/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java index dad5555f35..36fc84c6f9 100644 --- a/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java +++ b/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java @@ -1,34 +1,46 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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 + * + * http://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 feign.stream; +import feign.FeignException; +import feign.Response; +import feign.codec.DecodeException; +import feign.codec.Decoder; import java.io.IOException; import java.io.InputStream; import java.io.Reader; import java.lang.reflect.Type; -import feign.FeignException; -import feign.Response; -import feign.codec.DecodeException; -import feign.codec.Decoder; +public class InputStreamAndReaderDecoder implements Decoder { + private final Decoder delegateDecoder; + + public InputStreamAndReaderDecoder(Decoder delegate) { + this.delegateDecoder = delegate; + } -public class InputStreamAndReaderDecoder implements Decoder{ - private final Decoder delegateDecoder; - - public InputStreamAndReaderDecoder(Decoder delegate) { - this.delegateDecoder = delegate; - } + @Override + public Object decode(Response response, Type type) + throws IOException, DecodeException, FeignException { - @Override - public Object decode(Response response, Type type) throws IOException, DecodeException, FeignException { + if (InputStream.class.equals(type)) return response.body().asInputStream(); - if (InputStream.class.equals(type)) - return response.body().asInputStream(); - - if (Reader.class.equals(type)) - return response.body().asReader(); + if (Reader.class.equals(type)) return response.body().asReader(); - if (delegateDecoder == null) return null; - - return delegateDecoder.decode(response, type); - } + if (delegateDecoder == null) return null; + return delegateDecoder.decode(response, type); + } } diff --git a/core/src/main/java/feign/utils/ContentTypeParser.java b/core/src/main/java/feign/utils/ContentTypeParser.java index 3e87b33680..8684411f10 100644 --- a/core/src/main/java/feign/utils/ContentTypeParser.java +++ b/core/src/main/java/feign/utils/ContentTypeParser.java @@ -1,3 +1,18 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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 + * + * http://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 feign.utils; import java.nio.charset.Charset; @@ -8,53 +23,57 @@ public final class ContentTypeParser { - private ContentTypeParser() { - } - - public static ContentTypeResult parseContentTypeFromHeaders(Map> headers, String ifMissing) { - // The header map *should* be a case insensitive treemap - for (String val : headers.getOrDefault("content-type", Collections.emptyList())) { - return parseContentTypeHeader(val); - } - - return new ContentTypeResult(ifMissing, null); - } - - public static ContentTypeResult parseContentTypeHeader(String contentTypeHeader) { - - String[] contentTypeParmeters = contentTypeHeader.split(";"); - String contentType = contentTypeParmeters[0]; - String charsetString = ""; - if (contentTypeParmeters.length > 1) { - String[] charsetParts = contentTypeParmeters[1].split("="); - if (charsetParts.length == 2 && "charset".equalsIgnoreCase(charsetParts[0].trim())) { - // TODO: KD - this doesn't really implement the full parser definition for the content-type header (esp related to quoted strings, etc...) - see https://www.w3.org/Protocols/rfc1341/4_Content-Type.html - charsetString = charsetParts[1].trim(); - if (charsetString.length() > 1 && charsetString.startsWith("\"") && charsetString.endsWith("\"")) - charsetString = charsetString.substring(1, charsetString.length()-1); - } + private ContentTypeParser() {} + + public static ContentTypeResult parseContentTypeFromHeaders( + Map> headers, String ifMissing) { + // The header map *should* be a case insensitive treemap + for (String val : headers.getOrDefault("content-type", Collections.emptyList())) { + return parseContentTypeHeader(val); + } + + return new ContentTypeResult(ifMissing, null); + } + + public static ContentTypeResult parseContentTypeHeader(String contentTypeHeader) { + + String[] contentTypeParmeters = contentTypeHeader.split(";"); + String contentType = contentTypeParmeters[0]; + String charsetString = ""; + if (contentTypeParmeters.length > 1) { + String[] charsetParts = contentTypeParmeters[1].split("="); + if (charsetParts.length == 2 && "charset".equalsIgnoreCase(charsetParts[0].trim())) { + // TODO: KD - this doesn't really implement the full parser definition for the content-type + // header (esp related to quoted strings, etc...) - see + // https://www.w3.org/Protocols/rfc1341/4_Content-Type.html + charsetString = charsetParts[1].trim(); + if (charsetString.length() > 1 + && charsetString.startsWith("\"") + && charsetString.endsWith("\"")) + charsetString = charsetString.substring(1, charsetString.length() - 1); } - - return new ContentTypeResult(contentType, Charset.forName(charsetString, null)); - } - - public static class ContentTypeResult{ - public static final ContentTypeResult MISSING = new ContentTypeResult("", null); - - private String contentType; - private Optional charset; - - public ContentTypeResult(String contentType, Charset charset) { - this.contentType = contentType; - this.charset = Optional.ofNullable(charset); - } - - public String getContentType() { - return contentType; - } - - public Optional getCharset() { - return charset; - } - } + } + + return new ContentTypeResult(contentType, Charset.forName(charsetString, null)); + } + + public static class ContentTypeResult { + public static final ContentTypeResult MISSING = new ContentTypeResult("", null); + + private String contentType; + private Optional charset; + + public ContentTypeResult(String contentType, Charset charset) { + this.contentType = contentType; + this.charset = Optional.ofNullable(charset); + } + + public String getContentType() { + return contentType; + } + + public Optional getCharset() { + return charset; + } + } } diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java index c18b8b184e..15d744d214 100755 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -31,16 +31,23 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import java.io.BufferedInputStream; -import java.io.ByteArrayInputStream; -import java.io.File; +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; +import feign.Feign.ResponseMappingDecoder; +import feign.QueryMap.MapEncoder; +import feign.Request.HttpMethod; +import feign.Target.HardCodedTarget; +import feign.codec.DecodeException; +import feign.codec.Decoder; +import feign.codec.EncodeException; +import feign.codec.Encoder; +import feign.codec.ErrorDecoder; +import feign.codec.StringDecoder; +import feign.querymap.BeanQueryMapEncoder; +import feign.querymap.FieldQueryMapEncoder; import java.io.IOException; -import java.io.InputStream; -import java.io.Reader; import java.lang.reflect.Type; import java.net.URI; -import java.nio.file.Files; -import java.nio.file.Path; import java.time.Clock; import java.time.Instant; import java.time.ZoneId; @@ -53,38 +60,17 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; -import java.util.Random; import java.util.concurrent.atomic.AtomicReference; - +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.SocketPolicy; +import okio.Buffer; import org.assertj.core.data.MapEntry; import org.assertj.core.util.Maps; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; import org.mockito.ArgumentMatchers; -import com.google.gson.Gson; -import com.google.gson.reflect.TypeToken; - -import feign.Feign.ResponseMappingDecoder; -import feign.QueryMap.MapEncoder; -import feign.Request.HttpMethod; -import feign.Target.HardCodedTarget; -import feign.codec.DecodeException; -import feign.codec.Decoder; -import feign.codec.EncodeException; -import feign.codec.Encoder; -import feign.codec.ErrorDecoder; -import feign.codec.StringDecoder; -import feign.querymap.BeanQueryMapEncoder; -import feign.querymap.FieldQueryMapEncoder; -import feign.stream.InputStreamAndFileEncoder; -import feign.stream.InputStreamAndReaderDecoder; -import okhttp3.mockwebserver.MockResponse; -import okhttp3.mockwebserver.MockWebServer; -import okhttp3.mockwebserver.SocketPolicy; -import okio.Buffer; - @SuppressWarnings("deprecation") public class FeignTest { diff --git a/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java b/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java index df9ec97ec6..fc16d14dbc 100644 --- a/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java +++ b/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java @@ -1,8 +1,30 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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 + * + * http://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 feign.stream; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; +import feign.Feign; +import feign.FeignException; +import feign.Request.HttpMethod; +import feign.RequestLine; +import feign.Response; +import feign.RetryableException; +import feign.codec.StringDecoder; import java.io.BufferedInputStream; import java.io.ByteArrayInputStream; import java.io.File; @@ -12,140 +34,142 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Random; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; - -import feign.Feign; -import feign.FeignException; -import feign.Request.HttpMethod; -import feign.RequestLine; -import feign.Response; -import feign.RetryableException; -import feign.codec.StringDecoder; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; public class InputStreamAndFileEncoderTest { - public final MockWebServer server = new MockWebServer(); - private static final Long NON_RETRYABLE = null; - - interface LargeStreamTestInterface { - - @RequestLine("POST /") - String postLargeStream(InputStream stream); - - @RequestLine("POST /") - void postLargeFile(File file); - - } - - @Test - void streamingRequest() throws Exception{ - server.enqueue(new MockResponse()); - - byte[] expectedResponse = new byte[16184]; - new Random().nextBytes(expectedResponse); - - - LargeStreamTestInterface api = Feign.builder() - .encoder(new InputStreamAndFileEncoder(null)) - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - try(InputStream is = new ByteArrayInputStream(expectedResponse)){ - api.postLargeStream(is); - } - - byte[] result = server.takeRequest().getBody().readByteArray(); - - assertThat(result.length).isEqualByComparingTo(expectedResponse.length); - assertThat(result).isEqualTo(expectedResponse); - } - - Feign.Builder createRetryableFeignBuilder(){ - return Feign.builder() - .encoder(new InputStreamAndFileEncoder(null)) - .decoder( - new StringDecoder() { - @Override - public Object decode(Response response, Type type) throws IOException { - String string = super.decode(response, type).toString(); - if ("retry!".equals(string)) { - throw new RetryableException( - response.status(), - string, - HttpMethod.POST, - NON_RETRYABLE, - response.request()); - } - return string; - } - }); - } - - @Test - void streamingRequestCanRetry() throws Exception { - - server.enqueue(new MockResponse().setBody("retry!")); - server.enqueue(new MockResponse().setBody("success!")); - - LargeStreamTestInterface api = createRetryableFeignBuilder() - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - byte[] requestData = new byte[16184]; - new Random().nextBytes(requestData); - - // if we use a plain ByteArrayInputStream, then it is infinitely resettable, so the following will retry - // if instead we wrapped this in a reset limited inputstream (like a BufferedInputStream with size set to 1024), retry would fail. - // As of now, I don't see a way to tell the server to read *some* of the request and then fail, so we aren't getting good test coverage of the partial request reset scenario - String rslt = api.postLargeStream(new ByteArrayInputStream(requestData)); - byte[] dataReceivedByServer = server.takeRequest().getBody().readByteArray(); - - assertThat(rslt).isEqualTo("success!"); - assertThat(server.getRequestCount()).isEqualTo(2); - assertThat(dataReceivedByServer).isEqualTo(requestData); - } - - @Test - void streamingRequestRetryFailsIfTooMuchDataRead() throws Exception { - - server.enqueue(new MockResponse().setBody("retry!")); - server.enqueue(new MockResponse().setBody("success!")); - - LargeStreamTestInterface api = createRetryableFeignBuilder() - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - byte[] requestData = new byte[16184]; - new Random().nextBytes(requestData); - - // if we wrap the inputstream in a reset limited inputstream like a BufferedInputStream, retry will fail - // As of now, I don't see a way to tell the server to read *some* of the request and then fail, so we aren't getting good test coverage of the partial request reset scenario - FeignException e = assertThrows(FeignException.class, () -> api.postLargeStream(new BufferedInputStream(new ByteArrayInputStream(requestData), 1024)) ); - - assertThat(e).hasCauseInstanceOf(IOException.class); - } - - @Test - void streamingFileRequest(@TempDir Path tempPath) throws Exception{ - server.enqueue(new MockResponse()); - - byte[] expectedResponse = new byte[16184]; - new Random().nextBytes(expectedResponse); - - Path fileToSend = tempPath.resolve("temp.dat"); - - Files.write(fileToSend, expectedResponse); - - LargeStreamTestInterface api = Feign.builder() - .encoder(new InputStreamAndFileEncoder(null)) - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - api.postLargeFile(fileToSend.toFile()); - - byte[] result = server.takeRequest().getBody().readByteArray(); - - assertThat(result.length).isEqualByComparingTo(expectedResponse.length); - assertThat(result).isEqualTo(expectedResponse); - } - + public final MockWebServer server = new MockWebServer(); + private static final Long NON_RETRYABLE = null; + + interface LargeStreamTestInterface { + + @RequestLine("POST /") + String postLargeStream(InputStream stream); + + @RequestLine("POST /") + void postLargeFile(File file); + } + + @Test + void streamingRequest() throws Exception { + server.enqueue(new MockResponse()); + + byte[] expectedResponse = new byte[16184]; + new Random().nextBytes(expectedResponse); + + LargeStreamTestInterface api = + Feign.builder() + .encoder(new InputStreamAndFileEncoder(null)) + .target(LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try (InputStream is = new ByteArrayInputStream(expectedResponse)) { + api.postLargeStream(is); + } + + byte[] result = server.takeRequest().getBody().readByteArray(); + + assertThat(result.length).isEqualByComparingTo(expectedResponse.length); + assertThat(result).isEqualTo(expectedResponse); + } + + Feign.Builder createRetryableFeignBuilder() { + return Feign.builder() + .encoder(new InputStreamAndFileEncoder(null)) + .decoder( + new StringDecoder() { + @Override + public Object decode(Response response, Type type) throws IOException { + String string = super.decode(response, type).toString(); + if ("retry!".equals(string)) { + throw new RetryableException( + response.status(), + string, + HttpMethod.POST, + NON_RETRYABLE, + response.request()); + } + return string; + } + }); + } + + @Test + void streamingRequestCanRetry() throws Exception { + + server.enqueue(new MockResponse().setBody("retry!")); + server.enqueue(new MockResponse().setBody("success!")); + + LargeStreamTestInterface api = + createRetryableFeignBuilder() + .target(LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + byte[] requestData = new byte[16184]; + new Random().nextBytes(requestData); + + // if we use a plain ByteArrayInputStream, then it is infinitely resettable, so the following + // will retry + // if instead we wrapped this in a reset limited inputstream (like a BufferedInputStream with + // size set to 1024), retry would fail. + // As of now, I don't see a way to tell the server to read *some* of the request and then fail, + // so we aren't getting good test coverage of the partial request reset scenario + String rslt = api.postLargeStream(new ByteArrayInputStream(requestData)); + byte[] dataReceivedByServer = server.takeRequest().getBody().readByteArray(); + + assertThat(rslt).isEqualTo("success!"); + assertThat(server.getRequestCount()).isEqualTo(2); + assertThat(dataReceivedByServer).isEqualTo(requestData); + } + + @Test + void streamingRequestRetryFailsIfTooMuchDataRead() throws Exception { + + server.enqueue(new MockResponse().setBody("retry!")); + server.enqueue(new MockResponse().setBody("success!")); + + LargeStreamTestInterface api = + createRetryableFeignBuilder() + .target(LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + byte[] requestData = new byte[16184]; + new Random().nextBytes(requestData); + + // if we wrap the inputstream in a reset limited inputstream like a BufferedInputStream, retry + // will fail + // As of now, I don't see a way to tell the server to read *some* of the request and then fail, + // so we aren't getting good test coverage of the partial request reset scenario + FeignException e = + assertThrows( + FeignException.class, + () -> + api.postLargeStream( + new BufferedInputStream(new ByteArrayInputStream(requestData), 1024))); + + assertThat(e).hasCauseInstanceOf(IOException.class); + } + + @Test + void streamingFileRequest(@TempDir Path tempPath) throws Exception { + server.enqueue(new MockResponse()); + + byte[] expectedResponse = new byte[16184]; + new Random().nextBytes(expectedResponse); + + Path fileToSend = tempPath.resolve("temp.dat"); + + Files.write(fileToSend, expectedResponse); + + LargeStreamTestInterface api = + Feign.builder() + .encoder(new InputStreamAndFileEncoder(null)) + .target(LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + api.postLargeFile(fileToSend.toFile()); + + byte[] result = server.takeRequest().getBody().readByteArray(); + + assertThat(result.length).isEqualByComparingTo(expectedResponse.length); + assertThat(result).isEqualTo(expectedResponse); + } } diff --git a/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java b/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java index 7fb291e802..b5b4636c8e 100644 --- a/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java +++ b/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java @@ -1,88 +1,112 @@ +/* + * Copyright © 2012 The Feign Authors (feign@commonhaus.dev) + * + * 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 + * + * http://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 feign.stream; import static org.assertj.core.api.Assertions.assertThat; -import java.io.InputStream; -import java.io.Reader; -import java.util.Random; - -import org.junit.jupiter.api.Test; - import feign.Feign; import feign.RequestLine; import feign.Util; +import java.io.InputStream; +import java.io.Reader; +import java.util.Random; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okio.Buffer; +import org.junit.jupiter.api.Test; public class InputStreamAndReaderDecoderTest { - public final MockWebServer server = new MockWebServer(); - - interface LargeStreamTestInterface { - - @RequestLine("GET /") - InputStream getLargeStream(); - - @RequestLine("GET /") - Reader getLargeReader(); - - } - - @Test - void streamingResponse() throws Exception{ - byte[] expectedResponse = new byte[16184]; - new Random().nextBytes(expectedResponse); - server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse))); - - LargeStreamTestInterface api = Feign.builder() - .decoder(new InputStreamAndReaderDecoder(null)) - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - try(InputStream is = api.getLargeStream()){ - byte[] out = is.readAllBytes(); - assertThat(out.length).isEqualTo(expectedResponse.length); - assertThat(out).isEqualTo(expectedResponse); - } - } - - @Test - void streamingReaderResponse() throws Exception{ - String expectedResponse = new Random().ints(1, 1500 + 1) - .limit(16184) - .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) - .toString(); - - server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))).addHeader("content-type", "text/plan; charset=utf-8")); - - LargeStreamTestInterface api = Feign.builder() - .decoder(new InputStreamAndReaderDecoder(null)) - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - try(Reader r = api.getLargeReader()){ - String out = Util.toString(r); - assertThat(out.length()).isEqualTo(expectedResponse.length()); - assertThat(out).isEqualTo(expectedResponse); - } - } - - @Test - void streamingReaderResponseWithNoCharset() throws Exception{ - String expectedResponse = new Random().ints(1, 1500 + 1) - .limit(16184) - .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) - .toString(); - - server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))).addHeader("content-type", "text/plan")); - - LargeStreamTestInterface api = Feign.builder() - .decoder(new InputStreamAndReaderDecoder(null)) - .target( LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); - - try(Reader r = api.getLargeReader()){ - String out = Util.toString(r); - assertThat(out.length()).isEqualTo(expectedResponse.length()); - assertThat(out).isEqualTo(expectedResponse); - } - } - + public final MockWebServer server = new MockWebServer(); + + interface LargeStreamTestInterface { + + @RequestLine("GET /") + InputStream getLargeStream(); + + @RequestLine("GET /") + Reader getLargeReader(); + } + + @Test + void streamingResponse() throws Exception { + byte[] expectedResponse = new byte[16184]; + new Random().nextBytes(expectedResponse); + server.enqueue(new MockResponse().setBody(new Buffer().write(expectedResponse))); + + LargeStreamTestInterface api = + Feign.builder() + .decoder(new InputStreamAndReaderDecoder(null)) + .target(LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try (InputStream is = api.getLargeStream()) { + byte[] out = is.readAllBytes(); + assertThat(out.length).isEqualTo(expectedResponse.length); + assertThat(out).isEqualTo(expectedResponse); + } + } + + @Test + void streamingReaderResponse() throws Exception { + String expectedResponse = + new Random() + .ints(1, 1500 + 1) + .limit(16184) + .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) + .toString(); + + server.enqueue( + new MockResponse() + .setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))) + .addHeader("content-type", "text/plan; charset=utf-8")); + + LargeStreamTestInterface api = + Feign.builder() + .decoder(new InputStreamAndReaderDecoder(null)) + .target(LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try (Reader r = api.getLargeReader()) { + String out = Util.toString(r); + assertThat(out.length()).isEqualTo(expectedResponse.length()); + assertThat(out).isEqualTo(expectedResponse); + } + } + + @Test + void streamingReaderResponseWithNoCharset() throws Exception { + String expectedResponse = + new Random() + .ints(1, 1500 + 1) + .limit(16184) + .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) + .toString(); + + server.enqueue( + new MockResponse() + .setBody(new Buffer().write(expectedResponse.getBytes(Util.UTF_8))) + .addHeader("content-type", "text/plan")); + + LargeStreamTestInterface api = + Feign.builder() + .decoder(new InputStreamAndReaderDecoder(null)) + .target(LargeStreamTestInterface.class, "http://localhost:" + server.getPort()); + + try (Reader r = api.getLargeReader()) { + String out = Util.toString(r); + assertThat(out.length()).isEqualTo(expectedResponse.length()); + assertThat(out).isEqualTo(expectedResponse); + } + } } From b7448918b062b70a8b99a39f1a6055f813029a13 Mon Sep 17 00:00:00 2001 From: kevin Date: Mon, 10 Feb 2025 08:14:48 -0700 Subject: [PATCH 10/11] Add license headers to new source files --- .../stream/InputStreamAndFileEncoder.java | 26 +++++++++++++++++++ .../stream/InputStreamAndReaderDecoder.java | 26 +++++++++++++++++++ .../java/feign/utils/ContentTypeParser.java | 26 +++++++++++++++++++ .../stream/InputStreamAndFileEncoderTest.java | 26 +++++++++++++++++++ .../InputStreamAndReaderDecoderTest.java | 26 +++++++++++++++++++ 5 files changed, 130 insertions(+) diff --git a/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java b/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java index 9eb550abcb..057895bf3f 100644 --- a/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java +++ b/core/src/main/java/feign/stream/InputStreamAndFileEncoder.java @@ -1,3 +1,29 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ package feign.stream; import static java.lang.String.format; diff --git a/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java b/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java index dad5555f35..ccb2eb20c8 100644 --- a/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java +++ b/core/src/main/java/feign/stream/InputStreamAndReaderDecoder.java @@ -1,3 +1,29 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ package feign.stream; import java.io.IOException; diff --git a/core/src/main/java/feign/utils/ContentTypeParser.java b/core/src/main/java/feign/utils/ContentTypeParser.java index 3e87b33680..22d018e962 100644 --- a/core/src/main/java/feign/utils/ContentTypeParser.java +++ b/core/src/main/java/feign/utils/ContentTypeParser.java @@ -1,3 +1,29 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ package feign.utils; import java.nio.charset.Charset; diff --git a/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java b/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java index df9ec97ec6..4c41e296f8 100644 --- a/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java +++ b/core/src/test/java/feign/stream/InputStreamAndFileEncoderTest.java @@ -1,3 +1,29 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ package feign.stream; import static org.assertj.core.api.Assertions.assertThat; diff --git a/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java b/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java index 7fb291e802..7fe2d5d997 100644 --- a/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java +++ b/core/src/test/java/feign/stream/InputStreamAndReaderDecoderTest.java @@ -1,3 +1,29 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ package feign.stream; import static org.assertj.core.api.Assertions.assertThat; From 3b0d46dc18613fbca2e12593146c9f38e4db6829 Mon Sep 17 00:00:00 2001 From: kevin Date: Tue, 11 Feb 2025 12:07:38 -0700 Subject: [PATCH 11/11] Do string check with constant first (eliminate null-pointer-exception possibility) --- core/src/main/java/feign/RequestTemplate.java | 3 +-- core/src/main/java/feign/Util.java | 3 +++ core/src/main/java/feign/utils/ContentTypeParser.java | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java index 011522a383..8be8393550 100644 --- a/core/src/main/java/feign/RequestTemplate.java +++ b/core/src/main/java/feign/RequestTemplate.java @@ -33,7 +33,6 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Map.Entry; import java.util.TreeMap; import java.util.regex.Matcher; @@ -811,7 +810,7 @@ private RequestTemplate appendHeader(String name, Iterable values, boole this.headers.remove(name); return this; } - if (name.equalsIgnoreCase("Content-Type")) { // headers are case-insensitive + if (Util.CONTENT_TYPE.equalsIgnoreCase(name)) { // headers are case-insensitive // a client can only produce content of one single type, so always override Content-Type and // only add a single type this.headers.remove(name); diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java index 11d2ab0cc5..d2f86b5629 100644 --- a/core/src/main/java/feign/Util.java +++ b/core/src/main/java/feign/Util.java @@ -58,6 +58,9 @@ public class Util { /** The HTTP Content-Length header field name. */ public static final String CONTENT_LENGTH = "Content-Length"; + /** The HTTP Content-Length header field name. */ + public static final String CONTENT_TYPE = "Content-Type"; + /** The HTTP Content-Encoding header field name. */ public static final String CONTENT_ENCODING = "Content-Encoding"; diff --git a/core/src/main/java/feign/utils/ContentTypeParser.java b/core/src/main/java/feign/utils/ContentTypeParser.java index 22d018e962..412783fdd9 100644 --- a/core/src/main/java/feign/utils/ContentTypeParser.java +++ b/core/src/main/java/feign/utils/ContentTypeParser.java @@ -32,6 +32,8 @@ import java.util.Map; import java.util.Optional; +import feign.Util; + public final class ContentTypeParser { private ContentTypeParser() { @@ -39,7 +41,7 @@ private ContentTypeParser() { public static ContentTypeResult parseContentTypeFromHeaders(Map> headers, String ifMissing) { // The header map *should* be a case insensitive treemap - for (String val : headers.getOrDefault("content-type", Collections.emptyList())) { + for (String val : headers.getOrDefault(Util.CONTENT_TYPE, Collections.emptyList())) { return parseContentTypeHeader(val); }