Skip to content

Commit

Permalink
Always close HttpURLConnections in HurlStack. (#176)
Browse files Browse the repository at this point in the history
If we are finished with the connection (on network error or empty
response), close it before returning/throwing. If not (because the
response body still needs to be read), wrap the returned InputStream
in one which disconnects when the stream is closed.

Fixes #138
  • Loading branch information
jpd236 authored May 3, 2018
1 parent 36c80f2 commit 62c1901
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 125 deletions.
70 changes: 50 additions & 20 deletions src/main/java/com/android/volley/toolbox/HurlStack.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.android.volley.Request;
import com.android.volley.Request.Method;
import java.io.DataOutputStream;
import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
Expand All @@ -33,7 +34,7 @@
import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLSocketFactory;

/** An {@link HttpStack} based on {@link HttpURLConnection}. */
/** A {@link BaseHttpStack} based on {@link HttpURLConnection}. */
public class HurlStack extends BaseHttpStack {

private static final int HTTP_CONTINUE = 100;
Expand Down Expand Up @@ -84,27 +85,37 @@ public HttpResponse executeRequest(Request<?> request, Map<String, String> addit
}
URL parsedUrl = new URL(url);
HttpURLConnection connection = openConnection(parsedUrl, request);
for (String headerName : map.keySet()) {
connection.addRequestProperty(headerName, map.get(headerName));
}
setConnectionParametersForRequest(connection, request);
// Initialize HttpResponse with data from the HttpURLConnection.
int responseCode = connection.getResponseCode();
if (responseCode == -1) {
// -1 is returned by getResponseCode() if the response code could not be retrieved.
// Signal to the caller that something was wrong with the connection.
throw new IOException("Could not retrieve response code from HttpUrlConnection.");
}
boolean keepConnectionOpen = false;
try {
for (String headerName : map.keySet()) {
connection.addRequestProperty(headerName, map.get(headerName));
}
setConnectionParametersForRequest(connection, request);
// Initialize HttpResponse with data from the HttpURLConnection.
int responseCode = connection.getResponseCode();
if (responseCode == -1) {
// -1 is returned by getResponseCode() if the response code could not be retrieved.
// Signal to the caller that something was wrong with the connection.
throw new IOException("Could not retrieve response code from HttpUrlConnection.");
}

if (!hasResponseBody(request.getMethod(), responseCode)) {
return new HttpResponse(responseCode, convertHeaders(connection.getHeaderFields()));
}
if (!hasResponseBody(request.getMethod(), responseCode)) {
return new HttpResponse(responseCode, convertHeaders(connection.getHeaderFields()));
}

return new HttpResponse(
responseCode,
convertHeaders(connection.getHeaderFields()),
connection.getContentLength(),
inputStreamFromConnection(connection));
// Need to keep the connection open until the stream is consumed by the caller. Wrap the
// stream such that close() will disconnect the connection.
keepConnectionOpen = true;
return new HttpResponse(
responseCode,
convertHeaders(connection.getHeaderFields()),
connection.getContentLength(),
new UrlConnectionInputStream(connection));
} finally {
if (!keepConnectionOpen) {
connection.disconnect();
}
}
}

@VisibleForTesting
Expand Down Expand Up @@ -137,6 +148,25 @@ private static boolean hasResponseBody(int requestMethod, int responseCode) {
&& responseCode != HttpURLConnection.HTTP_NOT_MODIFIED;
}

/**
* Wrapper for a {@link HttpURLConnection}'s InputStream which disconnects the connection on
* stream close.
*/
static class UrlConnectionInputStream extends FilterInputStream {
private final HttpURLConnection mConnection;

UrlConnectionInputStream(HttpURLConnection connection) {
super(inputStreamFromConnection(connection));
mConnection = connection;
}

@Override
public void close() throws IOException {
super.close();
mConnection.disconnect();
}
}

/**
* Initializes an {@link InputStream} from the given {@link HttpURLConnection}.
*
Expand Down
74 changes: 0 additions & 74 deletions src/test/java/com/android/volley/mock/MockHttpURLConnection.java

This file was deleted.

Loading

0 comments on commit 62c1901

Please sign in to comment.