#1334 Timeout issue in code handling 100-continue

closed-fixed
None
5
2014-09-10
2014-02-06
No

Hi,

I think there may be an issue with the way libcurl 7.35.0 is handling the case of a server not sending a 100 (Continue) response after an Expect: 100-continue header has been sent.

When using the multi socket interface, libcurl calls the curl_multi_timer_callback asking to be woken up after CURL_TIMEOUT_EXPECT_100 milliseconds (the timeout is set in Curl_setup_transfer()).

        /* Set a timeout for the multi interface. Add the inaccuracy margin so                                                                                                                                                                                          
           that we don't fire slightly too early and get denied to run. */
        Curl_expire(data, CURL_TIMEOUT_EXPECT_100);

After the timeout has expired, calling curl_multi_socket_action with CURL_SOCKET_TIMEOUT as sockfd leads libcurl to check expired timeouts. When handling the 100-continue one, the following check in Curl_readwrite() fails if exactly CURL_TIMEOUT_EXPECT_100 milliseconds passed since the timeout has been set:

      long ms = Curl_tvdiff(k->now, k->start100);
      if(ms > CURL_TIMEOUT_EXPECT_100) {

As the event has now expired, it is discarded and the corresponding request will never succeed.

The following patch (proper git-formatted version attached) is working fine in our tests, and it seems logical to consider that having waited for CURL_TIMEOUT_EXPECT_100 ms is enough :

diff --git a/lib/transfer.c b/lib/transfer.c
index 3408a84..f996b0e 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -1075,7 +1075,7 @@ CURLcode Curl_readwrite(struct connectdata *conn,
       */

       long ms = Curl_tvdiff(k->now, k->start100);
-      if(ms > CURL_TIMEOUT_EXPECT_100) {
+      if(ms >= CURL_TIMEOUT_EXPECT_100) {
         /* we've waited long enough, continue anyway */
         k->exp100 = EXP100_SEND_DATA;
         k->keepon |= KEEP_SEND;

It seems this was not an issue before this commit (at least not exactly in the same way):
https://github.com/bagder/curl/commit/3b183df9cc781b329ca409ded1ea336530624715

It seems that the previous commit expected this case to be taken care of by this commit, but it does not seem to be the case:
https://github.com/bagder/curl/commit/980659a2caa2856078c8a860b9b95f659c5cc2c1

Kind regards,

Remi Gacogne
Nuage Labs SAS

1 Attachments

Discussion

  • Daniel Stenberg

    Daniel Stenberg - 2014-02-06
    • status: open --> closed-fixed
    • assigned_to: Daniel Stenberg
     
  • Daniel Stenberg

    Daniel Stenberg - 2014-02-06

    Thanks a lot, fix merged and pushed to git just now. Case closed!

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks