formdata: ack read callback abort

When doing a multipart formpost with a read callback, and that callback
returns CURL_READFUNC_ABORT, that return code must be properly
propagated back and handled accordingly. Previously it would be handled
as a zero byte read which would cause a hang!

Added test case 587 to verify. It uses the lib554.c source code with a
small ifdef.

Reported by: Anton Bychkov
Bug: http://curl.haxx.se/mail/lib-2011-10/0097.html

4 files changed, 75 insertions, 9 deletions

diff --git a/lib/formdata.c b/lib/formdata.c
index edd35ede8..cbef51171 100644
--- a/lib/formdata.c
+++ b/lib/formdata.c
@@ -855,10 +855,11 @@ int curl_formget(struct curl_httppost *form, void *arg,
 
       do {
         nread = readfromfile(&temp, buffer, sizeof(buffer));
-        if((nread == (size_t) -1) || (nread != append(arg, buffer, nread))) {
-          if(temp.fp) {
+        if((nread == (size_t) -1) ||
+           (nread > sizeof(buffer)) ||
+           (nread != append(arg, buffer, nread))) {
+          if(temp.fp)
             fclose(temp.fp);
-          }
           Curl_formclean(&data);
           return -1;
         }
@@ -1269,6 +1270,13 @@ int Curl_FormInit(struct Form *form, struct FormData *formdata )
   return 0;
 }
 
+/*
+ * readfromfile()
+ *
+ * The read callback that this function may use can return a value larger than
+ * 'size' (which then this function returns) that indicates a problem and it
+ * must be properly dealt with
+ */
 static size_t readfromfile(struct Form *form, char *buffer,
                            size_t size)
 {
@@ -1280,11 +1288,6 @@ static size_t readfromfile(struct Form *form, char *buffer,
       return 0;
     else
       nread = form->fread_func(buffer, 1, size, form->data->line);
-
-    if(nread > size)
-      /* the read callback can return a value larger than the buffer but
-         treat any such as no data in this case */
-      nread = 0;
   }
   else {
     if(!form->fp) {
diff --git a/tests/data/test587 b/tests/data/test587
new file mode 100644
index 000000000..6e1239a6a
--- /dev/null
+++ b/tests/data/test587
@@ -0,0 +1,51 @@
+<testcase>
+#
+# Server-side
+<reply>
+<data>
+</data>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+# tool is what to use instead of 'curl'
+<tool>
+lib587
+</tool>
+
+ <name>
+HTTP multi-part formpost with aborted read callback
+ </name>
+ <command>
+http://%HOSTIP:%HTTPPORT/587
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<strippart>
+s/^------------------------------[a-z0-9]*/------------------------------/
+s/boundary=----------------------------[a-z0-9]*/boundary=----------------------------/
+</strippart>
+<protocol>
+POST /587 HTTP/1.1

+Host: %HOSTIP:%HTTPPORT

+Accept: */*

+Content-Length: 732

+Expect: 100-continue

+Content-Type: multipart/form-data; boundary=----------------------------

+

+------------------------------

+Content-Disposition: form-data; name="sendfile"; filename="postit2.c"

+

+</protocol>
+# CURLE_ABORTED_BY_CALLBACK (42)
+<errorcode>
+42
+</errorcode>
+</verify>
+</testcase>
diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc
index a6f83134c..30883d227 100644
--- a/tests/libtest/Makefile.inc
+++ b/tests/libtest/Makefile.inc
@@ -15,7 +15,7 @@ noinst_PROGRAMS = chkhostname \
   lib579 lib529 lib530 lib532 lib533 lib536 lib537 lib540 lib541 lib542	\
   lib543 lib544 lib545 lib547 lib548 lib549 lib552 lib553 lib554 lib555	\
   lib556 lib539 lib557 lib560 lib562 lib564 lib565 lib566 lib567 lib568	\
-  lib569 lib570 lib571 lib572 lib573 lib582 lib583 lib585
+  lib569 lib570 lib571 lib572 lib573 lib582 lib583 lib585 lib587
 
 chkhostname_SOURCES = chkhostname.c $(top_srcdir)/lib/curl_gethostname.c
 chkhostname_LDADD = @CURL_NETWORK_LIBS@
@@ -166,3 +166,6 @@ lib583_SOURCES = lib583.c $(SUPPORTFILES)
 
 lib585_SOURCES = lib500.c $(SUPPORTFILES)
 lib585_CPPFLAGS = $(AM_CPPFLAGS) -DLIB585
+
+lib587_SOURCES = lib554.c $(SUPPORTFILES)
+lib587_CPPFLAGS = $(AM_CPPFLAGS) -DLIB587
diff --git a/tests/libtest/lib554.c b/tests/libtest/lib554.c
index 8e71c8899..ba42bc482 100644
--- a/tests/libtest/lib554.c
+++ b/tests/libtest/lib554.c
@@ -42,6 +42,14 @@ static size_t read_callback(void *ptr, size_t size, size_t nmemb, void *userp)
 {
   struct WriteThis *pooh = (struct WriteThis *)userp;
 
+#ifdef LIB587
+  (void)ptr;
+  (void)size;
+  (void)nmemb;
+  (void)userp;
+  return CURL_READFUNC_ABORT;
+#else
+
   if(size*nmemb < 1)
     return 0;
 
@@ -53,6 +61,7 @@ static size_t read_callback(void *ptr, size_t size, size_t nmemb, void *userp)
   }
 
   return 0;                         /* no more data left to deliver */
+#endif
 }
 
 int test(char *URL)