- 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.

diff --git a/CHANGES b/CHANGES
index 5cba3dc8e9565926408610f92422744c91ae69a9..f9a3c9fbacb1d5a7041180eee766d862076c2cff 100644 (file)
--- 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 7c31bc50eff838d5d365637c4e63c90d56238440..10e5660635a6de3d5c1d55863f2b45a8a8dde7e9 100644 (file)
--- 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 3bd541c64bd23e3f3291a9a2803d8cd7db8114e2..c2b2da5d5fa6f723c0d307f7ee92927403027a8c 100644 (file)
--- 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 195d661d6a5cc65e8d0d8142219cdff10c84a751..173de8edcc251ae2d1944e442bff78caa2d47635 100644 (file)
--- 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 5a04f8c432de2334e5be626c8b989f7eab403d34..1c53120dda63bd16bffd70affac7dd64ee99ae78 100644 (file)
--- 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 ea6cfa357e16c18b85006d953588d303da6e1384..330ba7df6a00374fe62b5b8e7c6c63da933247f7 100644 (file)
--- 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 33f2c8d6eebbc6bd5852b8e285756c58029aab46..fb3ae0ec927f147df75ce201deea81825a1fde3b 100644 (file)
--- 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 4aa31c1d64fd5a51a92014e52532d7b9fca97f85..080e1d1d5ab43557130e3874d0d9d12eb3f395d5 100644 (file)
--- 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\r
+HTTP/1.1 301 Redirect swsclose\r
 Date: Thu, 29 Jul 2008 14:49:00 GMT\r
 Server: test-server/fake\r
 Location: data/10510002.txt?coolsite=yes\r
 Content-Length: 0\r
-Connection: Keep-Alive\r
+Connection: close\r
 \r
 HTTP/1.1 100 Continue\r
 \r
@@ -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\r
 Host: %HOSTIP:%HTTPPORT\r
@@ -83,6 +89,15 @@ Accept: */*
 Content-Length: 78\r
 Expect: 100-continue\r
 \r
+Weird
+     file
+         to
+   upload
+for
+   testing
+the
+   PUT
+      feature
 PUT /want/data/10510002.txt?coolsite=yes HTTP/1.1\r
 Host: %HOSTIP:%HTTPPORT\r
 Accept: */*\r

diff --git a/tests/data/test1052 b/tests/data/test1052
index fac7e7d2fceea721851194610e815ebe17af5245..10c16b4a79cabbecf88e0c1bb88bc3db795accbb 100644 (file)
--- 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\r
+HTTP/1.0 301 Redirect swsclose\r
 Date: Thu, 29 Jul 2008 14:49:00 GMT\r
 Server: test-server/fake\r
 Location: data/10520002.txt?coolsite=yes\r
 Content-Length: 0\r
-Connection: Keep-Alive\r
+Connection: close\r
 \r
 HTTP/1.0 200 Followed here fine swsclose\r
 Date: Thu, 29 Jul 2008 14:49:00 GMT\r
@@ -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\r
+PUT /want/1052 HTTP/1.0\r
 Host: %HOSTIP:%HTTPPORT\r
 Accept: */*\r
 Content-Length: 78\r
 \r
-PUT /want/data/10520002.txt?coolsite=yes HTTP/1.1\r
+Weird
+     file
+         to
+   upload
+for
+   testing
+the
+   PUT
+      feature
+PUT /want/data/10520002.txt?coolsite=yes HTTP/1.0\r
 Host: %HOSTIP:%HTTPPORT\r
 Accept: */*\r
 Content-Length: 78\r

diff --git a/tests/data/test1055 b/tests/data/test1055
index 7e2eae1b8f51d908fe523975635c9c0e39b03ecb..510d490f410b19f4da73c0365cfa84104bb7dc61 100644 (file)
--- a/tests/data/test1055
+++ b/tests/data/test1055
@@ -65,7 +65,18 @@ the
 PUT /1055 HTTP/1.1\r
 Host: %HOSTIP:%HTTPPORT\r
 Accept: */*\r
+Content-Length: 78\r
+Expect: 100-continue\r
 \r
+Weird
+     file
+         to
+   upload
+for
+   testing
+the
+   PUT
+      feature
 USER anonymous\r
 PASS ftp@example.com\r
 PWD\r