- Test cases 1051, 1052 and 1055 were added by Daniel Fandrich on July 30 and

  proved how PUT and POST with a redirect could lead to a "hang" due to the
  data stream not being rewound properly when it had to in order to get sent
  properly (again) to the subsequent URL. This is now fixed and these test
  cases are no longer disabled.

10 files changed, 82 insertions, 21 deletions

diff --git a/CHANGES b/CHANGES
index 5cba3dc8e..f9a3c9fba 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,13 @@
 
                                   Changelog
 
+Daniel Stenberg (4 Aug 2008)
+- Test cases 1051, 1052 and 1055 were added by Daniel Fandrich on July 30 and
+  proved how PUT and POST with a redirect could lead to a "hang" due to the
+  data stream not being rewound properly when it had to in order to get sent
+  properly (again) to the subsequent URL. This is now fixed and these test
+  cases are no longer disabled.
+
 Yang Tse (4 Aug 2008)
 - Autoconf 2.62 has changed the behaviour of the AC_AIX macro which we use.
   Prior versions of autoconf defined _ALL_SOURCE if _AIX was defined. 2.62
@@ -14,9 +21,8 @@ Yang Tse (4 Aug 2008)
   and an uniform one across autoconf versions AC_AIX is replaced with our
   own internal macro CURL_CHECK_AIX_ALL_SOURCE.
 
-
 Daniel Stenberg (4 Aug 2008)
-- Test case 1041 (added by Daniel Fandrich April 14th) proved a bug where PUT
+- Test case 1041 (added by Daniel Fandrich July 14th) proved a bug where PUT
   with -C - sent garbage in the Content-Range: header. I fixed this problem by
   making sure libcurl always sets the size of the _entire_ upload if an app
   attemps to do resumed uploads since libcurl simply cannot know the size of
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 7c31bc50e..10e566063 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -45,7 +45,8 @@ This release includes the following bugfixes:
  o a user name in a proxy URL without a password was parsed incorrectly
  o library will now be built with _REENTRANT symbol defined only if needed
  o no longer link with gdi32 on Windows cross-compiled targets
- o PUT with -C - sent bad Content-Range: header
+ o HTTP PUT with -C - sent bad Content-Range: header
+ o HTTP PUT or POST with redirect could lead to hang
 
 This release includes the following known bugs:
 
diff --git a/TODO-RELEASE b/TODO-RELEASE
index 3bd541c64..c2b2da5d5 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -29,7 +29,4 @@ To be addressed before 7.19.0 (planned release: August 2008)
          tests on the system might allow determination of the problem origin.
          Solaris AutoBuilds suceeded on August 2 and 3.
 
-151 - PUT with -L hangs after receiving a redirect (test case 1051, but the
-      test harness has a problem with this, too)
-
 152 - 
diff --git a/lib/http.c b/lib/http.c
index 195d661d6..173de8edc 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -309,7 +309,7 @@ static bool pickoneauth(struct auth *pick)
 }
 
 /*
- * perhapsrewind()
+ * Curl_http_perhapsrewind()
  *
  * If we are doing POST or PUT {
  *   If we have more data to send {
@@ -331,18 +331,29 @@ static bool pickoneauth(struct auth *pick)
  *   }
  * }
  */
-static CURLcode perhapsrewind(struct connectdata *conn)
+CURLcode Curl_http_perhapsrewind(struct connectdata *conn)
 {
   struct SessionHandle *data = conn->data;
   struct HTTP *http = data->state.proto.http;
   curl_off_t bytessent;
   curl_off_t expectsend = -1; /* default is unknown */
 
-  if(!http)
+  if(!http || !(conn->protocol & PROT_HTTP))
     /* If this is still NULL, we have not reach very far and we can
-       safely skip this rewinding stuff */
+       safely skip this rewinding stuff, or this is attempted to get used
+       when HTTP isn't activated */
     return CURLE_OK;
 
+  infof(data, "now in %s\n", __func__);
+
+  switch(data->set.httpreq) {
+  case HTTPREQ_GET:
+  case HTTPREQ_HEAD:
+    return CURLE_OK;
+  default:
+    break;
+  }
+
   bytessent = http->writebytecount;
 
   if(conn->bits.authneg)
@@ -453,7 +464,7 @@ CURLcode Curl_http_auth_act(struct connectdata *conn)
     if((data->set.httpreq != HTTPREQ_GET) &&
        (data->set.httpreq != HTTPREQ_HEAD) &&
        !conn->bits.rewindaftersend) {
-      code = perhapsrewind(conn);
+      code = Curl_http_perhapsrewind(conn);
       if(code)
         return code;
     }
diff --git a/lib/http.h b/lib/http.h
index 5a04f8c43..1c53120dd 100644
--- a/lib/http.h
+++ b/lib/http.h
@@ -8,7 +8,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2007, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2008, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -57,6 +57,7 @@ void Curl_http_auth_stage(struct SessionHandle *data, int stage);
 CURLcode Curl_http_input_auth(struct connectdata *conn,
                               int httpcode, const char *header);
 CURLcode Curl_http_auth_act(struct connectdata *conn);
+CURLcode Curl_http_perhapsrewind(struct connectdata *conn);
 
 int Curl_http_should_fail(struct connectdata *conn);
 
diff --git a/lib/transfer.c b/lib/transfer.c
index ea6cfa357..330ba7df6 100644
--- a/lib/transfer.c
+++ b/lib/transfer.c
@@ -1116,6 +1116,12 @@ CURLcode Curl_readwrite(struct connectdata *conn,
                 data->req.newurl = strdup(data->req.location); /* clone */
                 if(!data->req.newurl)
                   return CURLE_OUT_OF_MEMORY;
+
+                /* some cases of POST and PUT etc needs to rewind the data
+                   stream at this point */
+                result = Curl_http_perhapsrewind(conn);
+                if(result)
+                  return result;
               }
             }
           }
