-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
HTTP2 streaming post data problem #982
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
Comments
TL;DR: Your hunch is wrong because on send 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 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;
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. |
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). |
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. |
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? |
Sounds like a bug we should fix |
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. |
I use HTTPS. The same binary works if it send to a non-http2 site. |
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. |
Thanks for continue watch for this. Here are sites I use for test:
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. |
Ok, I can reproduce. I first didn't understand I had to modify the example to trigger it. Working on a fix. |
@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! |
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():
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
The text was updated successfully, but these errors were encountered: