bugGNU Wget - Bugs: bug #48811, netrc password wins over...

 
 

bug #48811: netrc password wins over interactive --ask-password

Submitted by:  Ilya Basin <basinilya>
Submitted on:  Thu 18 Aug 2016 06:18:26 AM UTC  
 
Category: Program LogicSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Privacy: PublicAssigned to: None
Originator Name: Open/Closed: Closed
Release: 1.18Operating System: None
Reproducibility: Every TimeFixed Release: None
Planned Release: 1.18.1Regression: None
Work Required: NonePatch Included: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Fri 06 Jan 2017 04:05:28 PM UTC, comment #5:

Thank you. I pushed your code, slightly amended.

Tim Ruehsen <rockdaboot>
Project Administrator
Sat 24 Dec 2016 09:48:44 PM UTC, comment #4:

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 http://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 http://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)

Vijeth Aradhya <varadhya>
Tue 20 Dec 2016 09:57:46 AM UTC, comment #3:

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.

Tim Ruehsen <rockdaboot>
Project Administrator
Mon 19 Dec 2016 11:12:34 PM UTC, comment #2:

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)

Vijeth Aradhya <varadhya>
Thu 18 Aug 2016 08:24:07 PM UTC, comment #1:

Same logic for FTP and HTTP:

.netrc overrides all.

IMO, the order should be
1. from URL (URL specific)
2. from --ask-password (interactive)
3. --ftp-password / --http-password (protocol specific)
4. --password (ftp & http)
5. default value

Any opinions ?

Tim Ruehsen <rockdaboot>
Project Administrator
Thu 18 Aug 2016 06:18:26 AM UTC, original submission:

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

Ilya Basin <basinilya>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #39310:  fixes-http.c-and-ftp.c-passwd-logic-error_varadhya.patch added by varadhya (3KiB - text/x-patch - Hopefully, this fixes the issue :-) )
file #39280:  0001-Corrected-the-logic-for-ftp-and-http-password-usage-auth_varadhya.patch added by varadhya (2KiB - text/x-patch - One single patch file for the correction)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by varadhya (Updated the item)
  • -unavailable- added by rockdaboot (Posted a comment)
  • -unavailable- added by basinilya (Submitted the item)
  •  

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 06 Jan 2017 04:05:28 PM UTCrockdabootStatusConfirmed=>Fixed
      Open/ClosedOpen=>Closed
      Planned ReleaseNone=>1.18.1
    Sat 24 Dec 2016 09:48:44 PM UTCvaradhyaAttached File-=>Added fixes-http.c-and-ftp.c-passwd-logic-error_varadhya.patch, #39310
    Mon 19 Dec 2016 11:12:34 PM UTCvaradhyaAttached File-=>Added 0001-Corrected-the-logic-for-ftp-and-http-password-usage-auth_varadhya.patch, #39280
    Thu 18 Aug 2016 08:24:07 PM UTCrockdabootStatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1