Hello,

Here is my first attempt to tweak with the curl code. This patch is
not meant to be applied as-is (at least, it should be split into two
commits). It is only meant to check whether the attempted change is
going into the right direction.

The patch does not cause any functional changes (at least not by
intent ;-) and don't break any regression tests on my machine.

The first change in this patch is to factor out the (identical)
handling of host/proxy authentication from Curl_http_auth_act()
into the retry_auth() function (sidenote: I have to come up with
a better name for this function). This is to avoid introducing
yet more duplication in the code to come. My assumption here is
that credential retrieving will be identical for proxy and host.

The rest of this patch moves user_passwd/proxy_user_passwd flags
from conn->bits into conn->data.state.auth[host|proxy]. The aim
is to consolidate authentication information. Currently, auth
information is spread all over the data structures. I guess this
movement might go against libcurl's design. I don't have a clue
since I don't really understand the data structures yet.

While waiting for comments, I'll try to start a data structure
description in the INTERNALS paper.

Here's the patch:

Index: lib/http.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/http.c,v
retrieving revision 1.402
diff -u -r1.402 http.c
--- lib/http.c 28 Oct 2008 23:34:19 -0000 1.402
+++ lib/http.c 30 Oct 2008 16:55:32 -0000
@@ -419,6 +419,26 @@
   return CURLE_OK;
 }
 
+static bool retry_auth (struct connectdata *conn, bool isproxy)
+{
+ struct SessionHandle *data = conn->data;
+ struct UrlState *state = &data->state;
+ struct auth *authinfo = isproxy ? &state->authproxy : &state->authhost;
+ int httpcode = isproxy ? 407 : 401;
+ int pick = FALSE;
+
+ if (!authinfo->user_passwd) return FALSE; /* no authentication requested */
+
+ if ((data->req.httpcode == httpcode) ||
+ (conn->bits.authneg && data->req.httpcode < 300)) {
+ pick = pickoneauth(authinfo);
+ if (!pick)
+ state->authproblem = TRUE;
+ }
+
+ return pick;
+}
+
 /*
  * Curl_http_auth_act() gets called when all HTTP headers have been received
  * and it checks what authentication methods that are available and decides
@@ -429,8 +449,8 @@
 CURLcode Curl_http_auth_act(struct connectdata *conn)
 {
   struct SessionHandle *data = conn->data;
- bool pickhost = FALSE;
- bool pickproxy = FALSE;
+ bool retryhost = FALSE;
+ bool retryproxy = FALSE;
   CURLcode code = CURLE_OK;
 
   if(100 <= data->req.httpcode && 199 >= data->req.httpcode)
@@ -440,22 +460,10 @@
   if(data->state.authproblem)
     return data->set.http_fail_on_error?CURLE_HTTP_RETURNED_ERROR:CURLE_OK;
 
- if(conn->bits.user_passwd &&
- ((data->req.httpcode == 401) ||
- (conn->bits.authneg && data->req.httpcode < 300))) {
- pickhost = pickoneauth(&data->state.authhost);
- if(!pickhost)
- data->state.authproblem = TRUE;
- }
- if(conn->bits.proxy_user_passwd &&
- ((data->req.httpcode == 407) ||
- (conn->bits.authneg && data->req.httpcode < 300))) {
- pickproxy = pickoneauth(&data->state.authproxy);
- if(!pickproxy)
- data->state.authproblem = TRUE;
- }
+ retryhost = retry_auth (conn, FALSE);
+ retryproxy = retry_auth (conn, TRUE);
 
- if(pickhost || pickproxy) {
+ if(retryhost || retryproxy) {
     data->req.newurl = strdup(data->change.url); /* clone URL */
     if(!data->req.newurl)
       return CURLE_OUT_OF_MEMORY;
@@ -525,8 +533,7 @@
   authhost = &data->state.authhost;
   authproxy = &data->state.authproxy;
 
