Skip to content

Commit 5c58cb0

Browse files
icingbagder
authored andcommitted
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 curl#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 curl#11194 Cloes curl#11200
1 parent 1fe8de8 commit 5c58cb0

File tree

4 files changed

+138
-18
lines changed

4 files changed

+138
-18
lines changed

lib/http2.c

+2-4
Original file line numberDiff line numberDiff line change
@@ -1527,10 +1527,8 @@ static CURLcode http2_data_done_send(struct Curl_cfilter *cf,
15271527
if(!stream->send_closed) {
15281528
stream->send_closed = TRUE;
15291529
if(stream->upload_left) {
1530-
/* If we operated with unknown length, we now know that everything
1531-
* that is buffered is all we have to send. */
1532-
if(stream->upload_left == -1)
1533-
stream->upload_left = Curl_bufq_len(&stream->sendbuf);
1530+
/* we now know that everything that is buffered is all there is. */
1531+
stream->upload_left = Curl_bufq_len(&stream->sendbuf);
15341532
/* resume sending here to trigger the callback to get called again so
15351533
that it can signal EOF to nghttp2 */
15361534
(void)nghttp2_session_resume_data(ctx->h2, stream->id);

tests/http/test_07_upload.py

+20
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,26 @@ def test_07_33_issue_11157b(self, env: Env, httpd, nghttpx, repeat):
285285
assert r.exit_code == 0, r.dump_logs()
286286
r.check_stats(1, 200)
287287

288+
def test_07_34_issue_11194(self, env: Env, httpd, nghttpx, repeat):
289+
proto = 'h2'
290+
fdata = os.path.join(env.gen_dir, 'data-10m')
291+
# tell our test PUT handler to read the upload more slowly, so
292+
# that the send buffering and transfer loop needs to wait
293+
fdata = os.path.join(env.gen_dir, 'data-100k')
294+
url = f'https://{env.authority_for(env.domain1, proto)}/curltest/put'
295+
curl = CurlClient(env=env)
296+
r = curl.run_direct(with_stats=True, args=[
297+
'--verbose',
298+
'--resolve', f'{env.authority_for(env.domain1, proto)}:127.0.0.1',
299+
'--cacert', env.ca.cert_file,
300+
'--request', 'PUT',
301+
'--digest', '--user', 'test:test',
302+
'--data-binary', f'@{fdata}'
303+
'--url', url,
304+
])
305+
assert r.exit_code == 0, r.dump_logs()
306+
r.check_stats(1, 200)
307+
288308
def check_download(self, count, srcfile, curl):
289309
for i in range(count):
290310
dfile = curl.download_file(i)

tests/http/test_14_auth.py

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
#!/usr/bin/env python3
2+
# -*- coding: utf-8 -*-
3+
#***************************************************************************
4+
# _ _ ____ _
5+
# Project ___| | | | _ \| |
6+
# / __| | | | |_) | |
7+
# | (__| |_| | _ <| |___
8+
# \___|\___/|_| \_\_____|
9+
#
10+
# Copyright (C) Daniel Stenberg, <[email protected]>, et al.
11+
#
12+
# This software is licensed as described in the file COPYING, which
13+
# you should have received as part of this distribution. The terms
14+
# are also available at https://curl.se/docs/copyright.html.
15+
#
16+
# You may opt to use, copy, modify, merge, publish, distribute and/or sell
17+
# copies of the Software, and permit persons to whom the Software is
18+
# furnished to do so, under the terms of the COPYING file.
19+
#
20+
# This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
21+
# KIND, either express or implied.
22+
#
23+
# SPDX-License-Identifier: curl
24+
#
25+
###########################################################################
26+
#
27+
import difflib
28+
import filecmp
29+
import logging
30+
import os
31+
import pytest
32+
33+
from testenv import Env, CurlClient, LocalClient
34+
35+
36+
log = logging.getLogger(__name__)
37+
38+
39+
class TestAuth:
40+
41+
@pytest.fixture(autouse=True, scope='class')
42+
def _class_scope(self, env, httpd, nghttpx):
43+
if env.have_h3():
44+
nghttpx.start_if_needed()
45+
httpd.clear_extra_configs()
46+
httpd.reload()
47+
48+
# download 1 file, not authenticated
49+
@pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
50+
def test_14_01_digest_get_noauth(self, env: Env, httpd, nghttpx, repeat, proto):
51+
if proto == 'h3' and not env.have_h3():
52+
pytest.skip("h3 not supported")
53+
curl = CurlClient(env=env)
54+
url = f'https://{env.authority_for(env.domain1, proto)}/restricted/digest/data.json'
55+
r = curl.http_download(urls=[url], alpn_proto=proto)
56+
r.check_response(http_status=401)
57+
58+
# download 1 file, authenticated
59+
@pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
60+
def test_14_02_digest_get_auth(self, env: Env, httpd, nghttpx, repeat, proto):
61+
if proto == 'h3' and not env.have_h3():
62+
pytest.skip("h3 not supported")
63+
curl = CurlClient(env=env)
64+
url = f'https://{env.authority_for(env.domain1, proto)}/restricted/digest/data.json'
65+
r = curl.http_download(urls=[url], alpn_proto=proto, extra_args=[
66+
'--digest', '--user', 'test:test'
67+
])
68+
r.check_response(http_status=200)
69+
70+
# PUT data, authenticated
71+
@pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
72+
def test_14_03_digest_put_auth(self, env: Env, httpd, nghttpx, repeat, proto):
73+
if proto == 'h3' and not env.have_h3():
74+
pytest.skip("h3 not supported")
75+
data='0123456789'
76+
curl = CurlClient(env=env)
77+
url = f'https://{env.authority_for(env.domain1, proto)}/restricted/digest/data.json'
78+
r = curl.http_upload(urls=[url], data=data, alpn_proto=proto, extra_args=[
79+
'--digest', '--user', 'test:test'
80+
])
81+
r.check_response(http_status=200)

tests/http/testenv/httpd.py

+35-14
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,11 @@ def __init__(self, env: Env, proxy_auth: bool = False):
7070
self._logs_dir = os.path.join(self._apache_dir, 'logs')
7171
self._error_log = os.path.join(self._logs_dir, 'error_log')
7272
self._tmp_dir = os.path.join(self._apache_dir, 'tmp')
73-
self._passwords = os.path.join(self._conf_dir, 'passwords')
73+
self._basic_passwords = os.path.join(self._conf_dir, 'basic.passwords')
74+
self._digest_passwords = os.path.join(self._conf_dir, 'digest.passwords')
7475
self._mods_dir = None
75-
self._proxy_auth = proxy_auth
76+
self._auth_digest = True
77+
self._proxy_auth_basic = proxy_auth
7678
self._extra_configs = {}
7779
assert env.apxs
7880
p = subprocess.run(args=[env.apxs, '-q', 'libexecdir'],
@@ -108,7 +110,7 @@ def clear_extra_configs(self):
108110
self._extra_configs = {}
109111

110112
def set_proxy_auth(self, active: bool):
111-
self._proxy_auth = active
113+
self._proxy_auth_basic = active
112114

113115
def _run(self, args, intext=''):
114116
env = {}
@@ -219,9 +221,15 @@ def _write_config(self):
219221
'server': f'{domain2}',
220222
}
221223
fd.write(JSONEncoder().encode(data))
222-
if self._proxy_auth:
223-
with open(self._passwords, 'w') as fd:
224+
if self._proxy_auth_basic:
225+
with open(self._basic_passwords, 'w') as fd:
224226
fd.write('proxy:$apr1$FQfeInbs$WQZbODJlVg60j0ogEIlTW/\n')
227+
if self._auth_digest:
228+
with open(self._digest_passwords, 'w') as fd:
229+
fd.write('test:restricted area:57123e269fd73d71ae0656594e938e2f\n')
230+
self._mkpath(os.path.join(self.docs_dir, 'restricted/digest'))
231+
with open(os.path.join(self.docs_dir, 'restricted/digest/data.json'), 'w') as fd:
232+
fd.write('{"area":"digest"}\n')
225233
with open(self._conf_file, 'w') as fd:
226234
for m in self.MODULES:
227235
if os.path.exists(os.path.join(self._mods_dir, f'mod_{m}.so')):
@@ -252,7 +260,7 @@ def _write_config(self):
252260
f' DocumentRoot "{self._docs_dir}"',
253261
f' Protocols h2c http/1.1',
254262
])
255-
conf.extend(self._curltest_conf())
263+
conf.extend(self._curltest_conf(domain1))
256264
conf.extend([
257265
f'</VirtualHost>',
258266
f'',
@@ -267,7 +275,7 @@ def _write_config(self):
267275
f' SSLCertificateKeyFile {creds1.pkey_file}',
268276
f' DocumentRoot "{self._docs_dir}"',
269277
])
270-
conf.extend(self._curltest_conf())
278+
conf.extend(self._curltest_conf(domain1))
271279
if domain1 in self._extra_configs:
272280
conf.extend(self._extra_configs[domain1])
273281
conf.extend([
@@ -283,7 +291,7 @@ def _write_config(self):
283291
f' SSLCertificateKeyFile {creds2.pkey_file}',
284292
f' DocumentRoot "{self._docs_dir}/two"',
285293
])
286-
conf.extend(self._curltest_conf())
294+
conf.extend(self._curltest_conf(domain2))
287295
if domain2 in self._extra_configs:
288296
conf.extend(self._extra_configs[domain2])
289297
conf.extend([
@@ -329,13 +337,13 @@ def _write_config(self):
329337
]))
330338

