Skip to content

Commit 2627dc5

Browse files
stephencstephenc
committed
Ensure that connections are closed for error responses
- This was endless fun to trace, but I found it at last. This should stop the `WARNING: A connection to https://api.github.com/ was leaked. Did you forget to close a response body?` messages in the logs when using the OkHttpConnector.
1 parent 1f4325e commit 2627dc5

2 files changed

Lines changed: 32 additions & 23 deletions

File tree

‎src/main/java/org/kohsuke/github/GitHub.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,12 @@
4747
import java.util.Map;
4848
import java.util.Set;
4949
import java.util.TimeZone;
50-
import java.util.concurrent.TimeUnit;
51-
import java.util.logging.Level;
5250
import java.util.logging.Logger;
5351
import javax.annotation.CheckForNull;
5452
import javax.annotation.Nonnull;
5553
import org.apache.commons.codec.Charsets;
5654
import org.apache.commons.codec.binary.Base64;
55+
import org.apache.commons.io.IOUtils;
5756

5857
import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY;
5958
import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE;
@@ -703,8 +702,18 @@ private boolean isPrivateModeEnabled() {
703702
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
704703
X-Content-Type-Options: nosniff
705704
*/
706-
return uc.getResponseCode() == HTTP_UNAUTHORIZED
707-
&& uc.getHeaderField("X-GitHub-Media-Type") != null;
705+
try {
706+
return uc.getResponseCode() == HTTP_UNAUTHORIZED
707+
&& uc.getHeaderField("X-GitHub-Media-Type") != null;
708+
} finally {
709+
// ensure that the connection opened by getResponseCode gets closed
710+
try {
711+
IOUtils.closeQuietly(uc.getInputStream());
712+
} catch (IOException ignore) {
713+
// ignore
714+
}
715+
IOUtils.closeQuietly(uc.getErrorStream());
716+
}
708717
} catch (IOException e) {
709718
return false;
710719
}

‎src/main/java/org/kohsuke/github/Requester.java‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import java.util.Locale;
4949
import java.util.Map;
5050
import java.util.NoSuchElementException;
51-
import java.util.logging.Level;
5251
import java.util.logging.Logger;
5352
import java.util.regex.Matcher;
5453
import java.util.regex.Pattern;
@@ -642,6 +641,24 @@ private InputStream wrapStream(InputStream in) throws IOException {
642641
" handling exception " + e, e);
643642
throw e;
644643
}
644+
InputStream es = wrapStream(uc.getErrorStream());
645+
if (es != null) {
646+
try {
647+
String error = IOUtils.toString(es, "UTF-8");
648+
if (e instanceof FileNotFoundException) {
649+
// pass through 404 Not Found to allow the caller to handle it intelligently
650+
e = (IOException) new FileNotFoundException(error).initCause(e);
651+
} else if (e instanceof HttpException) {
652+
HttpException http = (HttpException) e;
653+
e = new HttpException(error, http.getResponseCode(), http.getResponseMessage(),
654+
http.getUrl(), e);
655+
} else {
656+
e = (IOException) new IOException(error).initCause(e);
657+
}
658+
} finally {
659+
IOUtils.closeQuietly(es);
660+
}
661+
}
645662
if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED) // 401 / Unauthorized == bad creds
646663
throw e;
647664

@@ -657,24 +674,7 @@ private InputStream wrapStream(InputStream in) throws IOException {
657674
return;
658675
}
659676

660-
InputStream es = wrapStream(uc.getErrorStream());
661-
try {
662-
if (es!=null) {
663-
String error = IOUtils.toString(es, "UTF-8");
664-
if (e instanceof FileNotFoundException) {
665-
// pass through 404 Not Found to allow the caller to handle it intelligently
666-
throw (IOException) new FileNotFoundException(error).initCause(e);
667-
} else if (e instanceof HttpException) {
668-
HttpException http = (HttpException) e;
669-
throw (IOException) new HttpException(error, http.getResponseCode(), http.getResponseMessage(), http.getUrl(), e);
670-
} else {
671-
throw (IOException) new IOException(error).initCause(e);
672-
}
673-
} else
674-
throw e;
675-
} finally {
676-
IOUtils.closeQuietly(es);
677-
}
677+
throw e;
678678
}
679679

680680
private static final List<String> METHODS_WITHOUT_BODY = asList("GET", "DELETE");

0 commit comments

Comments
 (0)