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

[Tim Ruehsen]

Update of bug #48811 (project wget):

  Status:   Confirmed => Fixed  
 Open/Closed:Open => Closed 
 Planned Release:None => 1.18.1 

___

Follow-up Comment #5:

Thank you. I pushed your code, slightly amended.


___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

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

[Vijeth Aradhya]

Follow-up Comment #4, bug #48811 (project wget):

I have attached a patch file which solves this issue perfectly. But, other
than that, I have a minor doubt on the priority problem, which can be looked
into later.

The man page of wget itself says that --ask-password and --password are
mutually exclusive, i.e they cannot be used simultaneously.

I also ran a test case for it -

wget --user=something --password=something2 --ask-password
https://www.example.com

ERROR MESSAGE :

Cannot specify both --ask-password and --password.
Usage: wget [OPTION]... [URL]...

So, IMO we don't really have to worry about the priority between
opt.http_passwd and opt.passwd? Is there any other reason why you think this
should be done?

And, that too for user, it is always command line argument ( there is no
prompt for user as of I know ), so even in that, any order can be given. And,
to my surprise it actually doesn't throw an error for giving both --user and
--http-user arguments?! 

wget --http-user=something2 --http-password=something3 --ask-password
www.example.com

I think here, --http-password should get priority ( although these cases
should throw an error? I could raise another issue? ).

wget --user=something --http-password=something3 --ask-password
www.example.com

I think here, --ask-password gets priority

So, shouldn't the password priority depend on what kind of username we get? (
whether it is general username --user or http username --http-user? )

Or is --ask-password only associated with --user, and NOT with --http-user or
--ftp-user?

To resolve unnecessary complications, like we discussed before, I have given
higher priority to opt.passwd when opt.ask_passwd is set.

Thank you,
Vijeth

(file #39310)
___

Additional Item Attachment:

File name: fixes-http.c-and-ftp.c-passwd-logic-error_varadhya.patch Size:3 KB


___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

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

[Tim Ruehsen]

Follow-up Comment #3, bug #48811 (project wget):

Thanks for working on this.

If you are working on that code anyways, please consider making it more
readable, e.g. like
  if (u->user)
*user = u->user;
  else if (opt.http_user)
*user = opt.http_user;
  else if (opt.user)
*user = opt.user;

 ... same for passwd ...

  if (opt.netrc && (!*user || !*password))
search_netrc (u->host, (const char **)user, (const char **)passwd, 0);

But that is not all... when opt.use_askpass is set and the user/password have
been retrieved by that mechanism, these values should have priority over
opt.http|ftp_user / opt.http|ftp_passwd, right ?

Similar with opt.ask_passwd.

___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

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

[Wajda, Piotr]

Hi All,
I've sent patch for this before already, but it hasn't been reviewed 
yet. Besides logic correction, I've rebuilt/cleaned up code a bit.


Piotr

On 20/12/16 00:12, Vijeth Aradhya wrote:

Follow-up Comment #2, bug #48811 (project wget):

Hi Basin,

I'm new to the wget community, but I have looked through the program logic for
ftp and http password and realized it is wrong.

BEFORE changes

Initially, search_netrc function was called first, and if it existed, the
password was assigned to passwd, and used in the next logic step ( this is
WRONG because if the user entered the password with --ask-password, then that
should be given priority ).

passwd = passwd ? passwd : ..

AFTER changes

I have changed the logic such that it first sees if there is an --ask-password
passwd stored, and if it is not there, then it uses the netrc password.

I have attached the corresponding patch file to this. Please have a look :-)

This would be my first contribution to wget (GNU) as well and so eagerly
waiting for the code review!

(file #39280)
___

Additional Item Attachment:

File name:
0001-Corrected-the-logic-for-ftp-and-http-password-usage-auth_varadhya.patch
Size:2 KB


___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

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

[Vijeth Aradhya]

Follow-up Comment #2, bug #48811 (project wget):

Hi Basin,

I'm new to the wget community, but I have looked through the program logic for
ftp and http password and realized it is wrong.

BEFORE changes

Initially, search_netrc function was called first, and if it existed, the
password was assigned to passwd, and used in the next logic step ( this is
WRONG because if the user entered the password with --ask-password, then that
should be given priority ).

passwd = passwd ? passwd : ..

AFTER changes

I have changed the logic such that it first sees if there is an --ask-password
passwd stored, and if it is not there, then it uses the netrc password.

I have attached the corresponding patch file to this. Please have a look :-)

