android/okhttp
1
0
Fork 0
mirror of https://github.com/square/okhttp.git synced 2026-01-15 20:56:41 +03:00
Code Releases Activity

Another approach to handling strange interceptor bodies.

Browse Source 
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.
This commit is contained in:
jwilson
2016-05-14 14:05:56 -04:00
parent 61c1dad837
commit ec0301acb4
2 changed files with 11 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
okhttp-tests/src/test/java/okhttp3/ConnectionReuseTest.java
View File

@@ -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() {

5
okhttp/src/main/java/okhttp3/RealCall.java
View File

@@ -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;