Skip to content

Commit

Permalink
Always close HttpURLConnections in HurlStack.
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 google#138
  • Loading branch information
jpd236 committed May 3, 2018
1 parent 36c80f2 commit 1db436f
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}. */
/** An {@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 1db436f

Please sign in to comment.