Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP2 streaming post data problem #982

Closed
shihancheng opened this issue Aug 25, 2016 · 11 comments
Closed

HTTP2 streaming post data problem #982

shihancheng opened this issue Aug 25, 2016 · 11 comments

Comments

@shihancheng
Copy link

I did this

I migrate my http program to http2. The original post function did not work anymore. I found the post content has not been send out at all.

I am using CURLOPT_READFUNCTION,
curl_easy_setopt(hnd, CURLOPT_READFUNCTION, readFunc);
curl_easy_setopt(hnd, CURLOPT_READDATA, readBlock);

But readFunc been only call once even readFunc return positive non-zero value. (the same function work correctly in HTTP1.1 version)
So I dig into the source and found in http2.c, function http2_send():

    nghttp2_session_resume_data(h2, stream->stream_id);
    rv = h2_session_send(conn->data, h2);
    if(nghttp2_is_fatal(rv)) {
      *err = CURLE_SEND_ERROR;
      return -1;
    }
    len -= stream->upload_len;

I think the line:
len -= stream->upload_len;
should be
len = stream->upload_len;

But not sure what the original expected to handle the case if stream->upload_len smaller then len.

I expected the following

send my full post message with content

curl/libcurl version

libcurl 7.50.1

[curl -V output perhaps?]
curl 7.50.1 (x86_64-pc-linux-gnu) libcurl/7.50.1 mbedTLS/2.2.0 zlib/1.2.8 nghttp2/1.13.0

operating system

Linux Ubuntu 14.04

@jay
Copy link
Member

jay commented Aug 25, 2016

TL;DR: Your hunch is wrong because on send stream->upload_len is adjusted so that the bytes sent are subtracted. Can you give us verbose output, or a self-contained example that we can use to reproduce?


I will try to explain what is happening with upload_len since that may be confusing due to the callback. You are referring to this code in http2_send:

    stream->upload_mem = mem;
    stream->upload_len = len;
    nghttp2_session_resume_data(h2, stream->stream_id);
    rv = h2_session_send(conn->data, h2);
    if(nghttp2_is_fatal(rv)) {
      *err = CURLE_SEND_ERROR;
      return -1;
    }
    len -= stream->upload_len;

h2_session_send is basically a wrapper for nghttp2_session_send which uses a read callback function that libcurl set earlier called data_source_read_callback. In that function stream->upload_len is adjusted, which is this code:

  nread = MIN(stream->upload_len, length);
  if(nread > 0) {
    memcpy(buf, stream->upload_mem, nread);
    stream->upload_mem += nread;
    stream->upload_len -= nread;
    stream->upload_left -= nread;
  }

So for example len starts at 500 and therefore so does upload_len. nghttp2 asks and gets 300 bytes so upload_len is now 200 because that's what's left to upload. Then back in http2_send len is set to bytes to be sent minus bytes that weren't sent, or 500 - 200 = 300. Or in other words len becomes number of bytes that were sent, and that is what is returned.

@bagder bagder added the HTTP/2 label Aug 25, 2016
@shihancheng
Copy link
Author

Thanks for the detail explanation. My understand is wrong.

Now I have to figure it out why data can not be sent. (since my stream->upload_len is same as len after nghttp2_session_send).

@jay
Copy link
Member

jay commented Aug 25, 2016

If you think libcurl is at fault reopen this and give us more information, ideally a self contained example that we can use to reproduce.

@shihancheng
Copy link
Author

Here is my new finding of this problem. The attached program is modified from libcurl example http2-upload.c. I just change the CURLOPT_UPLOAD to CURLOPT_POST. And CURLOPT_INFILESIZE_LARGE to CURLOPT_POSTFIELDSIZE and it works good.

The only thing makes different is the CURLOPT_POSTFIELDSIZE. Without the content length, http2 will not send the second block. In contrast, with old http program it can depend on the CURLOPT_READFUNCTION return value instead of content lenght.

What are my alternatives if I can not get the total length of data before POST?

utest2.zip

@bagder
Copy link
Member

bagder commented Aug 25, 2016

Sounds like a bug we should fix

@bagder bagder reopened this Aug 25, 2016
@bagder
Copy link
Member

bagder commented Aug 28, 2016

What server are you running this against? Are you using HTTPS or HTTP? I've ran a few tests now against nghttpd and it works just fine for me.

@shihancheng
Copy link
Author

I use HTTPS. The same binary works if it send to a non-http2 site.

@bagder
Copy link
Member

bagder commented Aug 30, 2016

So again: what server are you running this against? It is hard for me to work on this if I cannot reproduce the case so it is in your interest to help us repeat the problem.

@shihancheng
Copy link
Author

Thanks for continue watch for this. Here are sites I use for test:
https://avs-alexa-na.amazon.com/v20160207/event --> it is a http2 site. it will said your are not authenticate
https://echo.getpostman.com/post --> this is a http1.1 site, it always works
https://http2.golang.org/ECH --> this is another http2 site, but it won't take POST, it only take PUT
I am not expect any of above site reply successfully or not, what I care is the content not been sent:

0000: POST /ECHO HTTP/1.1
0015: Host: http2.golang.org
002d: Accept: */*
003a: Content-type: multipart/form-data; boundary=MULTIPLEPART_BOUNDAR
007a: Y
007d: 
=> Send data, 0 bytes (0x0)
=> Send data, 0 bytes (0x0)
=> Send data, 0 bytes (0x0)
=> Send data, 0 bytes (0x0)
<= Recv header, 13 bytes (0xd)
0000: HTTP/2 400 

Above is the curl with CURLOPT_VERBOSE and CURLOPT_DEBUGFUNCTION output. You can see the header has been sent, but no data can be sent at all.

@bagder
Copy link
Member

bagder commented Sep 4, 2016

Ok, I can reproduce. I first didn't understand I had to modify the example to trigger it. Working on a fix.

@bagder bagder closed this as completed in 3d4c0c8 Sep 5, 2016
@bagder
Copy link
Member

bagder commented Sep 5, 2016

@shihancheng thanks for your report. Please try out the commits I just merged just to make sure it works for you as it does for me!

@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

3 participants