netrc: don't ignore the login name specified with "--user"

- for "--netrc", don't ignore the login/password specified with "--user",
  only ignore the login/password in the URL.
  This restores the netrc behaviour of curl 7.61.1 and earlier.
- fix the documentation of CURL_NETRC_REQUIRED
- improve the detection of login/password changes when reading .netrc
- don't read .netrc if both login and password are already set

Fixes #3213
Closes #3224

6 files changed, 83 insertions, 55 deletions

diff --git a/docs/cmdline-opts/netrc-file.d b/docs/cmdline-opts/netrc-file.d
index 0b5d2400c..50126d255 100644
--- a/docs/cmdline-opts/netrc-file.d
+++ b/docs/cmdline-opts/netrc-file.d
@@ -5,7 +5,7 @@ Added: 7.21.5
 Mutexed: netrc
 ---
 This option is similar to --netrc, except that you provide the path (absolute
-or relative) to the netrc file that Curl should use.  You can only specify one
+or relative) to the netrc file that curl should use.  You can only specify one
 netrc file per invocation. If several --netrc-file options are provided,
 the last one will be used.
 
diff --git a/docs/libcurl/opts/CURLOPT_NETRC.3 b/docs/libcurl/opts/CURLOPT_NETRC.3
index ee19801ea..21178ef16 100644
--- a/docs/libcurl/opts/CURLOPT_NETRC.3
+++ b/docs/libcurl/opts/CURLOPT_NETRC.3
@@ -47,20 +47,21 @@ standard Unix ftp client does). It should only be readable by user.
 \fIlevel\fP should be set to one of the values described below.
 
 .IP CURL_NETRC_OPTIONAL
-The use of your \fI~/.netrc\fP file is optional, and information in the URL is
-to be preferred.  The file will be scanned for the host and user name (to
-find the password only) or for the host only, to find the first user name and
-password after that \fImachine\fP, which ever information is not specified in
-the URL.
+The use of the \fI~/.netrc\fP file is optional, and information in the URL is
+to be preferred.  The file will be scanned for the host and user name (to find
+the password only) or for the host only, to find the first user name and
+password after that \fImachine\fP, which ever information is not specified.
 
 Undefined values of the option will have this effect.
 .IP CURL_NETRC_IGNORED
-The library will ignore the file and use only the information in the URL.
+The library will ignore the \fI~/.netrc\fP file.
 
 This is the default.
 .IP CURL_NETRC_REQUIRED
-This value tells the library that use of the file is required, to ignore the
-information in the URL, and to search the file for the host only.
+The use of the \fI~/.netrc\fP file is required, and information in the URL is
+to be ignored.  The file will be scanned for the host and user name (to find
+the password only) or for the host only, to find the first user name and
+password after that \fImachine\fP, which ever information is not specified.
 .SH DEFAULT
 CURL_NETRC_IGNORED
 .SH PROTOCOLS