331339
def _get_proxy_conf(self):
332-
if self._proxy_auth:
340+
if self._proxy_auth_basic:
333341
return [
334342
f' <Proxy "*">',
335343
f' AuthType Basic',
336344
f' AuthName "Restricted Proxy"',
337345
f' AuthBasicProvider file',
338-
f' AuthUserFile "{self._passwords}"',
346+
f' AuthUserFile "{self._basic_passwords}"',
339347
f' Require user proxy',
340348
f' </Proxy>',
341349
]
@@ -355,9 +363,10 @@ def _get_log_level(self):
355363
return 'debug'
356364
return 'info'
357365

358-
def _curltest_conf(self) -> List[str]:
366+
def _curltest_conf(self, servername) -> List[str]:
367+
lines = []
359368
if Httpd.MOD_CURLTEST is not None:
360-
return [
369+
lines.extend([
361370
f' <Location /curltest/echo>',
362371
f' SetHandler curltest-echo',
363372
f' </Location>',
@@ -367,8 +376,20 @@ def _curltest_conf(self) -> List[str]:
367376
f' <Location /curltest/tweak>',
368377
f' SetHandler curltest-tweak',
369378
f' </Location>',
370-
]
371-
return []
379+
])
380+
if self._auth_digest:
381+
lines.extend([
382+
f' <Directory {self.docs_dir}/restricted/digest>',
383+
f' AuthType Digest',
384+
f' AuthName "restricted area"',
385+
f' AuthDigestDomain "https://{servername}"',
386+
f' AuthBasicProvider file',
387+
f' AuthUserFile "{self._digest_passwords}"',
388+
f' Require valid-user',
389+
f' </Directory>',
390+
391+
])
392+
return lines
372393

373394
def _init_curltest(self):
374395
if Httpd.MOD_CURLTEST is not None:

0 commit comments

Comments
 (0)