From 2627dc5ee4ed60c0250ce5a9e99d586d480603f2 Mon Sep 17 00:00:00 2001
From: Stephen Connolly <stephen.alan.connolly@gmail.com>
Date: Thu, 23 Feb 2017 12:30:17 +0000
Subject: [PATCH] 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.
---
 src/main/java/org/kohsuke/github/GitHub.java  | 17 +++++++--
 .../java/org/kohsuke/github/Requester.java    | 38 +++++++++----------
 2 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java
index ccc18b450e..896fb36c63 100644
--- a/src/main/java/org/kohsuke/github/GitHub.java
+++ b/src/main/java/org/kohsuke/github/GitHub.java
@@ -47,13 +47,12 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.TimeZone;
-import java.util.concurrent.TimeUnit;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import org.apache.commons.codec.Charsets;
 import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.io.IOUtils;
 
 import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY;
 import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE;
@@ -703,8 +702,18 @@ private boolean isPrivateModeEnabled() {
                 Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
                 X-Content-Type-Options: nosniff
              */
-            return uc.getResponseCode() == HTTP_UNAUTHORIZED
-                && uc.getHeaderField("X-GitHub-Media-Type") != null;
+            try {
+                return uc.getResponseCode() == HTTP_UNAUTHORIZED
+                        && uc.getHeaderField("X-GitHub-Media-Type") != null;
+            } finally {
+                // ensure that the connection opened by getResponseCode gets closed
+                try {
+                    IOUtils.closeQuietly(uc.getInputStream());
+                } catch (IOException ignore) {
+                    // ignore
+                }
+                IOUtils.closeQuietly(uc.getErrorStream());
+            }
         } catch (IOException e) {
             return false;
         }
diff --git a/src/main/java/org/kohsuke/github/Requester.java b/src/main/java/org/kohsuke/github/Requester.java
index db70fbd64b..3cd338b8c2 100644
--- a/src/main/java/org/kohsuke/github/Requester.java
+++ b/src/main/java/org/kohsuke/github/Requester.java
@@ -48,7 +48,6 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.NoSuchElementException;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -642,6 +641,24 @@ private InputStream wrapStream(InputStream in) throws IOException {
                         " handling exception " + e, e);
             throw e;
         }
+        InputStream es = wrapStream(uc.getErrorStream());
+        if (es != null) {
+            try {
+                String error = IOUtils.toString(es, "UTF-8");
+                if (e instanceof FileNotFoundException) {
+                    // pass through 404 Not Found to allow the caller to handle it intelligently
+                    e = (IOException) new FileNotFoundException(error).initCause(e);
+                } else if (e instanceof HttpException) {
+                    HttpException http = (HttpException) e;
+                    e = new HttpException(error, http.getResponseCode(), http.getResponseMessage(),
+                            http.getUrl(), e);
+                } else {
+                    e = (IOException) new IOException(error).initCause(e);
+                }
+            } finally {
+                IOUtils.closeQuietly(es);
+            }
+        }
         if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED) // 401 / Unauthorized == bad creds
             throw e;
 
@@ -657,24 +674,7 @@ private InputStream wrapStream(InputStream in) throws IOException {
             return;
         }
 
-        InputStream es = wrapStream(uc.getErrorStream());
-        try {
-            if (es!=null) {
-                String error = IOUtils.toString(es, "UTF-8");
-                if (e instanceof FileNotFoundException) {
-                    // pass through 404 Not Found to allow the caller to handle it intelligently
-                    throw (IOException) new FileNotFoundException(error).initCause(e);
-                } else if (e instanceof HttpException) {
-                    HttpException http = (HttpException) e;
-                    throw (IOException) new HttpException(error, http.getResponseCode(), http.getResponseMessage(), http.getUrl(), e);
-                } else {
-                    throw (IOException) new IOException(error).initCause(e);
-                }
-            } else
-                throw e;
-        } finally {
-            IOUtils.closeQuietly(es);
-        }
+        throw e;
     }
 
     private static final List<String> METHODS_WITHOUT_BODY = asList("GET", "DELETE");