diff --git a/lib/netrc.c b/lib/netrc.c
index 1724b35b0..aba355b76 100644
--- a/lib/netrc.c
+++ b/lib/netrc.c
@@ -53,6 +53,8 @@ enum host_lookup_state {
 int Curl_parsenetrc(const char *host,
                     char **loginp,
                     char **passwordp,
+                    bool *login_changed,
+                    bool *password_changed,
                     char *netrcfile)
 {
   FILE *file;
@@ -164,7 +166,7 @@ int Curl_parsenetrc(const char *host,
             if(specific_login) {
               state_our_login = strcasecompare(login, tok);
             }
-            else {
+            else if(!login || strcmp(login, tok)) {
               if(login_alloc) {
                 free(login);
                 login_alloc = FALSE;
@@ -179,7 +181,8 @@ int Curl_parsenetrc(const char *host,
             state_login = 0;
           }
           else if(state_password) {
-            if(state_our_login || !specific_login) {
+            if((state_our_login || !specific_login)
+                && (!password || strcmp(password, tok))) {
               if(password_alloc) {
                 free(password);
                 password_alloc = FALSE;
@@ -211,15 +214,19 @@ int Curl_parsenetrc(const char *host,
 
     out:
     if(!retcode) {
+      *login_changed = FALSE;
+      *password_changed = FALSE;
       if(login_alloc) {
         if(*loginp)
           free(*loginp);
         *loginp = login;
+        *login_changed = TRUE;
       }
       if(password_alloc) {
         if(*passwordp)
           free(*passwordp);
         *passwordp = password;
+        *password_changed = TRUE;
       }
     }
     else {
diff --git a/lib/netrc.h b/lib/netrc.h
index d980166e6..fe3dc357e 100644
--- a/lib/netrc.h
+++ b/lib/netrc.h
@@ -26,6 +26,8 @@
 int Curl_parsenetrc(const char *host,
                     char **loginp,
                     char **passwordp,
+                    bool *login_changed,
+                    bool *password_changed,
                     char *filename);
   /* Assume: (*passwordp)[0]=0, host[0] != 0.
    * If (*loginp)[0] = 0, search for login and password within a machine
diff --git a/lib/url.c b/lib/url.c
index 0d5a13f99..121920c76 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -2999,6 +2999,20 @@ static CURLcode override_login(struct Curl_easy *data,
   bool user_changed = FALSE;
   bool passwd_changed = FALSE;
   CURLUcode uc;
+
+  if(data->set.use_netrc == CURL_NETRC_REQUIRED && conn->bits.user_passwd) {
+    /* ignore user+password in the URL */
+    if(*userp) {
+      Curl_safefree(*userp);
+      user_changed = TRUE;
+    }
+    if(*passwdp) {
+      Curl_safefree(*passwdp);
+      passwd_changed = TRUE;
+    }
+    conn->bits.user_passwd = FALSE; /* disable user+password */
+  }
+
   if(data->set.str[STRING_USERNAME]) {
     free(*userp);
     *userp = strdup(data->set.str[STRING_USERNAME]);
@@ -3025,16 +3039,15 @@ static CURLcode override_login(struct Curl_easy *data,
   }
 
   conn->bits.netrc = FALSE;
-  if(data->set.use_netrc != CURL_NETRC_IGNORED) {
-    char *nuser = NULL;
-    char *npasswd = NULL;
+  if(data->set.use_netrc != CURL_NETRC_IGNORED &&
+      (!*userp || !**userp || !*passwdp || !**passwdp)) {
+    bool netrc_user_changed = FALSE;
+    bool netrc_passwd_changed = FALSE;
     int ret;
 
-    if(data->set.use_netrc == CURL_NETRC_OPTIONAL)
-      nuser = *userp; /* to separate otherwise identical machines */
-
     ret = Curl_parsenetrc(conn->host.name,
-                          &nuser, &npasswd,
+                          userp, passwdp,
+                          &netrc_user_changed, &netrc_passwd_changed,
                           data->set.str[STRING_NETRC_FILE]);
     if(ret > 0) {
       infof(data, "Couldn't find host %s in the "
@@ -3051,31 +3064,11 @@ static CURLcode override_login(struct Curl_easy *data,
       conn->bits.netrc = TRUE;
       conn->bits.user_passwd = TRUE; /* enable user+password */
 
-      if(data->set.use_netrc == CURL_NETRC_OPTIONAL) {
-        /* prefer credentials outside netrc */
-        if(nuser && !*userp) {
-          free(*userp);
-          *userp = nuser;
-          user_changed = TRUE;
-        }
-        if(npasswd && !*passwdp) {
-          free(*passwdp);
-          *passwdp = npasswd;
-          passwd_changed = TRUE;
-        }
+      if(netrc_user_changed) {
+        user_changed = TRUE;
       }
-      else {
-        /* prefer netrc credentials */
-        if(nuser) {
-          free(*userp);
-          *userp = nuser;
-          user_changed = TRUE;
-        }
-        if(npasswd) {
-          free(*passwdp);
-          *passwdp = npasswd;
-          passwd_changed = TRUE;
-        }
+      if(netrc_passwd_changed) {
+        passwd_changed = TRUE;
       }
     }
   }
diff --git a/tests/unit/unit1304.c b/tests/unit/unit1304.c
index 83375f55d..6d8334c98 100644
--- a/tests/unit/unit1304.c
+++ b/tests/unit/unit1304.c
@@ -47,6 +47,8 @@ static void unit_stop(void)
 
 UNITTEST_START
   int result;
+  bool login_changed;
+  bool password_changed;
 
   static const char * const filename1 = "log/netrc1304";
   memcpy(filename, filename1, strlen(filename1));
@@ -54,7 +56,8 @@ UNITTEST_START
   /*
    * Test a non existent host in our netrc file.
    */
-  result = Curl_parsenetrc("test.example.com", &login, &password, filename);
+  result = Curl_parsenetrc("test.example.com", &login, &password,
+             &login_changed, &password_changed, filename);
   fail_unless(result == 1, "Host not found should return 1");
   abort_unless(password != NULL, "returned NULL!");
   fail_unless(password[0] == 0, "password should not have been changed");
@@ -67,13 +70,16 @@ UNITTEST_START
   free(login);
   login = strdup("me");
   abort_unless(login != NULL, "returned NULL!");
-  result = Curl_parsenetrc("example.com", &login, &password, filename);
-  fail_unless(result == 0, "Host should be found");
+  result = Curl_parsenetrc("example.com", &login, &password,
+             &login_changed, &password_changed, filename);
+  fail_unless(result == 0, "Host should have been found");
   abort_unless(password != NULL, "returned NULL!");
   fail_unless(password[0] == 0, "password should not have been changed");
+  fail_unless(!password_changed, "password should not have been changed");
   abort_unless(login != NULL, "returned NULL!");
   fail_unless(strncmp(login, "me", 2) == 0,
               "login should not have been changed");
+  fail_unless(!login_changed, "login should not have been changed");
 
   /*
    * Test a non existent login and host in our netrc file.
@@ -81,8 +87,9 @@ UNITTEST_START
   free(login);
   login = strdup("me");
   abort_unless(login != NULL, "returned NULL!");
-  result = Curl_parsenetrc("test.example.com", &login, &password, filename);
-  fail_unless(result == 1, "Host should be found");
+  result = Curl_parsenetrc("test.example.com", &login, &password,
+             &login_changed, &password_changed, filename);
+  fail_unless(result == 1, "Host not found should return 1");
   abort_unless(password != NULL, "returned NULL!");
   fail_unless(password[0] == 0, "password should not have been changed");
   abort_unless(login != NULL, "returned NULL!");
@@ -96,13 +103,16 @@ UNITTEST_START
   free(login);
   login = strdup("admi");
   abort_unless(login != NULL, "returned NULL!");
-  result = Curl_parsenetrc("example.com", &login, &password, filename);
-  fail_unless(result == 0, "Host should be found");
+  result = Curl_parsenetrc("example.com", &login, &password,
+             &login_changed, &password_changed, filename);
+  fail_unless(result == 0, "Host should have been found");
   abort_unless(password != NULL, "returned NULL!");
   fail_unless(password[0] == 0, "password should not have been changed");
+  fail_unless(!password_changed, "password should not have been changed");
   abort_unless(login != NULL, "returned NULL!");
   fail_unless(strncmp(login, "admi", 4) == 0,
               "login should not have been changed");
+  fail_unless(!login_changed, "login should not have been changed");
 
   /*
    * Test a non existent login (superstring of an existing one)
@@ -111,13 +121,16 @@ UNITTEST_START
   free(login);
   login = strdup("adminn");
   abort_unless(login != NULL, "returned NULL!");
-  result = Curl_parsenetrc("example.com", &login, &password, filename);
-  fail_unless(result == 0, "Host should be found");
+  result = Curl_parsenetrc("example.com", &login, &password,
+             &login_changed, &password_changed, filename);
+  fail_unless(result == 0, "Host should have been found");
   abort_unless(password != NULL, "returned NULL!");
   fail_unless(password[0] == 0, "password should not have been changed");
+  fail_unless(!password_changed, "password should not have been changed");
   abort_unless(login != NULL, "returned NULL!");
   fail_unless(strncmp(login, "adminn", 6) == 0,
               "login should not have been changed");
+  fail_unless(!login_changed, "login should not have been changed");
 
   /*
    * Test for the first existing host in our netrc file
@@ -126,13 +139,16 @@ UNITTEST_START
   free(login);
   login = strdup("");
   abort_unless(login != NULL, "returned NULL!");
-  result = Curl_parsenetrc("example.com", &login, &password, filename);
+  result = Curl_parsenetrc("example.com", &login, &password,
+             &login_changed, &password_changed, filename);
   fail_unless(result == 0, "Host should have been found");
   abort_unless(password != NULL, "returned NULL!");
   fail_unless(strncmp(password, "passwd", 6) == 0,
               "password should be 'passwd'");
+  fail_unless(password_changed, "password should have been changed");
   abort_unless(login != NULL, "returned NULL!");
   fail_unless(strncmp(login, "admin", 5) == 0, "login should be 'admin'");
+  fail_unless(login_changed, "login should have been changed");
 
   /*
    * Test for the first existing host in our netrc file
@@ -141,13 +157,16 @@ UNITTEST_START
   free(password);
   password = strdup("");
   abort_unless(password != NULL, "returned NULL!");
-  result = Curl_parsenetrc("example.com", &login, &password, filename);
+  result = Curl_parsenetrc("example.com", &login, &password,
+             &login_changed, &password_changed, filename);
   fail_unless(result == 0, "Host should have been found");
   abort_unless(password != NULL, "returned NULL!");
   fail_unless(strncmp(password, "passwd", 6) == 0,
               "password should be 'passwd'");
+  fail_unless(password_changed, "password should have been changed");
   abort_unless(login != NULL, "returned NULL!");
   fail_unless(strncmp(login, "admin", 5) == 0, "login should be 'admin'");
+  fail_unless(!login_changed, "login should not have been changed");
 
   /*
    * Test for the second existing host in our netrc file
@@ -159,13 +178,16 @@ UNITTEST_START
   free(login);
   login = strdup("");
   abort_unless(login != NULL, "returned NULL!");
-  result = Curl_parsenetrc("curl.example.com", &login, &password, filename);
+  result = Curl_parsenetrc("curl.example.com", &login, &password,
+             &login_changed, &password_changed, filename);
   fail_unless(result == 0, "Host should have been found");
   abort_unless(password != NULL, "returned NULL!");
   fail_unless(strncmp(password, "none", 4) == 0,
               "password should be 'none'");
+  fail_unless(password_changed, "password should have been changed");
   abort_unless(login != NULL, "returned NULL!");
   fail_unless(strncmp(login, "none", 4) == 0, "login should be 'none'");
+  fail_unless(login_changed, "login should have been changed");
 
   /*
    * Test for the second existing host in our netrc file
@@ -174,13 +196,16 @@ UNITTEST_START
   free(password);
   password = strdup("");
   abort_unless(password != NULL, "returned NULL!");
-  result = Curl_parsenetrc("curl.example.com", &login, &password, filename);
+  result = Curl_parsenetrc("curl.example.com", &login, &password,
+             &login_changed, &password_changed, filename);
   fail_unless(result == 0, "Host should have been found");
   abort_unless(password != NULL, "returned NULL!");
   fail_unless(strncmp(password, "none", 4) == 0,
               "password should be 'none'");
+  fail_unless(password_changed, "password should have been changed");
   abort_unless(login != NULL, "returned NULL!");
   fail_unless(strncmp(login, "none", 4) == 0, "login should be 'none'");
+  fail_unless(!login_changed, "login should not have been changed");
 
   /* TODO:
    * Test over the size limit password / login!