Re: [Bug-wget] [PATCH v2] bug #48811: netrc password wins over interactive --ask-password

2016-11-22 Thread Wajda, Piotr

Hi Tim,
Just sent new version of this patch.
Shouldn't be any memleaks now.

Piotr

On 21/10/16 12:36, Tim Rühsen wrote:

Hi Piotr,

please include netrc.h in utils.c.

In getftp():
  struct net_credentials *ftp_cred = malloc (sizeof *ftp_cred);
...
  ftp_cred = pick_credentials()

This looks like a memleak. Also, where do you free ftp_cred ?
Is it really necessary  to allocate ftp_cred on each call to getftp (just a
question, maybe it is) ?

Basically the same in gethttp()...

Try 'make check-valgrind' to test for some kinds of memleaks.

Regards, Tim

On Mittwoch, 19. Oktober 2016 11:53:22 CEST losgrandes wrote:

* src/ftp.c: Leverage new struct net_credentials and function
pick_credentials. pick_credentials is responsible for taking proper order
when selecting source of credentials. * src/http.c: Leverage new struct
net_credentials and function pick_credentials. * src/utils.c: New function
pick_credentials.
* src/utils.h: New struct net_credentials.

---
 src/ftp.c   | 19 +++
 src/http.c  | 18 +++---
 src/utils.c | 19 +++
 src/utils.h | 11 +++
 4 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/src/ftp.c b/src/ftp.c
