Bugs item #1871269, was opened at 2008-01-14 17:44
Message generated for change (Comment added) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=1871269&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: libcurl
Group: hang
Status: Open
Resolution: None
Priority: 7
Private: No
Submitted By: Joe Malicki (jmalicki)
Assigned to: Daniel Stenberg (bagder)
Summary: Curl hangs when sending large request-headers

Initial Comment:
Using libcurl 7.13.2, 7.15.5 or 7.17.1 with software that sends extremely large (> ~40KB, whatever is greater than one syscall) request-headers as part of a post. The following patch fixes it, passes regression tests the same as the old curl, and has made it through several months of QA integration in a larger project with no issues:

This patch is against 7.15.5, and works with patch "fuzz" against 7.13.2 as well (7.17.1 requires a slightly different patch, still haven't tested fix for 7.17.1):

Index: lib/http.c
===================================================================
--- lib/http.c (revision 67598)
+++ lib/http.c (revision 67599)
@@ -778,7 +778,7 @@
     memcpy(buffer, http->postdata, (size_t)http->postsize);
     fullsize = (size_t)http->postsize;

- if(http->backup.postsize) {
+ if(http->backup.postsize || http->backup.fread) {
       /* move backup data into focus and continue on that */
       http->postdata = http->backup.postdata;
       http->postsize = http->backup.postsize;
@@ -787,6 +787,7 @@

       http->sending++; /* move one step up */

+ http->backup.fread=NULL;
       http->backup.postsize=0;
     }
     else

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

>Comment By: Daniel Stenberg (bagder)
Date: 2008-01-14 22:08

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

I can't make the problem (hang) happen with your test code even when I
upped the size to 200K headers (for some reason, I agree with your
conclusion). I do think your second attempt of a fix is correct, although I
would put it within the following condition as well as there's no point in
doing that assingment while in "auth negotiation". Alas, my adjusted
version of your patch is (made against current CVS):

--- lib/http.c 10 Jan 2008 10:30:20 -0000 1.356
+++ lib/http.c 14 Jan 2008 21:04:58 -0000
@@ -2705,8 +2705,10 @@
           /* set the pointer to mark that we will send the post body
using the
              read callback, but only if we're not in authenticate
              negotiation */
- if(!conn->bits.authneg)
+ if(!conn->bits.authneg) {
             http->postdata = (char *)&http->postdata;
+ http->postsize = postsize;
+ }
         }
       }
       /* issue the request */

What do you think?

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

Comment By: Joe Malicki (jmalicki)
Date: 2008-01-14 22:05

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

(Oh, I neglected to mention that if it doesn't fail, try bumping up the
size parameter passed into get_big_header()... the following updated
testcase takes it as a commandline argument for easier testing). Sorry for
not doing that earlier, I've been sitting on getting this to you for awhile
and I forgot some of the details temporarily.
File Added: curl-large-header-bug2.c

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-01-14 21:53

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

Sorry, I must be blind. I'll take it for a spin right away! Comments will
follow...

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

Comment By: Joe Malicki (jmalicki)
Date: 2008-01-14 21:49

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

If you notice, I attached a testcase that fails... does it not fail for
you?

In this testcase, it is clearly posting data. The reason why
http->backup.postsize is 0 is
that http->postsize is only ever initially set in curl 7.17.1 http.c:2643,
which is only
if CURLOPT_POSTFIELDS is set. Because I'm writing the posts with a
callback, this value is
never set (perhaps this is the real bug? The patch I posted was that way
because it was less intrusive).

I provided a testcase (curl-large-header-bug.c) so it could be reproduced
with code rather than description.

Are you proposing that the attached patch is rather the correct one? (it
is untested)

File Added: curl-large-headers2.diff

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

Comment By: Daniel Stenberg (bagder)
Date: 2008-01-14 21:34

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

Can you please be more specific on as exact details as possible on what
you do to get this problem so that I can work on writing up a test case for
this?

I don't understand the patch. If http->backup.postsize is zero, then what
exactly is libcurl going to send when that condition equals true?

Also, while the 'fread' is now 'fread_func' in recent libcurls, how come
the patch isn't needed the exact same way there? I can't recall any
particular fixes in this area in a very long time?

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

Comment By: Joe Malicki (jmalicki)
Date: 2008-01-14 17:45

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

Above patch for 7.15.5 attached as a file.
File Added: curl-large-headers.diff

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

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