Unable to perform "Chunked" uploading of multiform data over HTTP/2

[akshayvernekar]

I did this

Currently I am able to chunked transfer the data when the CURLFORM_CONTENTSLENGTH is known, but my application requires me to transfer the data as a stream which means that the CURLFORM_CONTENTSLENGTH is not known. I tried setting the CURLFORM_CONTENTSLENGTH to a false value , the uploading of data is successful and I am even getting the response from the server, but the number of running handles still_running from curl_multi_perform(multi_handle,&still_running) never becomes 0. It always returns "1" which makes my application to hang. Below is the pseudocode, this isnt the exact code i am using but explains the problem i am facing

    #include <stdio.h>
    #include <string.h>
    #include <curl/curl.h>

     #define MULTI_PART_CONTENT_TYPE_AUDIO "application/octet-stream" 
     #define DATA_SIZE 10  
     char demoBuffer[DATA_SIZE] = {1,2,3,4,5,6,7,8,9,0};
     std::string metadata("I am metadata");
     int Curr_index ;

     int getTransferStatus(CURLM *multiHandleInstance,CURL *currentHandle)
     {
         ALOGE("a<--getTransferStatus");
        //check the status of transfer    CURLcode return_code=0;
         int msgs_left=0;
         CURLMsg *msg=NULL;
         CURL *eh=NULL;
         CURLcode return_code;  
         int http_status_code = 0; 
         while ((msg = curl_multi_info_read(multiHandleInstance, &msgs_left))) 
         {  
             if (msg->msg == CURLMSG_DONE) {
             eh = msg->easy_handle;
             if(currentHandle == eh)
             {
               ALOGE("a<--found the handle");
               return_code = msg->data.result;
               if((return_code!=CURLE_OK) &&(return_code != CURLE_RECV_ERROR)) 
               {
                 fprintf(stderr, "CURL error code: %d\n", msg->data.result);
                 ALOGE("a<--return_code!=CURLE_OK CURL error code: %d\n", msg->data.result);
                 continue;
               }  
               // Get HTTP status code
               http_status_code=0;
               curl_easy_getinfo(eh, CURLINFO_RESPONSE_CODE, &http_status_code);
               ALOGE("a<--CURLINFO_RESPONSE_CODE=%d",http_status_code);
             }
           }
         }
         return http_status_code;
     }

     size_t processResponse(void *ptr, size_t size, size_t nmemb, void *instance)
     {
       //write callback
     }

     size_t datareader(void *ptr, size_t size, size_t nmemb, void* userp) 
     {
       //read callback
       char *pooh = ( char *)userp;

       if(size*nmemb < 1)
         return 0;

       if(index < DATA_SIZE) {
         *(char *)ptr = pooh[index]; /* copy one single byte */ 
         index++;
         return 1;                        /* we return 1 byte at a time! */ 
       }

       return 0;        
     }

     int main(void)
     {
       std::string postUrl("https://example.com");
       CURLM *multi_handle = curl_multi_init();
       int still_running = 0;
       Curr_index = 0;

       /*I want to upload the data using chunked encoding, 
       which means I dont know the total content size */
       struct curl_slist *header = NULL; 
       header = curl_slist_append(header, "Host: example.com"); 
       header = curl_slist_append(header, authorization.c_str());
       header = curl_slist_append(header, "Content-Type: multipart/form-data"); 
       header = curl_slist_append(header, "Transfer-Encoding: chunked");


       struct curl_httppost *formpost = NULL; 
       struct curl_httppost *lastptr = NULL;

       curl_formadd(&formpost,
       &lastptr, 
       CURLFORM_COPYNAME, "metadata", 
       CURLFORM_COPYCONTENTS, metadata.c_str(), 
       CURLFORM_CONTENTTYPE, MULTI_PART_CONTENT_TYPE_JSON, 
       CURLFORM_END); 

       /*In below code I am commenting out the CONTENTSLENGTH 
       as i dont know the total size of the buffer, 
       the behaviour is same when when CONTENTSLENGTH 
       is commented or set to a false value.The transfer never gets completed. */

       /*Curl documentation for CURLFORM_STREAM(          [https://curl.haxx.se/libcurl/c/curl_formadd.html] ) 
       tells developers its mandatory to set the CURLFORM_CONTENTSLENGTH,
       In cases in "Chunked" transfer encoding the CONTENTSLENGTH is not known*/

       curl_formadd(&formpost,
       &lastptr,
       CURLFORM_COPYNAME, "audio",
       CURLFORM_CONTENTTYPE, MULTI_PART_CONTENT_TYPE_AUDIO,
       CURLFORM_STREAM, demoBuffer, 
       //CURLFORM_CONTENTSLENGTH, DATA_SIZE, 
       CURLFORM_END); 


       CURL *eventHttp_handle;
       CURLcode res;

       // init the curl session  
       eventHttp_handle = curl_easy_init();

       if (eventHttp_handle) 
       {
    curl_easy_setopt(eventHttp_handle, CURLOPT_URL, postUrl.c_str()); 
    curl_easy_setopt(eventHttp_handle, CURLOPT_SSL_VERIFYPEER, 0L); 
    curl_easy_setopt(eventHttp_handle, CURLOPT_SSL_VERIFYHOST, 0L);
    curl_easy_setopt(eventHttp_handle, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE); 

     #ifdef DEBUG_MODE
    /* disable progress meter, set to 0L to enable and disable debug output */ 
    curl_easy_setopt(eventHttp_handle, CURLOPT_VERBOSE, 1L);
    curl_easy_setopt(eventHttp_handle, CURLOPT_NOPROGRESS, 0L); 
    curl_easy_setopt(eventHttp_handle, CURLOPT_DEBUGFUNCTION, my_trace);
     #endif
  //write callback function
  curl_easy_setopt(eventHttp_handle, CURLOPT_WRITEFUNCTION, processResponse); 
  curl_easy_setopt(eventHttp_handle, CURLOPT_WRITEDATA, NULL);

  //read callback function for sending audio
  curl_easy_setopt(eventHttp_handle, CURLOPT_READFUNCTION, datareader);
  curl_easy_setopt(eventHttp_handle, CURLOPT_READDATA,demoBuffer);

  curl_easy_setopt(eventHttp_handle, CURLOPT_HTTPPOST, formpost);
  curl_easy_setopt(eventHttp_handle, CURLOPT_HTTPHEADER, header);

  //adding easy handle to Multihandle
  curl_multi_add_handle(multi_handle, eventHttp_handle);


  do {
      CURLMcode mc;
      int numfds;

      /* when CURLFORM_CONTENTSLENGTH is set to false value 
      the "still_running" (active handles) never returns to 0*/

      mc = curl_multi_perform(multi_handle, &still_running);

      if(mc == CURLM_OK ) {
        /* wait for activity, timeout or "nothing" */
        mc = curl_multi_wait(multi_handle, NULL, 0, 1000, &numfds);
       }
      if(mc != CURLM_OK) {
        fprintf(stderr, "curl_multi failed, code %d.n", mc);
        ALOGE("a<---curl_multi failed, code %d.n", mc);
        break;
      }
      /* 'numfds' being zero means either a timeout or no file descriptors to
      wait for. Try timeout on first occurrence, then assume no file
      descriptors and no file descriptors to wait for means wait for 100
      milliseconds. */

      ALOGE("a<--numfds=%d,still_running=%d",numfds,still_running);

      if(!numfds) 
      {
          struct timeval wait = { 0, 100 * 1000 }; // 100ms  
          select(0, NULL, NULL, NULL, &wait);
      }

   } while(still_running);

   if (header)
      curl_slist_free_all(header);

    if (formpost)
    { 
      curl_formfree(formpost); 
      formpost = NULL; 
    } 

    int responseCode =  getTransferStatus(multi_handle,eventHttp_handle);
    if(responseCode == 200)//200 OK!
    {
      ALOGE("a<--Request success !!");
      // handle response body
             }
         }
    return 0;
 }

