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

diff --git a/lib/http2.c b/lib/http2.c
index f1b3f020b648d372c410f1b65b177d592e224d9c..ed831f6a191e82f7dedb3e79dbd703ac8fa626e3 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -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 "

diff --git a/tests/http/test_05_errors.py b/tests/http/test_05_errors.py
index 219faf3ccf24eb9052e5d9ecf61bba2f86fa79a2..ecace5f4abd2d39c189152717f71b4260c25be1b 100644 (file)
--- a/tests/http/test_05_errors.py
+++ b/tests/http/test_05_errors.py
@@ -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()
+

diff --git a/tests/http/testenv/httpd.py b/tests/http/testenv/httpd.py
index 500f5607ff9aece88fbbcfde2c03fa07f19deea0..962addfc469c1b11a30fda5db6455611cb6f1689 100644 (file)
--- a/tests/http/testenv/httpd.py
+++ b/tests/http/testenv/httpd.py
@@ -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([

diff --git a/tests/http/testenv/mod_curltest/mod_curltest.c b/tests/http/testenv/mod_curltest/mod_curltest.c
index 498f9e536d3f9dbcd074c13922023434529efbc9..30fb765aea4d2d602e166c506df88163d8acbe1b 100644 (file)
--- a/tests/http/testenv/mod_curltest/mod_curltest.c
+++ b/tests/http/testenv/mod_curltest/mod_curltest.c
@@ -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;
+}