@@ -1570,6 +1576,7 @@ CURLcode Curl_readwrite(struct connectdata *conn,
         /* we've waited long enough, continue anyway */
         k->exp100 = EXP100_SEND_DATA;
         k->keepon |= KEEP_WRITE;
+        infof(data, "Done waiting for 100-continue\n");
       }
     }
   }
diff --git a/tests/data/DISABLED b/tests/data/DISABLED
index 33f2c8d6e..fb3ae0ec9 100644
--- a/tests/data/DISABLED
+++ b/tests/data/DISABLED
@@ -3,6 +3,4 @@
 # test cases are run by runtests.pl. Just add the plain test case numbers, one
 # per line.
 # Lines starting with '#' letters are treated as comments.
-1051
-1052
-1055
+
diff --git a/tests/data/test1051 b/tests/data/test1051
index 4aa31c1d6..080e1d1d5 100644
--- a/tests/data/test1051
+++ b/tests/data/test1051
@@ -29,12 +29,12 @@ Content-Length: 51
 If this is received, the location following worked
 </data2>
 <datacheck>
-HTTP/1.1 301 Redirect

+HTTP/1.1 301 Redirect swsclose

 Date: Thu, 29 Jul 2008 14:49:00 GMT

 Server: test-server/fake

 Location: data/10510002.txt?coolsite=yes

 Content-Length: 0

-Connection: Keep-Alive

+Connection: close

 

 HTTP/1.1 100 Continue

 

@@ -76,6 +76,12 @@ the
 <strip>
 ^User-Agent:.*
 </strip>
+
+# The primary reason libcurl sends the data part twice in this test is that
+# the test HTTP server is blocking until it has read the entire request,
+# including the full request-body before it responds. So in this test the
+# server says 301 and 100 _after_ the entire PUT body has been sent.
+
 <protocol>
 PUT /want/1051 HTTP/1.1

 Host: %HOSTIP:%HTTPPORT

@@ -83,6 +89,15 @@ Accept: */*
 Content-Length: 78

 Expect: 100-continue

 

+Weird
+     file
+         to
+   upload
+for
+   testing
+the
+   PUT
+      feature
 PUT /want/data/10510002.txt?coolsite=yes HTTP/1.1

 Host: %HOSTIP:%HTTPPORT

 Accept: */*

diff --git a/tests/data/test1052 b/tests/data/test1052
index fac7e7d2f..10c16b4a7 100644
--- a/tests/data/test1052
+++ b/tests/data/test1052
@@ -27,12 +27,12 @@ Content-Length: 51
 If this is received, the location following worked
 </data2>
 <datacheck>
-HTTP/1.0 301 Redirect

+HTTP/1.0 301 Redirect swsclose

 Date: Thu, 29 Jul 2008 14:49:00 GMT

 Server: test-server/fake

 Location: data/10520002.txt?coolsite=yes

 Content-Length: 0

-Connection: Keep-Alive

+Connection: close

 

 HTTP/1.0 200 Followed here fine swsclose

 Date: Thu, 29 Jul 2008 14:49:00 GMT

@@ -72,13 +72,27 @@ the
 <strip>
 ^User-Agent:.*
 </strip>
+
+# The primary reason libcurl sends the data part twice in this test is that
+# the test HTTP server is blocking until it has read the entire request,
+# including the full request-body before it responds. So in this test the
+# server says 301 and 200 _after_ the entire PUT body has been sent.
 <protocol>
-PUT /want/1052 HTTP/1.1

+PUT /want/1052 HTTP/1.0

 Host: %HOSTIP:%HTTPPORT

 Accept: */*

 Content-Length: 78

 

-PUT /want/data/10520002.txt?coolsite=yes HTTP/1.1

+Weird
+     file
+         to
+   upload
+for
+   testing
+the
+   PUT
+      feature
+PUT /want/data/10520002.txt?coolsite=yes HTTP/1.0

 Host: %HOSTIP:%HTTPPORT

 Accept: */*

 Content-Length: 78

diff --git a/tests/data/test1055 b/tests/data/test1055
index 7e2eae1b8..510d490f4 100644
--- a/tests/data/test1055
+++ b/tests/data/test1055
@@ -65,7 +65,18 @@ the
 PUT /1055 HTTP/1.1

 Host: %HOSTIP:%HTTPPORT

 Accept: */*

+Content-Length: 78

+Expect: 100-continue

 

+Weird
+     file
+         to
+   upload
+for
+   testing
+the
+   PUT
+      feature
 USER anonymous

 PASS ftp@example.com

 PWD