Bugs item #2080222, was opened at 2008-08-28 10:45
Message generated for change (Comment added) made by vila-sf
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2080222&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: http
Group: bad behaviour
Status: Open
Resolution: None
Priority: 6
Private: No
Submitted By: vila-sf (vila-sf)
Assigned to: Daniel Stenberg (bagder)
Summary: race condition while POSTing to HTTP/1.0 servers

Initial Comment:
As explained at https://bugs.launchpad.net/bzr/+bug/225020, it seems that in rare occurrences curl/libcurl try to send the body of a POST request while the server has already sent a 403 response.

The version used is:
curl 7.18.0 (i486-pc-linux-gnu) libcurl/7.18.0 OpenSSL/0.9.8g zlib/1.2.3.3 libidn/1.1 on ubuntu hardy.

It occurs rarely and only with HTTP/1.0 servers.

It has been mentioned that a change in socket behaviour in the kernel may be provoking this bug.

The link above provides a wireshark capture of the transaction.

----------------------------------------------------------------------

>Comment By: vila-sf (vila-sf)
Date: 2008-08-29 12:23

Message:
Logged In: YES
user_id=1584517
Originator: YES

> There have been other POST-related issues fixed since 7.18.0 so I would
>rather argue that you should run at least 7.18.2.

It will be a pleasure :)

Unfortunately I can't upgrade all machines for my users so I worked around
the problem.

But since you were responsive enough to provide a patch, the least I could
do was to try it :)

> But that said, it's easy to adapt the patch for 7.18.0:

Thanks a ton.

I applied it locally and used the resulting library against my test
suite.

This indeed fixes the bug for 403 but also 501 (I had 23 tests failing
without your patch and without my workaround). Also note that it introduced
no regression while talking with HTTP/1.1 servers.

By the way, can you confirm that it's safe for my workaround to access the
error code and the headers of the response ?

Final request: in what official versions will that patch be included ?

That will help me diagnose other occurrences of the problem in other
configurations and give me some rules to recommend upgrades.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2008-08-29 10:48

Message:
Logged In: YES
user_id=1110
Originator: NO

There have been other POST-related issues fixed since 7.18.0 so I would
rather argue that you should run at least 7.18.2. But that said, it's easy
to adapt the patch for 7.18.0:

--- transfer.c~ 2008-01-16 13:24:00.000000000 +0100
+++ transfer.c 2008-08-29 10:46:14.000000000 +0200
@@ -1396,6 +1396,16 @@

       } while(data_pending(conn));

+ if(((k->keepon & (KEEP_READ|KEEP_WRITE)) == KEEP_WRITE) &&
+ conn->bits.close ) {
+ /* When we've read the entire thing and the close bit is set,
the
+ server may now close the connection. If there's now any kind
of
+ sending going on from our side, we need to stop that
+ immediately. */
+ infof(data, "we are done reading and this is set to close, stop
send\n");
+ k->keepon &= ~KEEP_WRITE; /* no writing anymore either */
+ }
+
     } /* if( read from socket ) */

     /* If we still have writing to do, we check if we have a writable

----------------------------------------------------------------------

Comment By: vila-sf (vila-sf)
Date: 2008-08-29 10:32

Message:
Logged In: YES
user_id=1584517
Originator: YES

I have version 7-18.0 here and I would like to validate your patch since I
have a test suite to exercise it against.

Unfortunately the patch doesn't apply cleanly :-/

Could you provide a patch against:

 * $Id: transfer.c,v 1.381 2008-01-16 12:24:00 bagder Exp $

In this file, there is a single occurrence of 'while(data_pending(conn));'
but the following line is not 'return CURLE_OK;', not at all :-/

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2008-08-28 13:00

Message:
Logged In: YES
user_id=1110
Originator: NO

Here's my first shot. This is a diff against current CVS but I think it
can be adjusted to older libcurls fairly easy:

--- lib/transfer.c 26 Aug 2008 21:28:57 -0000 1.404
+++ lib/transfer.c 28 Aug 2008 11:00:09 -0000
@@ -656,6 +656,15 @@

   } while(data_pending(conn));

+ if(((k->keepon & (KEEP_READ|KEEP_WRITE)) == KEEP_WRITE) &&
+ conn->bits.close ) {
+ /* When we've read the entire thing and the close bit is set, the
server may
+ now close the connection. If there's now any kind of sending going
on from
+ our side, we need to stop that immediately. */
+ infof(data, "we are done reading and this is set to close, stop
send\n");
+ k->keepon &= ~KEEP_WRITE; /* no writing anymore either */
+ }
+
   return CURLE_OK;
 }

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2008-08-28 11:44

Message:
Logged In: YES
user_id=1110
Originator: NO

I've now (locally) modified the curl test suite so that I was possible to
write up a test case that repeats this behavior. I'll now see what I can do
to make it behave...

----------------------------------------------------------------------

Comment By: vila-sf (vila-sf)
Date: 2008-08-28 11:29

Message:
Logged In: YES
user_id=1584517
Originator: YES

I can confirm that if curl wasn't trying to send the body when a 403
response has been received, it will indeed fix my use case. Thanks to
badger for linking to the other bug.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2008-08-28 10:58

Message:
Logged In: YES
user_id=1110
Originator: NO

This looks related to the previous mentions of libcurl not properly
stopping the sending when 400+ codes are received:

http://curl.haxx.se/mail/archive-2008-08/0138.html

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2080222&group_id=976
Received on 2008-08-29