From ec0301acb431a36e08d647dd335ca6fa6ec4b1b8 Mon Sep 17 00:00:00 2001
From: jwilson <jwilson@squareup.com>
Date: Sat, 14 May 2016 14:05:56 -0400
Subject: [PATCH] Another approach to handling strange interceptor bodies.

The previous approach Just Works if users have fancy or interesting interceptors
that genuinely need to swap the response body in an interceptor and keep the
original body.

One problem with that solution is that although it gives expert users a powerful
way to separate response bodies, it also allows normal users to accidentally leak
response bodies in their interceptors.

This is an alternate solution that forbids the expert use case and requires that
closing the response body stream also closes the underlying socket stream. It
throws an exception if that implicit contract is not honored.

I'm fine with either solution but think we should consider both.
---
 .../java/okhttp3/ConnectionReuseTest.java     | 19 +++++++------------
 okhttp/src/main/java/okhttp3/RealCall.java    |  5 ++++-
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/okhttp-tests/src/test/java/okhttp3/ConnectionReuseTest.java b/okhttp-tests/src/test/java/okhttp3/ConnectionReuseTest.java
index 63d2dd2b8..2dc6bd9fd 100644
--- a/okhttp-tests/src/test/java/okhttp3/ConnectionReuseTest.java
+++ b/okhttp-tests/src/test/java/okhttp3/ConnectionReuseTest.java
@@ -16,9 +16,7 @@
 package okhttp3;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.List;
 import java.util.concurrent.TimeUnit;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLException;
@@ -34,6 +32,7 @@ import org.junit.rules.Timeout;
 
 import static okhttp3.TestUtil.defaultClient;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 public final class ConnectionReuseTest {
@@ -299,12 +298,9 @@ public final class ConnectionReuseTest {
    * https://github.com/square/okhttp/issues/2409
    */
   @Test public void connectionsAreNotReusedIfNetworkInterceptorInterferes() throws Exception {
-    final List<ResponseBody> responseBodies = new ArrayList<>();
-
     client = client.newBuilder().addNetworkInterceptor(new Interceptor() {
       @Override public Response intercept(Chain chain) throws IOException {
         Response response = chain.proceed(chain.request());
-        responseBodies.add(response.body());
         return response.newBuilder()
             .body(ResponseBody.create(null, "unrelated response body!"))
             .build();
@@ -321,13 +317,12 @@ public final class ConnectionReuseTest {
     Request request = new Request.Builder()
         .url(server.url("/"))
         .build();
-    Response response = client.newCall(request).execute();
-    assertEquals("unrelated response body!", response.body().string());
-    assertEquals("/a has moved!", responseBodies.get(0).string());
-    assertEquals("/b is here", responseBodies.get(1).string());
-
-    assertEquals(0, server.takeRequest().getSequenceNumber()); // New connection.
-    assertEquals(0, server.takeRequest().getSequenceNumber()); // New connection.
+    try {
+      client.newCall(request).execute();
+      fail();
+    } catch (IllegalStateException expected) {
+      assertTrue(expected.getMessage().startsWith("Closing the body of"));
+    }
   }
 
   private void enableHttps() {
diff --git a/okhttp/src/main/java/okhttp3/RealCall.java b/okhttp/src/main/java/okhttp3/RealCall.java
index 4c325f8c6..57431fccd 100644
--- a/okhttp/src/main/java/okhttp3/RealCall.java
+++ b/okhttp/src/main/java/okhttp3/RealCall.java
@@ -292,9 +292,12 @@ final class RealCall implements Call {
         throw new ProtocolException("Too many follow-up requests: " + followUpCount);
       }
 
-      if (!engine.sameConnection(followUp.url()) || streamAllocation.stream() != null) {
+      if (!engine.sameConnection(followUp.url())) {
         streamAllocation.release();
         streamAllocation = null;
+      } else if (streamAllocation.stream() != null) {
+        throw new IllegalStateException("Closing the body of " + response
+            + " didn't close its backing stream. Bad interceptor?");
       }
 
       request = followUp;