I expected the following

Current version of Curl doesnt allow the user to do chunked transfer of Mutiform data using the "CURLFORM_STREAM" without knowing the "CURLFORM_CONTENTSLENGTH" . It would be great if we can ignore the "CURLFORM_CONTENTSLENGTH" for chunked transfer .

curl/libcurl version

curl v50

operating system

Android ndk

[bagder]

1. This code doesn't build so clearly this wasn't used to reproduce the problem.
2. There is no curl version 50. I suppose you mean 7.50.0 ?
3. Your use of CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE seems suspicious and I'd advice you to reconsider using that.
4. I cleaned up the code, made it pure C and added a debug callback to show exactly what's sent and received, see my take here: stream-upload-formpost.c

This version of the code works for me against my local Apache. If this fails for you, can you please show us what headers are sent/received?

[akshayvernekar]

Daniel,

1. Yes this was pseudo code, the original code has dependency across multiple files. Sorry to take you through the trouble.
   2.the version is 7.50.1
2. & 4.
   The server only accepts HTTP2 requests so I am using CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE flag. With the code you shared I additionally set the CURLOPT_HTTP_VERSION to use http2 .P.S.: I am using nghttp2 lib for http2 related requests. The http dump is in following link Http request dump. I think the issue occurs when we use HTTP version 2 , I went through the Curl source code and found on line 1937 in http.c , here we are not setting the data->req.upload_chunky flag. I tried forcefully setting the flag to true even for http2 but the Http body transfer wouldn’t happen.

