-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Observed test case failures 1700 / 1701 / 1702 (h2c upgrade) #7036
Comments
Those tests are seriously flaky. On my dev machine at home they fail reliably if I run them with valgrind enabled, but work if I run without valgrind! fails
works
|
TD;DR: What's known so farThe observed issue is believed to have been caused by Although curl is aware of such residual data and attempts to feed it to The role of Sharing some raw debugging data from my local runsIn the earlier mentioned temporary repository https://github.com/starrify/curl-test-failure-1700 I have added further raw data recorded during local investigations:
(the captured traffic and debugging output come from different runs) Observation: captured traffic for the succeeded and failed testsInspection of When running without
|
if(nread) | |
memcpy(httpc->inbuf, mem, nread); | |
httpc->inbuflen = nread; | |
DEBUGASSERT(httpc->nread_inbuf == 0); | |
if(-1 == h2_process_pending_input(data, httpc, &result)) | |
return CURLE_HTTP2; |
By further debugging practices, I also observed that the corresponding callbacks (e.g. on_frame_recv
and on_header
) were properly invoked when the residual payload was parsed by nghttp2
.
By linking my local debugging build of curl
to a local debugging build of nghttp2
I was able to further examine the nghttp2
debugging output, which did not look weird to me.
Further inspections suggested that the parsed HTTP/2 response was dropped (or more like ignored) at the entrance of http2_recv
, as the HTTP stream is already deemed as closed when the below code is reached:
Lines 1622 to 1624 in 1763ace
if(stream->closed) | |
/* closed overrides paused */ | |
return http2_handle_stream_close(conn, data, stream, err); |
This may be deemed necessary since some servers (e.g. nghttp2's reverse proxy) may start sending the response payload immediately following the server-side connection preface, other than waiting for the client's connection preface and possible setting negotiations. Resolves curl#7036. Please refer to this issue for further details.
It is confirmed that 252790c is the revision in which this issue may be first triggered.
Click here for debugging output (passing
|
This is considered not harmful as a following http2_recv shall be called very soon. This is considered helpful in the specific situation where some servers (e.g. nghttpx v1.43.0) may fulfill stream 1 immediately following the return of HTTP status 101, other than waiting for the client-side connection preface to arrive. Resolves curl#7036.
Overview
I have earlier observed failures of test cases 1700, 1701, and 1702 on my local machine at 1763ace, which was earlier mentioned in #7032.
These three cases are all for HTTP/2 on plaintext TCP with the
Upgrade: h2c
haeder. The only difference is the request method, which isGET
,POST
, andHEAD
, respectively. Those three failures look almost identical, thus I may talk about only case 1700 for convenience.Environment and issue reproduction
The failures may be reproduced stably at 51c0ebc and other recent revisions on my local machine, and in a Docker environment constructed purposely for reproducing this issue. Details on the Docker environment would be given below.
Click here for content of the
Dockerfile
, which may be used to build a Docker image using commands likedocker build -t $IMAGE_NAME - < Dockerfile
.Click here for some debugging information from the
configure
output during the build.Click here for the full test results for case 1700, which is generated from
srcdir=. /usr/bin/perl -I. ./runtests.pl -a -p -r 1700
as part ofTFLAGS=1700 make test-full
.The raw testing logs for case 1700 (as from the
./tests/log/
directory) have been pulled from the built image (usingdocker cp $(docker create $IMAGE_NAME):/curl/tests/log .
) and uploaded to a temporary git repository for further inspection in case that's needed: https://github.com/starrify/curl-test-failure-1700The temporary git repository also contains all three pieces of raw texts shared above.
Observations from online CI runs
However I'm yet unable to observe the exact failure on these three cases (1700 / 1701 / 1702) from recent online CI runs, although they often fail due to other reasons.
In all recent CI runs that I randomly picked and manually checked, the outcome of test case 1700 is observed to belong to two categories:
Further steps
Further triage may be needed for locating the exact cause of issue (curl code / test case / upstream tools like nghttp2 / etc.)
I would like to spend some more time on further understanding the reasons behind the failure. However given my lack of familiarity with this project, that might take some time.
Therefore I'm filing this issue in advance, so that the community may be aware of the failures (or maybe someone may come and tell me that the issue was merely caused by some silly mistake of mine 😅).
The text was updated successfully, but these errors were encountered: