From 5dcbc00989c94e314a018a8a9d79027796e95ffe Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:18:55 -0400 Subject: [PATCH 1/8] News fragment. --- newsfragments/4012.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/4012.bugfix diff --git a/newsfragments/4012.bugfix b/newsfragments/4012.bugfix new file mode 100644 index 000000000..97dfe6aad --- /dev/null +++ b/newsfragments/4012.bugfix @@ -0,0 +1 @@ +The command-line tools now have a 60-second timeout on individual network reads/writes/connects; previously they could block forever in some situations. \ No newline at end of file From d7ee1637dfabc3654ea6be735fbcd2094d39c1f3 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:22:06 -0400 Subject: [PATCH 2/8] Set a timeout. --- src/allmydata/scripts/common_http.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 95099a2eb..7542a045f 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -62,9 +62,9 @@ def do_http(method, url, body=b""): assert body.read scheme, host, port, path = parse_url(url) if scheme == "http": - c = http_client.HTTPConnection(host, port) + c = http_client.HTTPConnection(host, port, timeout=60) elif scheme == "https": - c = http_client.HTTPSConnection(host, port) + c = http_client.HTTPSConnection(host, port, timeout=60) else: raise ValueError("unknown scheme '%s', need http or https" % scheme) c.putrequest(method, path) From 67702572a9d4ce03c6614207234ae909672d6df5 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:22:14 -0400 Subject: [PATCH 3/8] Do a little modernization. --- src/allmydata/scripts/common_http.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 7542a045f..4da1345c9 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -1,18 +1,11 @@ """ Ported to Python 3. """ -from __future__ import unicode_literals -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function - -from future.utils import PY2 -if PY2: - from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 import os from io import BytesIO -from six.moves import urllib, http_client +from http import client as http_client +import urllib import six import allmydata # for __full_version__ From 1823dd4c03b3d715ef896453542d0ac10e7f4aad Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:24:00 -0400 Subject: [PATCH 4/8] Switch to a slightly larger block size. --- src/allmydata/scripts/common_http.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 4da1345c9..4c0319d3c 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -55,9 +55,9 @@ def do_http(method, url, body=b""): assert body.read scheme, host, port, path = parse_url(url) if scheme == "http": - c = http_client.HTTPConnection(host, port, timeout=60) + c = http_client.HTTPConnection(host, port, timeout=60, blocksize=65536) elif scheme == "https": - c = http_client.HTTPSConnection(host, port, timeout=60) + c = http_client.HTTPSConnection(host, port, timeout=60, blocksize=65536) else: raise ValueError("unknown scheme '%s', need http or https" % scheme) c.putrequest(method, path) @@ -78,7 +78,7 @@ def do_http(method, url, body=b""): return BadResponse(url, err) while True: - data = body.read(8192) + data = body.read(65536) if not data: break c.send(data) From 2916984114bdb9ef85df5a34aa439f45c5a14cab Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 10:29:25 -0400 Subject: [PATCH 5/8] More modernization. --- src/allmydata/scripts/common_http.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 4c0319d3c..7d627a7ad 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -1,12 +1,11 @@ """ -Ported to Python 3. +Blocking HTTP client APIs. """ import os from io import BytesIO from http import client as http_client import urllib -import six import allmydata # for __full_version__ from allmydata.util.encodingutil import quote_output @@ -44,7 +43,7 @@ class BadResponse(object): def do_http(method, url, body=b""): if isinstance(body, bytes): body = BytesIO(body) - elif isinstance(body, six.text_type): + elif isinstance(body, str): raise TypeError("do_http body must be a bytestring, not unicode") else: # We must give a Content-Length header to twisted.web, otherwise it @@ -87,16 +86,14 @@ def do_http(method, url, body=b""): def format_http_success(resp): - # ensure_text() shouldn't be necessary when Python 2 is dropped. return quote_output( - "%s %s" % (resp.status, six.ensure_text(resp.reason)), + "%s %s" % (resp.status, resp.reason), quotemarks=False) def format_http_error(msg, resp): - # ensure_text() shouldn't be necessary when Python 2 is dropped. return quote_output( - "%s: %s %s\n%s" % (msg, resp.status, six.ensure_text(resp.reason), - six.ensure_text(resp.read())), + "%s: %s %s\n%s" % (msg, resp.status, resp.reason, + resp.read()), quotemarks=False) def check_http_error(resp, stderr): From 2d81ddc297b336607bc8eac691913e7e3ac2f178 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 14 Apr 2023 11:15:47 -0400 Subject: [PATCH 6/8] Don't call str() on bytes. --- src/allmydata/scripts/common_http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 7d627a7ad..a2cae5a85 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -92,7 +92,7 @@ def format_http_success(resp): def format_http_error(msg, resp): return quote_output( - "%s: %s %s\n%s" % (msg, resp.status, resp.reason, + "%s: %s %s\n%r" % (msg, resp.status, resp.reason, resp.read()), quotemarks=False) From ebed5100b9cf2a208875eb2977da23364559881d Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 25 Apr 2023 08:16:12 -0400 Subject: [PATCH 7/8] Switch to longer timeout so it's unlikely to impact users. --- newsfragments/4012.bugfix | 2 +- src/allmydata/scripts/common_http.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/newsfragments/4012.bugfix b/newsfragments/4012.bugfix index 97dfe6aad..24d5bb49a 100644 --- a/newsfragments/4012.bugfix +++ b/newsfragments/4012.bugfix @@ -1 +1 @@ -The command-line tools now have a 60-second timeout on individual network reads/writes/connects; previously they could block forever in some situations. \ No newline at end of file +The command-line tools now have a 300-second timeout on individual network reads/writes/connects; previously they could block forever in some situations. \ No newline at end of file diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index a2cae5a85..46676b3f5 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -54,9 +54,9 @@ def do_http(method, url, body=b""): assert body.read scheme, host, port, path = parse_url(url) if scheme == "http": - c = http_client.HTTPConnection(host, port, timeout=60, blocksize=65536) + c = http_client.HTTPConnection(host, port, timeout=300, blocksize=65536) elif scheme == "https": - c = http_client.HTTPSConnection(host, port, timeout=60, blocksize=65536) + c = http_client.HTTPSConnection(host, port, timeout=300, blocksize=65536) else: raise ValueError("unknown scheme '%s', need http or https" % scheme) c.putrequest(method, path) From f9a1eedaeadb818315bef8158902a47c863bbc65 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 25 Apr 2023 12:31:37 -0400 Subject: [PATCH 8/8] Make timeout optional, enable it only for integration tests. --- integration/conftest.py | 6 ++++++ newsfragments/4012.bugfix | 1 - newsfragments/4012.minor | 0 src/allmydata/scripts/common_http.py | 11 +++++++++-- 4 files changed, 15 insertions(+), 3 deletions(-) delete mode 100644 newsfragments/4012.bugfix create mode 100644 newsfragments/4012.minor diff --git a/integration/conftest.py b/integration/conftest.py index 879649588..d76b2a9c7 100644 --- a/integration/conftest.py +++ b/integration/conftest.py @@ -4,6 +4,7 @@ Ported to Python 3. from __future__ import annotations +import os import sys import shutil from time import sleep @@ -49,6 +50,11 @@ from .util import ( ) +# No reason for HTTP requests to take longer than two minutes in the +# integration tests. See allmydata/scripts/common_http.py for usage. +os.environ["__TAHOE_CLI_HTTP_TIMEOUT"] = "120" + + # pytest customization hooks def pytest_addoption(parser): diff --git a/newsfragments/4012.bugfix b/newsfragments/4012.bugfix deleted file mode 100644 index 24d5bb49a..000000000 --- a/newsfragments/4012.bugfix +++ /dev/null @@ -1 +0,0 @@ -The command-line tools now have a 300-second timeout on individual network reads/writes/connects; previously they could block forever in some situations. \ No newline at end of file diff --git a/newsfragments/4012.minor b/newsfragments/4012.minor new file mode 100644 index 000000000..e69de29bb diff --git a/src/allmydata/scripts/common_http.py b/src/allmydata/scripts/common_http.py index 46676b3f5..f138b9c07 100644 --- a/src/allmydata/scripts/common_http.py +++ b/src/allmydata/scripts/common_http.py @@ -53,10 +53,17 @@ def do_http(method, url, body=b""): assert body.seek assert body.read scheme, host, port, path = parse_url(url) + + # For testing purposes, allow setting a timeout on HTTP requests. If this + # ever become a user-facing feature, this should probably be a CLI option? + timeout = os.environ.get("__TAHOE_CLI_HTTP_TIMEOUT", None) + if timeout is not None: + timeout = float(timeout) + if scheme == "http": - c = http_client.HTTPConnection(host, port, timeout=300, blocksize=65536) + c = http_client.HTTPConnection(host, port, timeout=timeout, blocksize=65536) elif scheme == "https": - c = http_client.HTTPSConnection(host, port, timeout=300, blocksize=65536) + c = http_client.HTTPSConnection(host, port, timeout=timeout, blocksize=65536) else: raise ValueError("unknown scheme '%s', need http or https" % scheme) c.putrequest(method, path)