From: Stefan Eissing <stefan@eissing.org>
Date: Thu, 1 Feb 2024 17:15:50 +0000 (+0100)
Subject: vtls: revert "receive max buffer" + add test case
X-Git-Tag: curl-8_7_0~204
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ed09a99af57200643d5ae001e815eeab9ffe3f84;p=thirdparty%2Fcurl.git

vtls: revert "receive max buffer" + add test case

- add test_05_04 for requests using http/1.0, http/1.1 and h2 against an
  Apache resource that does an unclean TLS shutdown.
- revert special workarund in openssl.c for suppressing shutdown errors
  on multiplexed connections
- vlts.c restore to its state before 9a90c9dd64d2f03601833a70786d485851bd1b53

Fixes #12885
Fixes #12844

Closes #12848
---

diff --git a/lib/vtls/vtls.c b/lib/vtls/vtls.c
index e928ba5d07..f654a9749c 100644
--- a/lib/vtls/vtls.c
+++ b/lib/vtls/vtls.c
@@ -1715,32 +1715,17 @@ static ssize_t ssl_cf_recv(struct Curl_cfilter *cf,
 {
   struct cf_call_data save;
   ssize_t nread;
-  size_t ntotal = 0;
 
   CF_DATA_SAVE(save, cf, data);
   *err = CURLE_OK;
-  /* Do receive until we fill the buffer somehwhat or EGAIN, error or EOF */
-  while(!ntotal || (len - ntotal) > (4*1024)) {
+  nread = Curl_ssl->recv_plain(cf, data, buf, len, err);
+  if(nread > 0) {
+    DEBUGASSERT((size_t)nread <= len);
+  }
+  else if(nread == 0) {
+    /* eof */
     *err = CURLE_OK;
-    nread = Curl_ssl->recv_plain(cf, data, buf + ntotal, len - ntotal, err);
-    if(nread < 0) {
-      if(*err == CURLE_AGAIN && ntotal > 0) {
-        /* we EAGAINed after having reed data, return the success amount */
-        *err = CURLE_OK;
-        break;
-      }
-      /* we have a an error to report */
-      goto out;
-    }
-    else if(nread == 0) {
-      /* eof */
-      break;
-    }
-    ntotal += (size_t)nread;
-    DEBUGASSERT((size_t)ntotal <= len);
   }
-  nread = (ssize_t)ntotal;
-out:
   CURL_TRC_CF(data, cf, "cf_recv(len=%zu) -> %zd, %d", len,
               nread, *err);
   CF_DATA_RESTORE(cf, save);
diff --git a/tests/http/test_05_errors.py b/tests/http/test_05_errors.py
index b59f3f177a..68e49f7362 100644
--- a/tests/http/test_05_errors.py
+++ b/tests/http/test_05_errors.py
@@ -108,3 +108,30 @@ class TestErrors:
         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()
+
+    # On the URL used here, Apache is doing an "unclean" TLS shutdown,
+    # meaning it sends no shutdown notice and just closes TCP.
+    # The HTTP response delivers a body without Content-Length. We expect:
+    # - http/1.0 to fail since it relies on a clean connection close to
+    #   detect the end of the body
+    # - http/1.1 to work since it will used "chunked" transfer encoding
+    #   and stop receiving when that signals the end
+    # - h2 to work since it will signal the end of the response before
+    #   and not see the "unclean" close either
+    @pytest.mark.parametrize("proto", ['http/1.0', 'http/1.1', 'h2'])
+    def test_05_04_unclean_tls_shutdown(self, env: Env, httpd, nghttpx, repeat, proto):
+        if proto == 'h3' and not env.have_h3():
+            pytest.skip("h3 not supported")
+        count = 10 if proto == 'h2' else 1
+        curl = CurlClient(env=env)
+        url = f'https://{env.authority_for(env.domain1, proto)}'\
+                f'/curltest/shutdown_unclean?id=[0-{count-1}]&chunks=4'
+        r = curl.http_download(urls=[url], alpn_proto=proto, extra_args=[
+            '--parallel',
+        ])
+        if proto == 'http/1.0':
+            r.check_exit_code(56)
+        else:
+            r.check_exit_code(0)
+            r.check_response(http_status=200, count=count)
+
diff --git a/tests/http/testenv/httpd.py b/tests/http/testenv/httpd.py
index 79497c5b30..c04c22699a 100644
--- a/tests/http/testenv/httpd.py
+++ b/tests/http/testenv/httpd.py
@@ -47,7 +47,7 @@ class Httpd:
         'authn_core', 'authn_file',
         'authz_user', 'authz_core', 'authz_host',
         'auth_basic', 'auth_digest',
-        'alias', 'env', 'filter', 'headers', 'mime',
+        'alias', 'env', 'filter', 'headers', 'mime', 'setenvif',
         'socache_shmcb',
         'rewrite', 'http2', 'ssl', 'proxy', 'proxy_http', 'proxy_connect',
         'mpm_event',
@@ -389,6 +389,11 @@ class Httpd:
                 f'    <Location /curltest/1_1>',
                 f'      SetHandler curltest-1_1-required',
                 f'    </Location>',
+                f'    <Location /curltest/shutdown_unclean>',
+                f'      SetHandler curltest-tweak',
+                f'      SetEnv force-response-1.0 1',
+                f'    </Location>',
+                f'    SetEnvIf Request_URI "/shutdown_unclean" ssl-unclean=1',
             ])
         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 ff1983d17f..a066be5222 100644
--- a/tests/http/testenv/mod_curltest/mod_curltest.c
+++ b/tests/http/testenv/mod_curltest/mod_curltest.c
@@ -347,7 +347,7 @@ static int curltest_tweak_handler(request_rec *r)
                 "request, %s", r->args? r->args : "(no args)");
   r->status = http_status;
   r->clength = -1;
-  r->chunked = 1;
+  r->chunked = (r->proto_num >= HTTP_VERSION(1,1));
   apr_table_setn(r->headers_out, "request-id", request_id);
   apr_table_unset(r->headers_out, "Content-Length");
   /* Discourage content-encodings */