This would be my first contribution to wget (GNU) as well and so eagerly
waiting for the code review!

(file #39280)
___

Additional Item Attachment:

File name:
0001-Corrected-the-logic-for-ftp-and-http-password-usage-auth_varadhya.patch
Size:2 KB


___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/

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

[pwa...@gmail.net.pl]

Sure.

W dniu 19.09.2016 o 19:07, Darshit Shah pisze:
This patch looks good. However, if we are touching this part of the 
code, I would like to take the opportunity to clean it up as well.


From a brief glance it looks like the code for check for username and 
password can be abstracted into its own function. This way we can 
improve readability and reduce redundant code.


Could you please refactor this so that both http and ftp use the same 
function to read the username and password?

* Wajda, Piotr  [160916 23:47]:
I've tried to tackle another bug. I've moved reading from .netrc file 
to the end, when no user and no passwd is defined (for both http and 
ftp).


Please review.

Thanks
Piotr


-- Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6

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

[Darshit Shah]

This patch looks good. However, if we are touching this part of the 
code, I would like to take the opportunity to clean it up as well.


From a brief glance it looks like the code for check for username and 
password can be abstracted into its own function. This way we can 
improve readability and reduce redundant code.


Could you please refactor this so that both http and ftp use the same 
function to read the username and password? 


* Wajda, Piotr  [160916 23:47]:
I've tried to tackle another bug. I've moved reading from .netrc file 
to the end, when no user and no passwd is defined (for both http and 
ftp).


Please review.

Thanks
Piotr


-- Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6


signature.asc
Description: PGP signature

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

[Wajda, Piotr]

I've tried to tackle another bug. I've moved reading from .netrc file to 
the end, when no user and no passwd is defined (for both http and ftp).


Please review.

Thanks
Piotr
diff --git a/src/ftp.c b/src/ftp.c
index 39f20fa..359cbce 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -360,11 +360,10 @@ 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);
+  passwd = opt.passwd ? opt.passwd : (u->passwd ? u->passwd : opt.ftp_passwd);
+  if(!user && !passwd) search_netrc (u->host, (const char **), (const 
char **), 1);
   if (!user) user = "anonymous";
-  passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd : opt.passwd);
   if (!passwd) passwd = "-wget@";

   dtsock = -1;
diff --git a/src/http.c b/src/http.c
index 7e2c4ec..9de5cc0 100644
--- a/src/http.c
+++ b/src/http.c
@@ -1877,10 +1877,9 @@ initialize_request (const struct url *u, struct 
http_stat *hs, int *dt, struct u

   /* 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);
+  *passwd = opt.passwd ? opt.passwd : (u->passwd ? u->passwd : 
opt.http_passwd);
+  if (!*user && !*passwd) search_netrc (u->host, (const char **)user, (const 
char **)passwd, 0);

   /* We only do "site-wide" authentication with "global" user/password
* values unless --auth-no-challange has been requested; URL user/password

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

[Ilya Basin]

URL:
  

 Summary: netrc password wins over interactive --ask-password
 Project: GNU Wget
Submitted by: basinilya
Submitted on: Thu 18 Aug 2016 06:18:26 AM GMT
Category: Program Logic
Severity: 3 - Normal
Priority: 5 - Normal
  Status: None
 Privacy: Public
 Assigned to: None
 Originator Name: 
Originator Email: 
 Open/Closed: Open
 Discussion Lock: Any
 Release: 1.18
Operating System: None
 Reproducibility: Every Time
   Fixed Release: None
 Planned Release: None
  Regression: None
   Work Required: None
  Patch Included: None

___

Details:

With '--ask-password' argument Wget still uses the password from ~/.netrc (at
least for ftp)




___

Reply to this item at:

  

___
  Message sent via/by Savannah
  http://savannah.gnu.org/