index 39f20fa..cc98ca3 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -327,7 +327,8 @@ getftp (struct url *u, struct url *original_url,
   uerr_t err = RETROK;  /* appease the compiler */
   FILE *fp = NULL;
   char *respline, *tms;
-  const char *user, *passwd, *tmrate;
+  const char *tmrate;
+  struct net_credentials *ftp_cred = malloc (sizeof *ftp_cred);
   int cmd = con->cmd;
   wgint expected_bytes = 0;
   bool got_expected_bytes = false;
@@ -359,13 +360,7 @@ getftp (struct url *u, struct url *original_url,

   *qtyread = restval;

-  user = u->user;
-  passwd = u->passwd;
-  search_netrc (u->host, (const char **), (const char **), 1);
-  user = user ? user : (opt.ftp_user ? opt.ftp_user : opt.user);
-  if (!user) user = "anonymous";
-  passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd :
opt.passwd); -  if (!passwd) passwd = "-wget@";
+  ftp_cred = pick_credentials (u, opt.ftp_user, opt.ftp_passwd, opt.user,
opt.passwd, 1);

   dtsock = -1;
   local_sock = -1;
@@ -461,18 +456,18 @@ getftp (struct url *u, struct url *original_url,

   /* Second: Login with proper USER/PASS sequence.  */
   logprintf (LOG_VERBOSE, _("Logging in as %s ... "),
- quotearg_style (escape_quoting_style, user));
+ quotearg_style (escape_quoting_style, ftp_cred->user));
   if (opt.server_response)
 logputs (LOG_ALWAYS, "\n");
   if (con->proxy)
 {
   /* If proxy is in use, log in as username@target-site. */
-  char *logname = concat_strings (user, "@", u->host, (char *) 0);
-  err = ftp_login (csock, logname, passwd);
+  char *logname = concat_strings (ftp_cred->user, "@", u->host,
(char *) 0); +  err = ftp_login (csock, logname, ftp_cred->passwd);
   xfree (logname);
 }
   else
-err = ftp_login (csock, user, passwd);
+err = ftp_login (csock, ftp_cred->user, ftp_cred->passwd);

   /* FTPRERR, FTPSRVERR, WRITEFAILED, FTPLOGREFUSED, FTPLOGINC */
   switch (err)
diff --git a/src/http.c b/src/http.c
index 7e2c4ec..41eaa42 100644
--- a/src/http.c
+++ b/src/http.c
@@ -1813,7 +1813,7 @@ time_to_rfc1123 (time_t time, char *buf, size_t
bufsize) static struct request *
 initialize_request (const struct url *u, struct http_stat *hs, int *dt,
struct url *proxy, bool inhibit_keep_alive, bool *basic_auth_finished, -
wgint *body_data_size, char **user, char **passwd, uerr_t
*ret) +wgint *body_data_size, struct net_credentials
**http_cred, uerr_t *ret) {
   bool head_only = !!(*dt & HEAD_ONLY);
   struct request *req;
@@ -1876,20 +1876,16 @@ initialize_request (const struct url *u, struct
http_stat *hs, int *dt, struct u request_set_header (req,
"Accept-Encoding", "identity", rel_none);

   /* Find the username and password for authentication. */
-  *user = u->user;
-  *passwd = u->passwd;
-  search_netrc (u->host, (const char **)user, (const char **)passwd, 0);
-  *user = *user ? *user : (opt.http_user ? opt.http_user : opt.user);
-  *passwd = *passwd ? *passwd : (opt.http_passwd ? opt.http_passwd :
opt.passwd); +  *http_cred = pick_credentials (u, opt.http_user,
opt.http_passwd, opt.user, opt.passwd, 0);

   /* We only do "site-wide" authentication with "global" user/password
* values unless --auth-no-challange has been requested; URL
user/password * info overrides. */
-  if (*user && *passwd && (!u->user || opt.auth_without_challenge))
+  if ((*http_cred)->user && (*http_cred)->passwd && (!u->user ||
opt.auth_without_challenge)) {
   /* If this is a host for which we've already received a Basic
* challenge, we'll go ahead and send Basic authentication creds. */
-  *basic_auth_finished = maybe_send_basic_creds (u->host, *user,
*passwd, req); + 

Re: [Bug-wget] [PATCH v2] bug #48811: netrc password wins over interactive --ask-password

2016-10-21 Thread Tim Rühsen
Hi Piotr,

please include netrc.h in utils.c.

In getftp(): 
  struct net_credentials *ftp_cred = malloc (sizeof *ftp_cred);
...
  ftp_cred = pick_credentials()

This looks like a memleak. Also, where do you free ftp_cred ?
Is it really necessary  to allocate ftp_cred on each call to getftp (just a 
question, maybe it is) ?

Basically the same in gethttp()...

Try 'make check-valgrind' to test for some kinds of memleaks.

Regards, Tim

On Mittwoch, 19. Oktober 2016 11:53:22 CEST losgrandes wrote:
> * src/ftp.c: Leverage new struct net_credentials and function
> pick_credentials. pick_credentials is responsible for taking proper order
> when selecting source of credentials. * src/http.c: Leverage new struct
> net_credentials and function pick_credentials. * src/utils.c: New function
> pick_credentials.
> * src/utils.h: New struct net_credentials.
> 
> ---
>  src/ftp.c   | 19 +++
>  src/http.c  | 18 +++---
>  src/utils.c | 19 +++
>  src/utils.h | 11 +++
>  4 files changed, 44 insertions(+), 23 deletions(-)
> 
> diff --git a/src/ftp.c b/src/ftp.c
> index 39f20fa..cc98ca3 100644
> --- a/src/ftp.c
> +++ b/src/ftp.c
> @@ -327,7 +327,8 @@ getftp (struct url *u, struct url *original_url,
>uerr_t err = RETROK;  /* appease the compiler */
>FILE *fp = NULL;
>char *respline, *tms;
> -  const char *user, *passwd, *tmrate;
> +  const char *tmrate;
> +  struct net_credentials *ftp_cred = malloc (sizeof *ftp_cred);
>int cmd = con->cmd;
>wgint expected_bytes = 0;
>bool got_expected_bytes = false;
> @@ -359,13 +360,7 @@ getftp (struct url *u, struct url *original_url,
> 
>*qtyread = restval;
> 
> -  user = u->user;
> -  passwd = u->passwd;
> -  search_netrc (u->host, (const char **), (const char **), 1);
> -  user = user ? user : (opt.ftp_user ? opt.ftp_user : opt.user);
> -  if (!user) user = "anonymous";
> -  passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd :
> opt.passwd); -  if (!passwd) passwd = "-wget@";
> +  ftp_cred = pick_credentials (u, opt.ftp_user, opt.ftp_passwd, opt.user,
> opt.passwd, 1);
> 
>dtsock = -1;
>local_sock = -1;
> @@ -461,18 +456,18 @@ getftp (struct url *u, struct url *original_url,
> 
>/* Second: Login with proper USER/PASS sequence.  */
>logprintf (LOG_VERBOSE, _("Logging in as %s ... "),
> - quotearg_style (escape_quoting_style, user));
> + quotearg_style (escape_quoting_style, ftp_cred->user));
>if (opt.server_response)
>  logputs (LOG_ALWAYS, "\n");
>if (con->proxy)
>  {
>/* If proxy is in use, log in as username@target-site. */
> -  char *logname = concat_strings (user, "@", u->host, (char *) 0);
> -  err = ftp_login (csock, logname, passwd);
> +  char *logname = concat_strings (ftp_cred->user, "@", u->host,
> (char *) 0); +  err = ftp_login (csock, logname, ftp_cred->passwd);
>xfree (logname);
>  }
>else
> -err = ftp_login (csock, user, passwd);
> +err = ftp_login (csock, ftp_cred->user, ftp_cred->passwd);
> 
>/* FTPRERR, FTPSRVERR, WRITEFAILED, FTPLOGREFUSED, FTPLOGINC */
>switch (err)
> diff --git a/src/http.c b/src/http.c
> index 7e2c4ec..41eaa42 100644
> --- a/src/http.c
> +++ b/src/http.c
> @@ -1813,7 +1813,7 @@ time_to_rfc1123 (time_t time, char *buf, size_t
> bufsize) static struct request *
>  initialize_request (const struct url *u, struct http_stat *hs, int *dt,
> struct url *proxy, bool inhibit_keep_alive, bool *basic_auth_finished, -   
> wgint *body_data_size, char **user, char **passwd, uerr_t
> *ret) +wgint *body_data_size, struct net_credentials
> **http_cred, uerr_t *ret) {
>bool head_only = !!(*dt & HEAD_ONLY);
>struct request *req;
> @@ -1876,20 +1876,16 @@ initialize_request (const struct url *u, struct
> http_stat *hs, int *dt, struct u request_set_header (req,
> "Accept-Encoding", "identity", rel_none);
> 
>/* Find the username and password for authentication. */
> -  *user = u->user;
> -  *passwd = u->passwd;
> -  search_netrc (u->host, (const char **)user, (const char **)passwd, 0);
> -  *user = *user ? *user : (opt.http_user ? opt.http_user : opt.user);
> -  *passwd = *passwd ? *passwd : (opt.http_passwd ? opt.http_passwd :
> opt.passwd); +  *http_cred = pick_credentials (u, opt.http_user,
> opt.http_passwd, opt.user, opt.passwd, 0);
> 
>/* We only do "site-wide" authentication with "global" user/password
> * values unless --auth-no-challange has been requested; URL
> user/password * info overrides. */
> -  if (*user && *passwd && (!u->user || opt.auth_without_challenge))
> +  if ((*http_cred)->user && (*http_cred)->passwd && (!u->user ||
> opt.auth_without_challenge)) {
>/* If this is a host for which we've already received a Basic
> * challenge, we'll go ahead and send Basic authentication creds. */
> - 

[Bug-wget] [PATCH v2] bug #48811: netrc password wins over interactive --ask-password

2016-10-19 Thread losgrandes
* src/ftp.c: Leverage new struct net_credentials and function pick_credentials. 
  
  pick_credentials is responsible for taking proper order when selecting source 
of credentials. 
* src/http.c: Leverage new struct net_credentials and function 
pick_credentials.   
* src/utils.c: New function pick_credentials. 
* src/utils.h: New struct net_credentials.

---
 src/ftp.c   | 19 +++
 src/http.c  | 18 +++---
 src/utils.c | 19 +++
 src/utils.h | 11 +++
 4 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/src/ftp.c b/src/ftp.c
index 39f20fa..cc98ca3 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -327,7 +327,8 @@ getftp (struct url *u, struct url *original_url,
   uerr_t err = RETROK;  /* appease the compiler */
   FILE *fp = NULL;
   char *respline, *tms;
-  const char *user, *passwd, *tmrate;
+  const char *tmrate;
+  struct net_credentials *ftp_cred = malloc (sizeof *ftp_cred);
   int cmd = con->cmd;
   wgint expected_bytes = 0;
   bool got_expected_bytes = false;
@@ -359,13 +360,7 @@ getftp (struct url *u, struct url *original_url,
 
   *qtyread = restval;
 
-  user = u->user;
-  passwd = u->passwd;
-  search_netrc (u->host, (const char **), (const char **), 1);
-  user = user ? user : (opt.ftp_user ? opt.ftp_user : opt.user);
-  if (!user) user = "anonymous";
-  passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd : opt.passwd);
-  if (!passwd) passwd = "-wget@";
+  ftp_cred = pick_credentials (u, opt.ftp_user, opt.ftp_passwd, opt.user, 
opt.passwd, 1);
 
   dtsock = -1;
   local_sock = -1;
@@ -461,18 +456,18 @@ getftp (struct url *u, struct url *original_url,
 
   /* Second: Login with proper USER/PASS sequence.  */
   logprintf (LOG_VERBOSE, _("Logging in as %s ... "),
- quotearg_style (escape_quoting_style, user));
+ quotearg_style (escape_quoting_style, ftp_cred->user));
   if (opt.server_response)
 logputs (LOG_ALWAYS, "\n");
   if (con->proxy)
 {
   /* If proxy is in use, log in as username@target-site. */
-  char *logname = concat_strings (user, "@", u->host, (char *) 0);
-  err = ftp_login (csock, logname, passwd);
+  char *logname = concat_strings (ftp_cred->user, "@", u->host, (char 
*) 0);
+  err = ftp_login (csock, logname, ftp_cred->passwd);
   xfree (logname);
 }
   else
-err = ftp_login (csock, user, passwd);
+err = ftp_login (csock, ftp_cred->user, ftp_cred->passwd);
 
   /* FTPRERR, FTPSRVERR, WRITEFAILED, FTPLOGREFUSED, FTPLOGINC */
   switch (err)
diff --git a/src/http.c b/src/http.c
index 7e2c4ec..41eaa42 100644
--- a/src/http.c
+++ b/src/http.c
@@ -1813,7 +1813,7 @@ time_to_rfc1123 (time_t time, char *buf, size_t bufsize)
 static struct request *
 initialize_request (const struct url *u, struct http_stat *hs, int *dt, struct 
url *proxy,
 bool inhibit_keep_alive, bool *basic_auth_finished,
-wgint *body_data_size, char **user, char **passwd, uerr_t 
*ret)
+wgint *body_data_size, struct net_credentials **http_cred, 
uerr_t *ret)
 {
   bool head_only = !!(*dt & HEAD_ONLY);
   struct request *req;
@@ -1876,20 +1876,16 @@ initialize_request (const struct url *u, struct 
http_stat *hs, int *dt, struct u
   request_set_header (req, "Accept-Encoding", "identity", rel_none);
 
   /* Find the username and password for authentication. */
-  *user = u->user;
-  *passwd = u->passwd;
-  search_netrc (u->host, (const char **)user, (const char **)passwd, 0);
-  *user = *user ? *user : (opt.http_user ? opt.http_user : opt.user);
-  *passwd = *passwd ? *passwd : (opt.http_passwd ? opt.http_passwd : 
opt.passwd);
+  *http_cred = pick_credentials (u, opt.http_user, opt.http_passwd, opt.user, 
opt.passwd, 0);
 
   /* We only do "site-wide" authentication with "global" user/password
* values unless --auth-no-challange has been requested; URL user/password
* info overrides. */
-  if (*user && *passwd && (!u->user || opt.auth_without_challenge))
+  if ((*http_cred)->user && (*http_cred)->passwd && (!u->user || 
opt.auth_without_challenge))
 {
   /* If this is a host for which we've already received a Basic
* challenge, we'll go ahead and send Basic authentication creds. */
-  *basic_auth_finished = maybe_send_basic_creds (u->host, *user, *passwd, 
req);
+  *basic_auth_finished = maybe_send_basic_creds (u->host, 
(*http_cred)->user, (*http_cred)->passwd, req);
 }
 
   /* Generate the Host header, HOST:PORT.  Take into account that:
@@ -2920,7 +2916,7 @@ gethttp (const struct url *u, struct url *original_url, 
struct http_stat *hs,
   struct request *req = NULL;
 
   char *type = NULL;
-  char *user, *passwd;
+  struct net_credentials *http_cred = malloc (sizeof *http_cred);
   char *proxyauth;
   int statcode;
   int write_error;
@@ -3029,7 +3025,7 @@ gethttp (const struct url *u, struct url