- if((conn->bits.httpproxy && conn->bits.proxy_user_passwd) ||
- conn->bits.user_passwd)
+ if((conn->bits.httpproxy && authproxy->user_passwd) || authhost->user_passwd)
     /* continue please */ ;
   else {
     authhost->done = TRUE;
@@ -573,7 +580,7 @@
 #endif
       if(authproxy->picked == CURLAUTH_BASIC) {
         /* Basic */
- if(conn->bits.proxy_user_passwd &&
+ if(authproxy->user_passwd &&
            !checkheaders(data, "Proxy-authorization:")) {
           auth="Basic";
           result = http_output_basic(conn, TRUE);
@@ -661,7 +668,7 @@
         } else
 #endif
         if(authhost->picked == CURLAUTH_BASIC) {
- if(conn->bits.user_passwd &&
+ if(authhost->user_passwd &&
              !checkheaders(data, "Authorization:")) {
             auth="Basic";
             result = http_output_basic(conn, FALSE);
@@ -895,9 +902,9 @@
   ** Either we're not authenticating, or we're supposed to
   ** be authenticating something else. This is an error.
   */
- if((httpcode == 401) && !conn->bits.user_passwd)
+ if((httpcode == 401) && !data->state.authhost.user_passwd)
     return TRUE;
- if((httpcode == 407) && !conn->bits.proxy_user_passwd)
+ if((httpcode == 407) && !data->state.authproxy.user_passwd)
     return TRUE;
 
   return data->state.authproblem;
Index: lib/dict.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/dict.c,v
retrieving revision 1.58
diff -u -r1.58 dict.c
--- lib/dict.c 23 Oct 2008 11:49:19 -0000 1.58
+++ lib/dict.c 30 Oct 2008 16:55:31 -0000
@@ -160,7 +160,7 @@
 
   *done = TRUE; /* unconditionally */
 
- if(conn->bits.user_passwd) {
+ if(data->state.authhost.user_passwd) {
     /* AUTH is missing */
   }
 
Index: lib/telnet.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/telnet.c,v
retrieving revision 1.106
diff -u -r1.106 telnet.c
--- lib/telnet.c 23 Oct 2008 11:49:19 -0000 1.106
+++ lib/telnet.c 30 Oct 2008 16:55:33 -0000
@@ -822,7 +822,7 @@
 
   /* Add the user name as an environment variable if it
      was given on the command line */
- if(conn->bits.user_passwd)
+ if(data->state.authhost.user_passwd)
   {
     snprintf(option_arg, sizeof(option_arg), "USER,%s", conn->user);
     tn->telnet_vars = curl_slist_append(tn->telnet_vars, option_arg);
Index: lib/transfer.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/transfer.c,v
retrieving revision 1.421
diff -u -r1.421 transfer.c
--- lib/transfer.c 29 Oct 2008 19:06:48 -0000 1.421
+++ lib/transfer.c 30 Oct 2008 16:55:34 -0000
@@ -1059,8 +1059,8 @@
          * are definitely errors, so give up here.
          */
         if(data->set.http_fail_on_error && (k->httpcode >= 400) &&
- ((k->httpcode != 401) || !conn->bits.user_passwd) &&
- ((k->httpcode != 407) || !conn->bits.proxy_user_passwd) ) {
+ ((k->httpcode != 401) || !data->state.authhost.user_passwd) &&
+ ((k->httpcode != 407) || !data->state.authproxy.user_passwd)) {
 
           if(data->state.resume_from &&
              (data->set.httpreq==HTTPREQ_GET) &&
Index: lib/url.c
===================================================================
RCS file: /cvsroot/curl/curl/lib/url.c,v
retrieving revision 1.769
diff -u -r1.769 url.c
--- lib/url.c 23 Oct 2008 11:49:19 -0000 1.769
+++ lib/url.c 30 Oct 2008 16:55:35 -0000
@@ -3485,7 +3485,7 @@
       }
 
       if(CURLE_OK == res) {
- conn->bits.proxy_user_passwd = TRUE; /* enable it */
+ data->state.authproxy.user_passwd = TRUE; /* enable it */
         atsign = strdup(atsign+1); /* the right side of the @-letter */
 
         if(atsign) {
@@ -3633,7 +3633,7 @@
       if(data->set.use_netrc != CURL_NETRC_REQUIRED) {
         /* We could use the one in the URL */
 
- conn->bits.user_passwd = 1; /* enable user+password */
+ data->state.authhost.user_passwd = 1; /* enable user+password */
 
         if(*userpass != ':') {
           /* the name is given, get user+password */
@@ -3794,7 +3794,7 @@
          different host or similar. */
       conn->bits.netrc = TRUE;
 
- conn->bits.user_passwd = 1; /* enable user+password */
+ data->state.authhost.user_passwd = 1; /* enable user+password */
     }
   }
 }
@@ -3807,7 +3807,7 @@
 {
   /* If our protocol needs a password and we have none, use the defaults */
   if( (conn->protocol & PROT_FTP) &&
- !conn->bits.user_passwd) {
+ !conn->data->state.authhost.user_passwd) {
 
     conn->user = strdup(CURL_DEFAULT_USER);
     conn->passwd = strdup(CURL_DEFAULT_PASSWORD);
@@ -3944,8 +3944,7 @@
 
   /* get the user+password information from the old_conn struct since it may
    * be new for this request even when we re-use an existing connection */
- conn->bits.user_passwd = old_conn->bits.user_passwd;
- if(conn->bits.user_passwd) {
+ if(conn->data->state.authhost.user_passwd) {
     /* use the new user name and password though */
     Curl_safefree(conn->user);
     Curl_safefree(conn->passwd);
@@ -3955,8 +3954,7 @@
     old_conn->passwd = NULL;
   }
 
- conn->bits.proxy_user_passwd = old_conn->bits.proxy_user_passwd;
- if(conn->bits.proxy_user_passwd) {
+ if(conn->data->state.authproxy.user_passwd) {
     /* use the new proxy user name and proxy password though */
     Curl_safefree(conn->proxyuser);
     Curl_safefree(conn->proxypasswd);
@@ -4055,8 +4053,8 @@
                                 && (conn->proxytype == CURLPROXY_HTTP));
 
 
- conn->bits.user_passwd = (bool)(NULL != data->set.str[STRING_USERNAME]);
- conn->bits.proxy_user_passwd = (bool)(NULL != data->set.str[STRING_PROXYUSERNAME]);
+ conn->data->state.authhost.user_passwd = (bool)(NULL != data->set.str[STRING_USERNAME]);
+ conn->data->state.authproxy.user_passwd = (bool)(NULL != data->set.str[STRING_PROXYUSERNAME]);
   conn->bits.tunnel_proxy = data->set.tunnel_thru_httpproxy;
   conn->bits.ftp_use_epsv = data->set.ftp_use_epsv;
   conn->bits.ftp_use_eprt = data->set.ftp_use_eprt;
@@ -4117,7 +4115,7 @@
   /*************************************************************
    * Extract the user and password from the authentication string
    *************************************************************/
- if(conn->bits.proxy_user_passwd) {
+ if(conn->data->state.authproxy.user_passwd) {
     result = parse_proxy_auth(data, conn);
     if(result != CURLE_OK)
         return result;
Index: lib/urldata.h
===================================================================
RCS file: /cvsroot/curl/curl/lib/urldata.h,v
retrieving revision 1.393
diff -u -r1.393 urldata.h
--- lib/urldata.h 25 Oct 2008 05:41:02 -0000 1.393
+++ lib/urldata.h 30 Oct 2008 16:55:36 -0000
@@ -577,8 +577,6 @@
   bool reuse; /* if set, this is a re-used connection */
   bool proxy; /* if set, this transfer is done through a proxy - any type */
   bool httpproxy; /* if set, this transfer is done through a http proxy */
- bool user_passwd; /* do we use user+password for this connection? */
- bool proxy_user_passwd; /* user+password for the proxy? */
   bool ipv6_ip; /* we communicate with a remote site specified with pure IPv6
                    IP address */
   bool ipv6; /* we communicate with a site using an IPv6 address */
@@ -1123,6 +1121,7 @@
 #define MAX_CURL_PASSWORD_LENGTH_TXT "255"
 
 struct auth {
+ bool user_passwd; /* do we use user+password for this connection? */
   long want; /* Bitmask set to the authentication methods wanted by the app
                  (with CURLOPT_HTTPAUTH or CURLOPT_PROXYAUTH). */
   long picked;
Received on 2008-10-30