Merge pull request #250 from brandonjacob/master

Ensure disposal of native auth state after executing Http Method closes #250
microsoft · Oct 14, 2020 · 282be8a · 282be8a
2 parents 56fab3c + 649aee1
commit 282be8a
Show file tree

Hide file tree

Showing 9 changed files with 71 additions and 23 deletions.
diff --git a/...crosoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/HttpMethodDirector.java b/...crosoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/HttpMethodDirector.java
@@ -224,6 +224,8 @@ public void executeMethod(final HttpMethod method) throws IOException, HttpExcep
 
             } // end of retry loop
         } finally {
+            cleanupHostAndProxyAuthState(method);
+
             if (conn != null) {
                 conn.setLocked(false);
             }
@@ -237,6 +239,17 @@ public void executeMethod(final HttpMethod method) throws IOException, HttpExcep
             }
         }
     }
+
+    private void cleanupHostAndProxyAuthState(final HttpMethod method) {
+        cleanupAuthState(method.getHostAuthState());
+        cleanupAuthState(method.getProxyAuthState());
+    }
+
+    private void cleanupAuthState(AuthState authState) {
+        if (authState != null) {
+            authState.invalidate();
+        }
+    }
 
     private Credentials getPreemptiveCredentials(final String host, final int port) {
         if (host == null) {

diff --git a/....microsoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/AuthScheme.java b/....microsoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/AuthScheme.java
@@ -79,6 +79,11 @@ public interface AuthScheme {
      */
     public boolean supportsCredentials(Credentials credentials);
 
+    /**
+     * Perform optional cleanup of auth scheme state (e.g. dispose of native OS handles, etc..).
+     */
+    public void cleanup();
+
     /**
      * Processes the given challenge token. Some authentication schemes may
      * involve multiple challenge-response exchanges. Such schemes must be able

diff --git a/...m.microsoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/AuthState.java b/...m.microsoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/AuthState.java
@@ -76,7 +76,11 @@ public AuthState() {
      * Invalidates the authentication state by resetting its parameters.
      */
     public void invalidate() {
-        authScheme = null;
+        if (authScheme != null) {
+            authScheme.cleanup();
+            authScheme = null;
+        }
+
         authRequested = false;
         authAttempted = false;
         preemptive = false;
@@ -193,6 +197,10 @@ public void setAuthScheme(final AuthScheme authScheme) {
             preemptive = false;
             authAttempted = false;
         }
+
+        if (this.authScheme != null) {
+            this.authScheme.cleanup();
+        }
         this.authScheme = authScheme;
     }
 

diff --git a/...microsoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/BasicScheme.java b/...microsoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/BasicScheme.java
@@ -253,4 +253,9 @@ public static String authenticate(final UsernamePasswordCredentials credentials,
             + EncodingUtil.getAsciiString(Base64.encodeBase64(EncodingUtil.getBytes(buffer.toString(), charset)));
     }
 
+    @Override
+    public void cleanup() {
+
+    }
+
 }
diff --git a/...soft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/CookieAuthScheme.java b/...soft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/CookieAuthScheme.java
@@ -14,6 +14,11 @@ public boolean supportsCredentials(final Credentials credentials) {
         return (credentials instanceof CookieCredentials);
     }
 
+    @Override
+    public void cleanup() {
+        //No disposing of credentials needed
+    }
+
     @Override
     public void processChallenge(final String challenge) throws MalformedChallengeException {
         throw new MalformedChallengeException("Cookie authentication is not challenge/response"); //$NON-NLS-1$

diff --git a/...icrosoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/DigestScheme.java b/...icrosoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/DigestScheme.java
@@ -609,4 +609,9 @@ public static String createCnonce() {
 
         return cnonce;
     }
+
+    @Override
+    public void cleanup() {
+
+    }
 }
diff --git a/...crosoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/JwtAuthScheme.java b/...crosoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/JwtAuthScheme.java
@@ -21,6 +21,11 @@ public boolean supportsCredentials(final Credentials credentials) {
         return credentials instanceof JwtCredentials;
     }
 
+    @Override
+    public void cleanup() {
+        //No disposing of credentials needed
+    }
+
     @Override
     public void processChallenge(final String challenge) throws MalformedChallengeException {
         complete = true;

diff --git a/....microsoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/NTLMScheme.java b/....microsoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/NTLMScheme.java
@@ -70,6 +70,16 @@ public boolean supportsCredentials(final Credentials credentials) {
         return supportsCredentials(credentials.getClass());
     }
 
+    @Override
+    public void cleanup() {
+        if(ntlmClient != null) {
+            ntlmClient.dispose();
+            ntlmClient = null;
+        }
+
+        inputToken = null;
+    }
+
     public static boolean supportsCredentials(final Class<?> credentialClass) {
         if (credentialClass == null || !isSupported()) {
             return false;
@@ -221,11 +231,7 @@ else if (status != STATUS_EXCHANGING || ntlmClient == null || inputToken == null
             if (ntlmClient.isComplete()) {
                 status = STATUS_COMPLETE;
 
-                /* Clean up */
-                ntlmClient.dispose();
-
-                ntlmClient = null;
-                inputToken = null;
+                cleanup();
             } else {
                 status = STATUS_EXCHANGING;
             }

diff --git a/...osoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/NegotiateScheme.java b/...osoft.tfs.core.httpclient/src/com/microsoft/tfs/core/httpclient/auth/NegotiateScheme.java
@@ -70,6 +70,16 @@ public boolean supportsCredentials(final Credentials credentials) {
         return supportsCredentials(credentials.getClass());
     }
 
+    @Override
+    public void cleanup() {
+        if(negotiateClient != null) {
+            negotiateClient.dispose();
+            negotiateClient = null;
+        }
+
+        inputToken = null;
+    }
+
     public static boolean supportsCredentials(final Class<?> credentialClass) {
         if (credentialClass == null || !isSupported()) {
             return false;
@@ -150,16 +160,7 @@ public void processChallenge(final String challenge) throws MalformedChallengeEx
              * We may be called for retry at an arbitrary time. If that's the
              * case, tear down any existing objects
              */
-            if (negotiateClient != null) {
-                try {
-                    negotiateClient.dispose();
-                } catch (final Exception e) {
-                }
-            }
-
-            negotiateClient = null;
-            inputToken = null;
-
+            cleanup();
             status = STATUS_INITIATED;
         }
         /* The server has responded with a challenge to our request */
@@ -193,7 +194,7 @@ protected String authenticate(final AuthScope authscope, final Credentials crede
             if (status == STATUS_INITIATED && negotiateClient == null && inputToken == null) {
                 negotiateClient = (NegotiateClient) NegotiateEngine.getInstance().newClient();
                 negotiateClient.setTarget("http@" + authscope.getHost().toUpperCase());
-
+                
                 if (credentials instanceof DefaultNTCredentials) {
                     negotiateClient.setCredentialsDefault();
                 } else if (credentials instanceof UsernamePasswordCredentials) {
@@ -221,12 +222,7 @@ else if (status != STATUS_EXCHANGING || negotiateClient == null || inputToken ==
 
             if (negotiateClient.isComplete()) {
                 status = STATUS_COMPLETE;
-
-                /* Clean up */
-                negotiateClient.dispose();
-
-                negotiateClient = null;
-                inputToken = null;
+                cleanup();
             } else {
                 status = STATUS_EXCHANGING;
             }