[bagder]

So HTTP/2 is required for this bug to trigger? CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE is primarily made for doing HTTP/2 to HTTP:// URLs, as with plain old HTTPS you could just use CURL_HTTP_VERSION_2_0. Hence my confusion. Your code will break if you point it to a random HTTP:// URL.

[akshayvernekar]

Yes this happens for HTTP2. I understand the risk here, CURL_HTTP_VERSION_2_PRIOR_KNOWLEDGE is to be used only if we are sure that the server accepts HTTP2 requests and it would fail for other servers that doesnt support HTTP2. Right now all my requests are targeted for a particular server(which supports http2) so this is safe for my implementation. Regarding the issue, I went through some documentation for HTTP2 and because of the framed structure of HTTP/2 it is no longer necessary (in fact explicitly forbidden) to use the chunked transfer encoding that HTTP/1.1 uses for entities of unknown lengths . So I think CURLFORM_STREAM ideally we should ignore Content length for HTTP2 requests. Isnt it?

[bagder]

The problem with chunked in curl's HTTP/2 handling is known: see https://curl.haxx.se/docs/knownbugs.html#transfer_encoding_chunked_in_HT and #662

[bagder]

I updated the formpost example in the gist above to enable HTTP/2 and I removed the custom Host: header which only will lead to pain and sad faces. Using this, I could reproduce the problem.

My proposed patch for fixing the bug is here: 0001-http-accept-Transfer-Encoding-chunked-for-HTTP-2-as-.patch

[akshayvernekar]

Hi Daniel, I tried the patch this does fix the issue mentioned in #662now there is no transfer encoding or content length field in header, but now the transfer is always failing , specifically the read callback is not being called by libcurl , please check the latest dump . My original issue was that the transfer would happen successfully but the "still_running" flag would always be set even after the response is downloaded. I am looking for a solution where after the response is downloaded the flag would be reset to 0.

[bagder]

My example works against my HTTP/2 server, what more can I do?

Are you saying the same example fails against your server? If so, please show me that output.

[akshayvernekar]

The new output is here . After applying the patch the read callback function is not invoked . I can see from the dump that only the HTTP headers are sent and not the body.

[bagder]

That output is not made with the example code we use to reproduce the case with. Please reproduce the case with the example you and me have worked on together (and update as you see fit so that it matches your problematic case) so that we both run the same thing!

[akshayvernekar]

HI Daniel, so that we both are on the same page I have created a sample program which is based on our previous example which can reproduce the issue, here you will be making a request to the same server which I am also using so that there is no discrepancy . The example program is here:demo_code.c . Without the patch the "read callback function" (datareader) is invoked as intended but with patch the read callback function is not invoked. Thanks for your support. You might require a sample audio , you can find it here. PS: I am writing the dump to external file, please change according to your convenience .

[bagder]

