http2: error stream resets with code CURLE_HTTP2_STREAM

- refs #11357, where it was reported that HTTP/1.1 downgrades
  no longer works
- fixed with suggested change
- added test_05_03 and a new handler in the curltest module
  to reproduce that downgrades work

Fixes #11357
Closes #11362
Reported-by: Jay Satiro
This commit is contained in:
Stefan Eissing 2023-06-21 15:59:42 +02:00 committed by Daniel Stenberg
parent 27242bbad3
commit d435bf1baf
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
4 changed files with 95 additions and 6 deletions

View File

@ -1584,11 +1584,6 @@ static ssize_t http2_handle_stream_close(struct Curl_cfilter *cf,
*err = CURLE_SEND_ERROR; /* trigger Curl_retry_request() later */
return -1;
}
else if(stream->reset) {
failf(data, "HTTP/2 stream %u was reset", stream->id);
*err = stream->bodystarted? CURLE_PARTIAL_FILE : CURLE_RECV_ERROR;
return -1;
}
else if(stream->error != NGHTTP2_NO_ERROR) {
failf(data, "HTTP/2 stream %u was not closed cleanly: %s (err %u)",
stream->id, nghttp2_http2_strerror(stream->error),
@ -1596,6 +1591,11 @@ static ssize_t http2_handle_stream_close(struct Curl_cfilter *cf,
*err = CURLE_HTTP2_STREAM;
return -1;
}
else if(stream->reset) {
failf(data, "HTTP/2 stream %u was reset", stream->id);
*err = stream->bodystarted? CURLE_PARTIAL_FILE : CURLE_RECV_ERROR;
return -1;
}
if(!stream->bodystarted) {
failf(data, "HTTP/2 stream %u was closed cleanly, but before getting "

View File

@ -92,3 +92,20 @@ class TestErrors:
if 'exitcode' not in s or s['exitcode'] not in [18, 55, 56, 92, 95]:
invalid_stats.append(f'request {idx} exit with {s["exitcode"]}\n{s}')
assert len(invalid_stats) == 0, f'failed: {invalid_stats}'
# access a resource that, on h2, RST the stream with HTTP_1_1_REQUIRED
def test_05_03_required(self, env: Env, httpd, nghttpx, repeat):
curl = CurlClient(env=env)
proto = 'http/1.1'
urln = f'https://{env.authority_for(env.domain1, proto)}/curltest/1_1'
r = curl.http_download(urls=[urln], alpn_proto=proto)
r.check_exit_code(0)
r.check_response(http_status=200, count=1)
proto = 'h2'
urln = f'https://{env.authority_for(env.domain1, proto)}/curltest/1_1'
r = curl.http_download(urls=[urln], alpn_proto=proto)
r.check_exit_code(0)
r.check_response(http_status=200, count=1)
# check that we did a downgrade
assert r.stats[0]['http_version'] == '1.1', r.dump_logs()

View File

@ -376,6 +376,9 @@ class Httpd:
f' <Location /curltest/tweak>',
f' SetHandler curltest-tweak',
f' </Location>',
f' <Location /curltest/1_1>',
f' SetHandler curltest-1_1-required',
f' </Location>',
])
if self._auth_digest:
lines.extend([

View File

@ -37,6 +37,7 @@ static void curltest_hooks(apr_pool_t *pool);
static int curltest_echo_handler(request_rec *r);
static int curltest_put_handler(request_rec *r);
static int curltest_tweak_handler(request_rec *r);
static int curltest_1_1_required(request_rec *r);
AP_DECLARE_MODULE(curltest) = {
STANDARD20_MODULE_STUFF,
@ -84,6 +85,7 @@ static void curltest_hooks(apr_pool_t *pool)
ap_hook_handler(curltest_echo_handler, NULL, NULL, APR_HOOK_MIDDLE);
ap_hook_handler(curltest_put_handler, NULL, NULL, APR_HOOK_MIDDLE);
ap_hook_handler(curltest_tweak_handler, NULL, NULL, APR_HOOK_MIDDLE);
ap_hook_handler(curltest_1_1_required, NULL, NULL, APR_HOOK_MIDDLE);
}
#define SECS_PER_HOUR (60*60)
@ -400,7 +402,7 @@ cleanup:
if(rv == APR_SUCCESS) {
return OK;
}
if(error_bucket && 0) {
if(error_bucket) {
http_status = ap_map_http_request_error(rv, HTTP_BAD_REQUEST);
b = ap_bucket_error_create(http_status, NULL, r->pool, c->bucket_alloc);
ap_log_rerror(APLOG_MARK, APLOG_TRACE1, rv, r,
@ -512,3 +514,70 @@ cleanup:
return DECLINED;
}
static int curltest_1_1_required(request_rec *r)
{
conn_rec *c = r->connection;
apr_bucket_brigade *bb;
apr_bucket *b;
apr_status_t rv;
char buffer[16*1024];
const char *ct;
const char *request_id = "none";
apr_time_t chunk_delay = 0;
apr_array_header_t *args = NULL;
long l;
int i;
if(strcmp(r->handler, "curltest-1_1-required")) {
return DECLINED;
}
if (HTTP_VERSION_MAJOR(r->proto_num) > 1) {
apr_table_setn(r->notes, "ssl-renegotiate-forbidden", "1");
ap_die(HTTP_FORBIDDEN, r);
return OK;
}
ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, "1_1_handler: processing");
r->status = 200;
r->clength = -1;
r->chunked = 1;
apr_table_unset(r->headers_out, "Content-Length");
/* Discourage content-encodings */
apr_table_unset(r->headers_out, "Content-Encoding");
apr_table_setn(r->subprocess_env, "no-brotli", "1");
apr_table_setn(r->subprocess_env, "no-gzip", "1");
ct = apr_table_get(r->headers_in, "content-type");
ap_set_content_type(r, ct? ct : "text/plain");
bb = apr_brigade_create(r->pool, c->bucket_alloc);
/* flush response */
b = apr_bucket_flush_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, b);
rv = ap_pass_brigade(r->output_filters, bb);
if (APR_SUCCESS != rv) goto cleanup;
/* we are done */
rv = apr_brigade_printf(bb, NULL, NULL, "well done!");
if(APR_SUCCESS != rv) goto cleanup;
b = apr_bucket_eos_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, b);
ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, r, "1_1_handler: request read");
rv = ap_pass_brigade(r->output_filters, bb);
cleanup:
if(rv == APR_SUCCESS
|| r->status != HTTP_OK
|| c->aborted) {
ap_log_rerror(APLOG_MARK, APLOG_TRACE1, rv, r, "1_1_handler: done");
return OK;
}
else {
/* no way to know what type of error occurred */
ap_log_rerror(APLOG_MARK, APLOG_TRACE1, rv, r, "1_1_handler failed");
return AP_FILTER_ERROR;
}
return DECLINED;
}