On Thu, 21 Aug 2014, Paras S wrote:

Attached code reproduces the issue, at least part of it.

Thanks! Am I thinking right that I should replace that URL with one of my own that provides NTLM auth? The thing is, I don't really have any...

Anyway, I believe the correct fix for this problem should be something like in the patch I attach here. It moves the resetting of the easy handle state for auth to the point where it really should reset it: when a fresh connection is used.

Can you give it a shot and see how it behaves for your test case?

Note that I already committed 30f2d0c0b3 which moved away the wrong freeing of the URL.

--

 / daniel.haxx.se
From b2f262f43b1d80d995b2ec0750ef1f28a68a1787 Mon Sep 17 00:00:00 2001
From: Daniel Stenberg <[email protected]>
Date: Thu, 21 Aug 2014 11:20:19 +0200
Subject: [PATCH] disconnect: don't touch easy-related state on disconnects

This was done to make sure NTLM state that is bound to a connection
doesn't survive and gets used for the subsequent request - but
disconnects can also be done to for example make room in the connection
cache and thus that connection is not strictly related to the easy
handle's current operation.

The http authentication state is still kept in the easy handle since all
http auth _except_ NTLM is connection independent and thus survive over
multiple connections.

Bug: http://curl.haxx.se/mail/lib-2014-08/0148.html
Reported-by: Paras S
---
 lib/url.c | 52 +++++++++++++++++++++++++---------------------------
 1 file changed, 25 insertions(+), 27 deletions(-)

diff --git a/lib/url.c b/lib/url.c
index e43b19d..cdc4f7f 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -2600,10 +2600,20 @@ static void conn_free(struct connectdata *conn)
   Curl_free_ssl_config(&conn->ssl_config);
 
   free(conn); /* free all the connection oriented data */
 }
 
+/*
+ * Disconnects the given connection. Note the connection may not be the
+ * primary connection, like when freeing room in the connection cache or
+ * killing of a dead old connection.
+ *
+ * This function MUST NOT reset state in the SessionHandle struct if that
+ * isn't strictly bound to the life-time of *this* particular connection.
+ *
+ */
+
 CURLcode Curl_disconnect(struct connectdata *conn, bool dead_connection)
 {
   struct SessionHandle *data;
   if(!conn)
     return CURLE_OK; /* this is closed and fine already */
@@ -2619,34 +2629,10 @@ CURLcode Curl_disconnect(struct connectdata *conn, bool dead_connection)
     conn->dns_entry = NULL;
   }
 
   Curl_hostcache_prune(data); /* kill old DNS cache entries */
 
-  {
-    int has_host_ntlm = (conn->ntlm.state != NTLMSTATE_NONE);
-    int has_proxy_ntlm = (conn->proxyntlm.state != NTLMSTATE_NONE);
-
-    /* Authentication data is a mix of connection-related and sessionhandle-
-       related stuff. NTLM is connection-related so when we close the shop
-       we shall forget. */
-
-    if(has_host_ntlm) {
-      data->state.authhost.done = FALSE;
-      data->state.authhost.picked =
-        data->state.authhost.want;
-    }
-
-    if(has_proxy_ntlm) {
-      data->state.authproxy.done = FALSE;
-      data->state.authproxy.picked =
-        data->state.authproxy.want;
-    }
-
-    if(has_host_ntlm || has_proxy_ntlm)
-      data->state.authproblem = FALSE;
-  }
-
   /* Cleanup NTLM connection-related data */
   Curl_http_ntlm_cleanup(conn);
 
   if(conn->handler->disconnect)
     /* This is set if protocol-specific cleanups should be made */
@@ -2683,12 +2669,10 @@ CURLcode Curl_disconnect(struct connectdata *conn, bool dead_connection)
     signalPipeClose(conn->recv_pipe, TRUE);
   }
 
   conn_free(conn);
 
-  Curl_speedinit(data);
-
   return CURLE_OK;
 }
 
 /*
  * This function should return TRUE if the socket is to be assumed to
@@ -5615,10 +5599,25 @@ static CURLcode create_conn(struct SessionHandle *data,
        * This is a brand new connection, so let's store it in the connection
        * cache of ours!
        */
       ConnectionStore(data, conn);
     }
+
+    /* If NTLM is requested in a part of this connection, make sure we don't
+       assume the state is fine as this is a fresh connection and NTLM is
+       connection based. */
+    if((data->state.authhost.picked & (CURLAUTH_NTLM | CURLAUTH_NTLM_WB)) &&
+       data->state.authhost.done) {
+      infof(data, "NTLM picked AND auth done set, clear picked!\n");
+      data->state.authhost.picked = 0;
+    }
+    if((data->state.authproxy.picked & (CURLAUTH_NTLM | CURLAUTH_NTLM_WB)) &&
+       data->state.authproxy.done) {
+      infof(data, "NTLM-proxy picked AND auth done set, clear picked!\n");
+      data->state.authproxy.picked = 0;
+    }
+
   }
 
   /* Mark the connection as used */
   conn->inuse = TRUE;
 
@@ -6025,6 +6024,5 @@ CURLcode Curl_do_more(struct connectdata *conn, int *complete)
     /* do_complete must be called after the protocol-specific DO function */
     do_complete(conn);
 
   return result;
 }
-
-- 
2.1.0

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html

Reply via email to