Do not leak body in intermediate buffers to converters

Multiple calls to source() would create a new BufferedSource which, when a single byte was read, would cache data in its buffer which would be lost on a subsequent call to source(). Calls to source() will now always return the same BufferedSource ensuring data cannot be lost.

retrofit/src/main/java/retrofit2/OkHttpCall.java

@@ -277,10 +277,21 @@ static final class NoContentResponseBody extends ResponseBody {
 
   static final class ExceptionCatchingResponseBody extends ResponseBody {
     private final ResponseBody delegate;
+    private final BufferedSource delegateSource;
     @Nullable IOException thrownException;
 
     ExceptionCatchingResponseBody(ResponseBody delegate) {
       this.delegate = delegate;
+      this.delegateSource = Okio.buffer(new ForwardingSource(delegate.source()) {
+        @Override public long read(Buffer sink, long byteCount) throws IOException {
+          try {
+            return super.read(sink, byteCount);
+          } catch (IOException e) {
+            thrownException = e;
+            throw e;
+          }
+        }
+      });
     }
 
     @Override public MediaType contentType() {
@@ -292,16 +303,7 @@ static final class ExceptionCatchingResponseBody extends ResponseBody {
     }
 
     @Override public BufferedSource source() {
-      return Okio.buffer(new ForwardingSource(delegate.source()) {
-        @Override public long read(Buffer sink, long byteCount) throws IOException {
-          try {
-            return super.read(sink, byteCount);
-          } catch (IOException e) {
-            thrownException = e;
-            throw e;
-          }
-        }
-      });
+      return delegateSource;
     }
 
     @Override public void close() {

retrofit/src/test/java/retrofit2/CallTest.java

@@ -49,6 +49,7 @@
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static retrofit2.TestingUtils.repeat;
 
 public final class CallTest {
   @Rule public final MockWebServer server = new MockWebServer();
@@ -421,6 +422,31 @@ public Converter<?, RequestBody> requestBodyConverter(Type type,
     verifyNoMoreInteractions(converter);
   }
 
+  @Test public void converterBodyDoesNotLeakContentInIntermediateBuffers() throws IOException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new Converter.Factory() {
+          @Override public Converter<ResponseBody, ?> responseBodyConverter(Type type,
+              Annotation[] annotations, Retrofit retrofit) {
+            return new Converter<ResponseBody, String>() {
+              @Override public String convert(ResponseBody value) throws IOException {
+                String prefix = value.source().readUtf8(2);
+                value.source().skip(20_000 - 4);
+                String suffix = value.source().readUtf8();
+                return prefix + suffix;
+              }
+            };
+          }
+        })
+        .build();
+    Service example = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse().setBody(repeat('a', 10_000) + repeat('b', 10_000)));
+
+    Response<String> response = example.getString().execute();
+    assertThat(response.body()).isEqualTo("aabb");
+  }
+
   @Test public void executeCallOnce() throws IOException {
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))

retrofit/src/test/java/retrofit2/TestingUtils.java

@@ -16,13 +16,20 @@
 package retrofit2;
 
 import java.lang.reflect.Method;
+import java.util.Arrays;
 
-public final class TestingUtils {
-  public static Method onlyMethod(Class c) {
+final class TestingUtils {
+  static Method onlyMethod(Class c) {
     Method[] declaredMethods = c.getDeclaredMethods();
     if (declaredMethods.length == 1) {
       return declaredMethods[0];
     }
     throw new IllegalArgumentException("More than one method declared.");
   }
+
+  static String repeat(char c, int times) {
+    char[] cs = new char[times];
+    Arrays.fill(cs, c);
+    return new String(cs);
+  }
 }