I had to adjust the example slightly for it to build and run properly (your debug function didn't redirect everything to the output file for example). I got this to work just fine actually. It produced this
dump.txt. As you can see in there, it sent the entire stream (I used a source code file as a sample input) and then it gets a 403 response back.

I then looked at your dump again, and we can see that you're getting a 400 response very quickly from the server and that can very well be why curl stopped sending: your server refused it already so there was no point in continue sending it!

So, either curl sends the entire thing before it gets an error back - or it gets a positive response back (<400), otherwise we're not looking for the (same) bug anymore.

[jay]

I tried both your example and Daniel's example after applying the patch. In both cases when I connect to localhost Apache/2.4.18 it uses http2 and POSTs fine. However, when I connect to 54.239.23.243 using @akshayvernekar's example the server will disconnect (FIN) immediately after the headers are sent:

== Info: multi changed, check CONNECT_PEND queue!
== Info: http2_recv: easy 0x80db68 (stream 1)
== Info: SSL read: error:00000000:lib(0):func(0):reason(0), errno 10053
== Info: Failed receiving HTTP2 data
== Info: Kill stream: Transfer returned error
== Info: multi_done

libcurl built from curl-7_50_3-2-gd932156
curl 7.51.0-DEV (i386-pc-win32) libcurl/7.51.0-DEV OpenSSL/1.0.2h nghttp2/1.14.1
I tried nghttp2/1.11.1 as well, same thing.

In @akshayvernekar's dump2.txt after the headers are sent there's some => Send data, 0000000000 bytes (0x00000000) , I don't have anything like that. Please give us the libcurl version information printf("%s\n", curl_version());.

[bagder]

I used this build for my tests (on a Debian Linux):

curl 7.51.0-DEV (x86_64-pc-linux-gnu) libcurl/7.51.0-DEV OpenSSL/1.0.2h zlib/1.2.8 c-ares/1.11.0 libidn/1.33 libpsl/0.14.0 (+libicu/57.1) nghttp2/1.15.0-DEV librtmp/2.3

[bagder]

you're getting a 400 response very quickly

Ah sorry, that was silly of me. The 400 in question is returned because it says it didn't get any multipart formpost so it is an effect of the problem and not a cause of it as I thought there for a moment.

[jay]

Results seem to vary for me in Ubuntu 16 LTS, I get == Info: SSL_write() returned SYSCALL, errno = 32 with no server reply but sometimes instead I get a 403 reply and == Info: HTTP error before end of send, stop sending. The interesting thing I think is if the server is sending the reply in all cases and for some reason it's dropped somewhere

curl 7.50.3 (x86_64-pc-linux-gnu) libcurl/7.50.3 OpenSSL/1.0.2h zlib/1.2.8 libidn/1.32 nghttp2/1.14.1 librtmp/2.3

[akshayvernekar]

My curl version is

libcurl/7.50.1 OpenSSL/1.0.2h zlib/1.2.7 nghttp2/1.14.0-DEV

My latest debug dump is here. The 403 error code is because the access tokens expire after 1 hour. Here is the new access token, this is valid for 1hour more. @jay I guess even though you got a 403 response , the uploading of http body wouldnt have happened. @bagder your output is what is desired , it seems like the uploading is happening fine in your code base. Just to make sure , http.c is the only file changed in the patch correct ?

[bagder]

libcurl/7.50.1

We fixed at least two h2 bugs in 7.50.2 that might affect this program and since you're rebuilding from source anyway to try out the patch I suggest you bump up to 7.50.3 or use the code straight from git. That also makes us try the same code.

Your latest dump has these two really weird lines at line 145-146 that I can't explain and that seem to indicate a problem:

=> Send data, 0000000000 bytes (0x00000000)
=> Send data, 0000000000 bytes (0x00000000)

Any change you can reubuild curl with ./configure --enable-debug to get even more info into the dump trace output?

Just to make sure , http.c is the only file changed in the patch correct

Correct!

[bagder]

Uh, it struck me that the fix is still incomplete since it actually sends data in "chunked" format even over h2 and that's incorrect. I'll work on a follow-up patch to address that. I doubt that's the reason for the remaining problem @akshayvernekar reports though.

[bagder]

Here's my slightly updated patch: 0001-http-accept-Transfer-Encoding-chunked-for-HTTP-2-as-.patch

It avoids the chunked formatting of the request body when sent over h2.

[akshayvernekar]

The issue I reported seems to be fixed in the latest version of CURL. I updated my CURL version from GIT ,as per your suggestion and I didn’t see this issue. Thanks for your support . The request is working without applying the patch too.

[bagder]

Oh lovely! Thanks for reporting back. I'll push my patch to master within shortly.

[Kaned1as]

Hi!

Can you please update docs in curl.haxx.se for CURLFORM_STREAM option? I mean, that setting CONTENTS_LENGTH to arbitrary value is allowed when using chunked encoding.

[bagder]

Added a comment just now about it not being needed when data is sent chunked.

[Kaned1as]

Thanks a lot!