Re: [Bug-wget] [Bug-Wget] Wget closes connection despite Keep-Alive

[Darshit Shah]

On 11/04, Tim Rühsen wrote:

Am Dienstag, 4. November 2014, 23:25:40 schrieb Darshit Shah:

While looking at some debug output from Wget, I noticed that in case of a
416 Range Not Satisfiable response, Wget forces the connection close
despite the fact that the server explicitly sent a `Connection: Keep-Alive`
header.

Looking at the code, I found this at http.c:2775


      CLOSE_INVALIDATE (sock);        /* would be CLOSE_FINISH, but there
                                   might be more bytes in the body. */

I'm not sure what exactly we're trying to protect in this case. Why is
CLOSE_INVALIDATE required instead of CLOSE_FINISH? If the server responds
with a 416 status, there should be no more data in the stream and we should
be able to reuse that connection without any issues.

What am I missing here?

The code is with an if construct with || :
if (statcode == HTTP_STATUS_RANGE_NOT_SATISFIABLE
     || (!opt.timestamping && hs->restval > 0 && statcode == HTTP_STATUS_OK
         && contrange == 0 && contlen >= 0 && hs->restval >= contlen))
   {

So if statcode == HTTP_STATUS_RANGE_NOT_SATISFIABLE i guess we could use
CLOSE_FINISH. But I can't judge that for the other part of the expression.
If in doubt, CLOSE_INVALIDATE might be a better choice than CLOSE_FINISH.
Wget simply opens a new connection for the next request in this case.

I'm pretty sure that in case of `statcode == HTTP_STATUS_RANGE_NOT_SATISFIABLE` we can use CLOSE_FINISH(). The other condition is what I am confused about.

* Off-topic:
Looking at the condition, there's one comparison that we can avoid:
hs->restval > 0
contlen >= 0
hs->restval >= contlen

In this case, hs->restval > 0 is redundant and un-needed.

I wanted to write a test case to demonstrate this, but detecting a closed
socket will take more than a quick test case. I'll sit down and write the
required functionality to generate the relevant test case when I can.

Don't spend too much time here. It is not a bug, more of a missing feature.
And also a very seldom used one. And how I read the comment, servers might
behave buggy. And again, in that case it seems more reliable when Wget closes
the connection and opens a new one.

The reason why I came across this was that I needed Wget to maintain a persistent connection, and that was exactly what the server expected. However, Wget went ahead and closed the connection. This *missing feature* was precisely what I was looking for.

--
Thanking You,
Darshit Shah

Also, this behaviour will become a bug with SPDY and HTTP/2 since the server will try to use the open connection to suggest other resources which the client may ask for. I'm waiting for Giuseppe's views on this. Else, I'll split the condition into two parts, with the first one called CLOSE_FINISH instead of CLOSE_INVALIDATE

pgpZztcuk8cLV.pgp
Description: PGP signature