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.

Sign up for GitHub

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

Jump to bottom

Quiche CI job failing due to an http/3 upload test #12590

Closed
jay opened this issue Dec 23, 2023 · 1 comment
Closed

Quiche CI job failing due to an http/3 upload test #12590

jay opened this issue Dec 23, 2023 · 1 comment
Labels
CI Continuous Integration HTTP/3 h3 or quic related

Comments

@jay
Copy link
Member

jay commented Dec 23, 2023

An h2/h3 upload test has been failing in the quiche ci job since it was added two days ago in 3538027:

tests/http/test_07_upload.py ->
test_07_22_upload_parallel_fail aka "upload large data parallel to a URL that denies uploads"

The test expects curl to return error CURLE_HTTP3 (95) for the failed HTTP/3 transfer but instead with quiche HTTP/3 it returns CURLE_SEND_ERROR (55).

Example:

...
  09:20:09.404665 [0-0] > POST /curltest/tweak?status=400&delay=5ms&chunks=1&body_error=reset&id=0 HTTP/3
  09:20:09.404665 [0-0] > Host: one.http.curl.se:60423
  09:20:09.404665 [0-0] > User-Agent: curl/8.6.0-DEV
  09:20:09.404665 [0-0] > Accept: */*
  09:20:09.404665 [0-0] > Content-Length: 10485760
  09:20:09.404665 [0-0] > Content-Type: application/x-www-form-urlencoded
  09:20:09.404665 [0-0] > 
  09:20:09.405099 [0-0] * STATE: DO => DID handle 0x559f546149f8; line 2194
  09:20:09.405218 [0-0] * STATE: DID => PERFORMING handle 0x559f546149f8; line 2312
  09:20:09.405431 [0-0] } [11995 bytes data]
  09:20:09.515920 [0-0] * Curl_readwrite() -> 55
  09:20:09.515996 [0-0] * multi_done[PERFORMING]: status: 55 prem: 1 done: 0
  09:20:09.516089 [0-x] * Connection still in use 49, no more multi_done now!
  09:20:09.516161 [0-x] * Expire cleared
  
assert 55 == 95
============= 1 failed, 192 passed, 55 skipped in 60.52s (0:01:00) =============

curl/tests/http/test_07_upload.py

Lines 192 to 207 in 9faeebc

# upload large data parallel to a URL that denies uploads
@pytest.mark.parametrize("proto", ['h2', 'h3'])
def test_07_22_upload_parallel_fail(self, env: Env, httpd, nghttpx, repeat, proto):
if proto == 'h3' and not env.have_h3():
pytest.skip("h3 not supported")
if proto == 'h3' and env.curl_uses_lib('msh3'):
pytest.skip("msh3 stalls here")
fdata = os.path.join(env.gen_dir, 'data-10m')
count = 100
curl = CurlClient(env=env)
url = f'https://{env.authority_for(env.domain1, proto)}'\
f'/curltest/tweak?status=400&delay=5ms&chunks=1&body_error=reset&id=[0-{count-1}]'
r = curl.http_upload(urls=[url], data=f'@{fdata}', alpn_proto=proto,
extra_args=['--parallel'])
exp_exit = 92 if proto == 'h2' else 95
r.check_stats(count=count, exitcode=exp_exit)

/cc @icing
@jay jay added HTTP/3 h3 or quic related CI Continuous Integration labels Dec 23, 2023
@jay
Copy link
Member Author

jay commented Dec 23, 2023 •
edited

quiche and ngtcp2 have a difference in send failure error code if the stream is closed but response headers were not received completely. in that case ngtcp2 returns CURLE_HTTP3 and quiche may return CURLE_SEND_ERROR:

ngtcp2:

curl/lib/vquic/curl_ngtcp2.c

Lines 1884 to 1900 in 7161cb1

else if(stream->closed) {
if(stream->resp_hds_complete) {
/* Server decided to close the stream after having sent us a final
* response. This is valid if it is not interested in the request
* body. This happens on 30x or 40x responses.
* We silently discard the data sent, since this is not a transport
* error situation. */
CURL_TRC_CF(data, cf, "[%" PRId64 "] discarding data"
"on closed stream with response", stream->id);
*err = CURLE_OK;
sent = (ssize_t)len;
goto out;
}
*err = CURLE_HTTP3;
sent = -1;
goto out;
}

quiche:

curl/lib/vquic/curl_quiche.c

Lines 1098 to 1127 in 7161cb1

else if(nwritten == QUICHE_H3_TRANSPORT_ERR_INVALID_STREAM_STATE &&
stream->closed && stream->resp_hds_complete) {
/* sending request body on a stream that has been closed by the
* server. If the server has send us a final response, we should
* silently discard the send data.
* This happens for example on redirects where the server, instead
* of reading the full request body just closed the stream after
* sending the 30x response.
* This is sort of a race: had the transfer loop called recv first,
* it would see the response and stop/discard sending on its own- */
CURL_TRC_CF(data, cf, "[%" PRId64 "] discarding data"
"on closed stream with response", stream->id);
*err = CURLE_OK;
nwritten = (ssize_t)len;
goto out;
}
else if(nwritten == QUICHE_H3_TRANSPORT_ERR_FINAL_SIZE) {
CURL_TRC_CF(data, cf, "[%" PRId64 "] send_body(len=%zu) "
"-> exceeds size", stream->id, len);
*err = CURLE_SEND_ERROR;
nwritten = -1;
goto out;
}
else if(nwritten < 0) {
CURL_TRC_CF(data, cf, "[%" PRId64 "] send_body(len=%zu) "
"-> quiche err %zd", stream->id, len, nwritten);
*err = CURLE_SEND_ERROR;
nwritten = -1;
goto out;
}

jay added a commit to jay/curl that referenced this issue Dec 26, 2023
@jay
quiche: return CURLE_HTTP3 on send to invalid stream
6369ba2 
Prior to this change if a send failed on a stream in an invalid state
(according to quiche) and not marked as closed (according to libcurl)
then the send function would return CURLE_SEND_ERROR.

We already have similar code for ngtcp2 to return CURLE_HTTP3 in this
case.

Caught by test test_07_upload.py: test_07_22_upload_parallel_fail.

Fixes curl#12590
Closes #xxxx
@jay jay mentioned this issue Dec 26, 2023
Closed
@jay jay closed this as completed in b83729a Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CI Continuous Integration HTTP/3 h3 or quic related
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

quiche: return CURLE_HTTP3 on send to invalid stream jay/curl
1 participant
@jay