From: Stefan Eissing Date: Wed, 24 May 2023 16:48:16 +0000 (+0200) Subject: http2: fix EOF handling on uploads with auth negotiation X-Git-Tag: curl-8_1_2~16 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5c58cb0212bcf63cce33a974906bf9905948b4bb;p=thirdparty%2Fcurl.git http2: fix EOF handling on uploads with auth negotiation - doing a POST with `--digest` does an override on the initial request with `Content-Length: 0`, but the http2 filter was unaware of that and expected the originally request body. It did therefore not send a final DATA frame with EOF flag to the server. - The fix overrides any initial notion of post size when the `done_send` event is triggered by the transfer loop, leading to the EOF that is necessary. - refs #11194. The fault did not happen in testing, as Apache httpd never tries to read the request body of the initial request, sends the 401 reply and closes the stream. The server used in the reported issue however tried to read the EOF and timed out on the request. Reported-by: Aleksander Mazur Fixes #11194 Cloes #11200 --- diff --git a/lib/http2.c b/lib/http2.c index 224424bf62..191d8cd335 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -1527,10 +1527,8 @@ static CURLcode http2_data_done_send(struct Curl_cfilter *cf, if(!stream->send_closed) { stream->send_closed = TRUE; if(stream->upload_left) { - /* If we operated with unknown length, we now know that everything - * that is buffered is all we have to send. */ - if(stream->upload_left == -1) - stream->upload_left = Curl_bufq_len(&stream->sendbuf); + /* we now know that everything that is buffered is all there is. */ + stream->upload_left = Curl_bufq_len(&stream->sendbuf); /* resume sending here to trigger the callback to get called again so that it can signal EOF to nghttp2 */ (void)nghttp2_session_resume_data(ctx->h2, stream->id); diff --git a/tests/http/test_07_upload.py b/tests/http/test_07_upload.py index f28c644f01..84a23b2615 100644 --- a/tests/http/test_07_upload.py +++ b/tests/http/test_07_upload.py @@ -285,6 +285,26 @@ class TestUpload: assert r.exit_code == 0, r.dump_logs() r.check_stats(1, 200) + def test_07_34_issue_11194(self, env: Env, httpd, nghttpx, repeat): + proto = 'h2' + fdata = os.path.join(env.gen_dir, 'data-10m') + # tell our test PUT handler to read the upload more slowly, so + # that the send buffering and transfer loop needs to wait + fdata = os.path.join(env.gen_dir, 'data-100k') + url = f'https://{env.authority_for(env.domain1, proto)}/curltest/put' + curl = CurlClient(env=env) + r = curl.run_direct(with_stats=True, args=[ + '--verbose', + '--resolve', f'{env.authority_for(env.domain1, proto)}:127.0.0.1', + '--cacert', env.ca.cert_file, + '--request', 'PUT', + '--digest', '--user', 'test:test', + '--data-binary', f'@{fdata}' + '--url', url, + ]) + assert r.exit_code == 0, r.dump_logs() + r.check_stats(1, 200) + def check_download(self, count, srcfile, curl): for i in range(count): dfile = curl.download_file(i) diff --git a/tests/http/test_14_auth.py b/tests/http/test_14_auth.py new file mode 100644 index 0000000000..23acb35989 --- /dev/null +++ b/tests/http/test_14_auth.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +#*************************************************************************** +# _ _ ____ _ +# Project ___| | | | _ \| | +# / __| | | | |_) | | +# | (__| |_| | _ <| |___ +# \___|\___/|_| \_\_____| +# +# Copyright (C) Daniel Stenberg, , et al. +# +# This software is licensed as described in the file COPYING, which +# you should have received as part of this distribution. The terms +# are also available at https://curl.se/docs/copyright.html. +# +# You may opt to use, copy, modify, merge, publish, distribute and/or sell +# copies of the Software, and permit persons to whom the Software is +# furnished to do so, under the terms of the COPYING file. +# +# This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY +# KIND, either express or implied. +# +# SPDX-License-Identifier: curl +# +########################################################################### +# +import difflib +import filecmp +import logging +import os +import pytest + +from testenv import Env, CurlClient, LocalClient + + +log = logging.getLogger(__name__) + + +class TestAuth: + + @pytest.fixture(autouse=True, scope='class') + def _class_scope(self, env, httpd, nghttpx): + if env.have_h3(): + nghttpx.start_if_needed() + httpd.clear_extra_configs() + httpd.reload() + + # download 1 file, not authenticated + @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3']) + def test_14_01_digest_get_noauth(self, env: Env, httpd, nghttpx, repeat, proto): + if proto == 'h3' and not env.have_h3(): + pytest.skip("h3 not supported") + curl = CurlClient(env=env) + url = f'https://{env.authority_for(env.domain1, proto)}/restricted/digest/data.json' + r = curl.http_download(urls=[url], alpn_proto=proto) + r.check_response(http_status=401) + + # download 1 file, authenticated + @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3']) + def test_14_02_digest_get_auth(self, env: Env, httpd, nghttpx, repeat, proto): + if proto == 'h3' and not env.have_h3(): + pytest.skip("h3 not supported") + curl = CurlClient(env=env) + url = f'https://{env.authority_for(env.domain1, proto)}/restricted/digest/data.json' + r = curl.http_download(urls=[url], alpn_proto=proto, extra_args=[ + '--digest', '--user', 'test:test' + ]) + r.check_response(http_status=200) + + # PUT data, authenticated + @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3']) + def test_14_03_digest_put_auth(self, env: Env, httpd, nghttpx, repeat, proto): + if proto == 'h3' and not env.have_h3(): + pytest.skip("h3 not supported") + data='0123456789' + curl = CurlClient(env=env) + url = f'https://{env.authority_for(env.domain1, proto)}/restricted/digest/data.json' + r = curl.http_upload(urls=[url], data=data, alpn_proto=proto, extra_args=[ + '--digest', '--user', 'test:test' + ]) + r.check_response(http_status=200) diff --git a/tests/http/testenv/httpd.py b/tests/http/testenv/httpd.py index d3b2de4cda..500f5607ff 100644 --- a/tests/http/testenv/httpd.py +++ b/tests/http/testenv/httpd.py @@ -70,9 +70,11 @@ class Httpd: self._logs_dir = os.path.join(self._apache_dir, 'logs') self._error_log = os.path.join(self._logs_dir, 'error_log') self._tmp_dir = os.path.join(self._apache_dir, 'tmp') - self._passwords = os.path.join(self._conf_dir, 'passwords') + self._basic_passwords = os.path.join(self._conf_dir, 'basic.passwords') + self._digest_passwords = os.path.join(self._conf_dir, 'digest.passwords') self._mods_dir = None - self._proxy_auth = proxy_auth + self._auth_digest = True + self._proxy_auth_basic = proxy_auth self._extra_configs = {} assert env.apxs p = subprocess.run(args=[env.apxs, '-q', 'libexecdir'], @@ -108,7 +110,7 @@ class Httpd: self._extra_configs = {} def set_proxy_auth(self, active: bool): - self._proxy_auth = active + self._proxy_auth_basic = active def _run(self, args, intext=''): env = {} @@ -219,9 +221,15 @@ class Httpd: 'server': f'{domain2}', } fd.write(JSONEncoder().encode(data)) - if self._proxy_auth: - with open(self._passwords, 'w') as fd: + if self._proxy_auth_basic: + with open(self._basic_passwords, 'w') as fd: fd.write('proxy:$apr1$FQfeInbs$WQZbODJlVg60j0ogEIlTW/\n') + if self._auth_digest: + with open(self._digest_passwords, 'w') as fd: + fd.write('test:restricted area:57123e269fd73d71ae0656594e938e2f\n') + self._mkpath(os.path.join(self.docs_dir, 'restricted/digest')) + with open(os.path.join(self.docs_dir, 'restricted/digest/data.json'), 'w') as fd: + fd.write('{"area":"digest"}\n') with open(self._conf_file, 'w') as fd: for m in self.MODULES: if os.path.exists(os.path.join(self._mods_dir, f'mod_{m}.so')): @@ -252,7 +260,7 @@ class Httpd: f' DocumentRoot "{self._docs_dir}"', f' Protocols h2c http/1.1', ]) - conf.extend(self._curltest_conf()) + conf.extend(self._curltest_conf(domain1)) conf.extend([ f'', f'', @@ -267,7 +275,7 @@ class Httpd: f' SSLCertificateKeyFile {creds1.pkey_file}', f' DocumentRoot "{self._docs_dir}"', ]) - conf.extend(self._curltest_conf()) + conf.extend(self._curltest_conf(domain1)) if domain1 in self._extra_configs: conf.extend(self._extra_configs[domain1]) conf.extend([ @@ -283,7 +291,7 @@ class Httpd: f' SSLCertificateKeyFile {creds2.pkey_file}', f' DocumentRoot "{self._docs_dir}/two"', ]) - conf.extend(self._curltest_conf()) + conf.extend(self._curltest_conf(domain2)) if domain2 in self._extra_configs: conf.extend(self._extra_configs[domain2]) conf.extend([ @@ -329,13 +337,13 @@ class Httpd: ])) def _get_proxy_conf(self): - if self._proxy_auth: + if self._proxy_auth_basic: return [ f' ', f' AuthType Basic', f' AuthName "Restricted Proxy"', f' AuthBasicProvider file', - f' AuthUserFile "{self._passwords}"', + f' AuthUserFile "{self._basic_passwords}"', f' Require user proxy', f' ', ] @@ -355,9 +363,10 @@ class Httpd: return 'debug' return 'info' - def _curltest_conf(self) -> List[str]: + def _curltest_conf(self, servername) -> List[str]: + lines = [] if Httpd.MOD_CURLTEST is not None: - return [ + lines.extend([ f' ', f' SetHandler curltest-echo', f' ', @@ -367,8 +376,20 @@ class Httpd: f' ', f' SetHandler curltest-tweak', f' ', - ] - return [] + ]) + if self._auth_digest: + lines.extend([ + f' ', + f' AuthType Digest', + f' AuthName "restricted area"', + f' AuthDigestDomain "https://{servername}"', + f' AuthBasicProvider file', + f' AuthUserFile "{self._digest_passwords}"', + f' Require valid-user', + f' ', + + ]) + return lines def _init_curltest(self): if Httpd.MOD_CURLTEST is not None: