[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Bug-wget] [PATCH] Due to keep_alive handling login to protected site us

From: Tomas Hozza
Subject: [Bug-wget] [PATCH] Due to keep_alive handling login to protected site using http://username:address@hidden/ does not work
Date: Thu, 11 Jul 2013 07:24:25 -0400 (EDT)


I already proposed this bug in Savannah (https://savannah.gnu.org/bugs/?38554)
and will copy the discussion we already had there to answer some questions.

----------------- BEGINNING --------------------

Wed 20 Mar 2013 11:52:09 AM GMT, original submission: (Tomas Hozza)

This problem exists for example when trying to download something
from Boa web server with Basic authentication.

Problem description from Red Hat Bugzilla [1] (on Fedora 18 with wget-1.13.4 
AND wget-1.14)

The problem is that boa server answers to the initial GET packet with two 401
Unauthorised packets (at least in my case). From your dumps I see that boa is
answering with only one packet, but it is somehow broken. Although wget seems
to interpret it correctly it will close the connection without setting opened
socket variable to -1. Then it fails trying to write to that already closed
socket. This repeats for default maximum retries times.

Reason why wget-1.12 works is how it handle connections. In wget-1.12 the
connection established for GET request is closed before authentication and
the socket variable is set to -1. So when wget tries to authenticate to the
server, it establishes new connection and everything works.

Reason why --user and --password options works is that those credentials are
used as default for site-wide authentication. What wget does in this case is
pretty much the same as when you pass credentials in the URL with one 
When wget receives 401 Unauthorised from server and have those "default"
credentials set AND have NOT credentials in URL, then it adds the host address
to a list of hosts which request authentication. After that it also closes the
initial connection, then fails to write new GET request with authentication
credentials. BUT when it tries for the second time it will find out that the
host address is in list of hosts which request authentication and therefore
it will send GET request including authentication credentials. So that's why
it works.

I think the proper fix for this would be to set the socket variable to -1
in case of a problem when authenticating and closing the socket. This will
make sure that wget will not try to write data to already closed socket
but rather it will create new connection.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=912358


Wed 20 Mar 2013 11:56:06 AM GMT, comment #1: (Tomas Hozza)

I think Bug https://savannah.gnu.org/bugs/?34141 may be connected
to this one. Sorry for the new Bug BUT in 34141 was no activity
what so ever so hopefully now it will be better.


Thu 04 Apr 2013 05:08:30 PM GMT, comment #2: (Giuseppe Scrivano)

wouldn't that leak the socket if it is not valid? Should you perhaps use 

Can you also add the entry to the src/ChangeLog file?


Fri 05 Apr 2013 07:42:13 AM GMT, comment #3: (Tomas Hozza)

Thank you for your response.

> wouldn't that leak the socket if it is not valid?

I scanned wget-1.14 without the patch and with the patch
using static analysis tool Coverity 6.5.1. It didn't find
any added errors, so there should be NO resource leak. I know
static analysis is not perfect, but from my experiences with
Coverity I must say it is very reliable.

Anyway there is the following construction:

if (persistent_available_p(...))
else if (host_lookup_failed)
else if (sock != -1)
sock = -1;

persistent_available_p(...) returns false only if there is NO active
connection OR if we want to use new connection (SSL) or if there
was some problem with connection and it was invalidated. In other
words if persistent_available_p(...) doesn't returned true, then we
have to create new connection anyway.

> Should you perhaps use CLOSE_INVALIDATE?

Problem is that persistent_available_p(...) deals with "pconn.socket"
while outside of this function we use "sock" variable. Even if we used
CLOSE_INVALIDATE() in persistent_available_p() we have to set "sock"
to -1 because we might be using already closed socket. Also calling
CLOSE_INVALIDATE() on "sock" if persistent_available_p() returned
false would be not a good idea, because we might be closing already
closed socket.

> Can you also add the entry to the src/ChangeLog file?

I attached new patch with entry in src/ChangeLog.

------------------------ END ---------------------------

So if anyone could have a look at the patch, review it and possibly
merge it into the git, it would be great. If you have any questions
regarding the patch, please let me know.


Tomas Hozza

Attachment: 0001-Set-sock-variable-to-1-if-no-persistent-conn-exists.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]