From d7fe25f7c7ffc1e8eb9da45755085c5dd04c25d8 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 29 Nov 2022 10:49:20 -0500 Subject: [PATCH 1/8] Correct the assertion about how "not found" should be handled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavior verified visually against a live client node: ``` ❯ curl -v 'http://localhost:3456/uri/URI:CHK:cmtcxq7hwxvfxan34yiev6ivhy:qvcekmjtoetdcw4kmi7b3rtblvgx7544crnwaqtiewemdliqsokq:1:1:1' * Trying 127.0.0.1:3456... * Connected to localhost (127.0.0.1) port 3456 (#0) > GET /uri/URI:CHK:cmtcxq7hwxvfxan34yiev6ivhy:qvcekmjtoetdcw4kmi7b3rtblvgx7544crnwaqtiewemdliqsokq:1:1:1 HTTP/1.1 > Host: localhost:3456 > User-Agent: curl/7.83.1 > Accept: */* > * Mark bundle as not supporting multiuse < HTTP/1.1 410 Gone < X-Frame-Options: DENY < Referrer-Policy: no-referrer < Server: TwistedWeb/22.10.0 < Date: Tue, 29 Nov 2022 15:39:47 GMT < Content-Type: text/plain;charset=utf-8 < Accept-Ranges: bytes < Content-Length: 294 < ETag: ui2tnwl5lltj5clzpyff42jdce- < NoSharesError: no shares could be found. Zero shares usually indicates a corrupt URI, or that no servers were connected, but it might also indicate severe corruption. You should perform a filecheck on this object to learn more. The full error message is: * Connection #0 to host localhost left intact no shares (need 1). Last failure: None ``` --- src/allmydata/test/test_testing.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/allmydata/test/test_testing.py b/src/allmydata/test/test_testing.py index 3715d1aca..07bebb7a1 100644 --- a/src/allmydata/test/test_testing.py +++ b/src/allmydata/test/test_testing.py @@ -9,18 +9,7 @@ """ Tests for the allmydata.testing helpers - -Ported to Python 3. - """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -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 from twisted.internet.defer import ( inlineCallbacks, @@ -56,10 +45,12 @@ from testtools.matchers import ( IsInstance, MatchesStructure, AfterPreprocessing, + Contains, ) from testtools.twistedsupport import ( succeeded, ) +from twisted.web.http import GONE class FakeWebTest(SyncTestCase): @@ -144,7 +135,8 @@ class FakeWebTest(SyncTestCase): def test_download_missing(self): """ - Error if we download a capability that doesn't exist + The response to a request to download a capability that doesn't exist + is 410 (GONE). """ http_client = create_tahoe_treq_client() @@ -157,7 +149,11 @@ class FakeWebTest(SyncTestCase): resp, succeeded( MatchesStructure( - code=Equals(500) + code=Equals(GONE), + content=AfterPreprocessing( + lambda m: m(), + succeeded(Contains(b"No data for")), + ), ) ) ) From 02aeb68f1731c58e146ccefd1d8ea99ad364b73d Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 29 Nov 2022 10:51:07 -0500 Subject: [PATCH 2/8] Take care with str vs bytes in the implementation Also replace the intentional BAD_REQUEST with GONE for this case. --- src/allmydata/testing/web.py | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index bb858b555..6538dc3a4 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -6,22 +6,13 @@ # This file is part of Tahoe-LAFS. # # See the docs/about.rst file for licensing information. -"""Test-helpers for clients that use the WebUI. - -Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -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 - +Test-helpers for clients that use the WebUI. +""" import hashlib +from typing import Dict import attr @@ -147,7 +138,7 @@ class _FakeTahoeUriHandler(Resource, object): isLeaf = True - data = attr.ib(default=attr.Factory(dict)) + data: Dict[bytes, bytes] = attr.ib(default=attr.Factory(dict)) capability_generators = attr.ib(default=attr.Factory(dict)) def _generate_capability(self, kind): @@ -209,7 +200,7 @@ class _FakeTahoeUriHandler(Resource, object): capability = None for arg, value in uri.query: if arg == u"uri": - capability = value + capability = value.encode("utf-8") # it's legal to use the form "/uri/" if capability is None and request.postpath and request.postpath[0]: capability = request.postpath[0] @@ -221,10 +212,9 @@ class _FakeTahoeUriHandler(Resource, object): # the user gave us a capability; if our Grid doesn't have any # data for it, that's an error. - capability = capability.encode('ascii') if capability not in self.data: - request.setResponseCode(http.BAD_REQUEST) - return u"No data for '{}'".format(capability.decode('ascii')) + request.setResponseCode(http.GONE) + return u"No data for '{}'".format(capability.decode('ascii')).encode("utf-8") return self.data[capability] From 6c0e5f5807cf72d38b9fb4a9461d0fd085920360 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Tue, 29 Nov 2022 10:52:02 -0500 Subject: [PATCH 3/8] news fragment --- newsfragments/3874.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3874.minor diff --git a/newsfragments/3874.minor b/newsfragments/3874.minor new file mode 100644 index 000000000..e69de29bb From 9619e286f4e0f50c20975f5789418eed09f4e350 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 2 Dec 2022 08:16:02 -0500 Subject: [PATCH 4/8] Switch the web testing double to BytesKeyDict This will catch more str/bytes errors by default than `dict` --- src/allmydata/testing/web.py | 3 ++- src/allmydata/util/dictutil.py | 34 +++++++--------------------------- 2 files changed, 9 insertions(+), 28 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 6538dc3a4..a687e5480 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -45,6 +45,7 @@ import allmydata.uri from allmydata.util import ( base32, ) +from ..util.dictutil import BytesKeyDict __all__ = ( @@ -138,7 +139,7 @@ class _FakeTahoeUriHandler(Resource, object): isLeaf = True - data: Dict[bytes, bytes] = attr.ib(default=attr.Factory(dict)) + data: BytesKeyDict[bytes, bytes] = attr.ib(default=attr.Factory(BytesKeyDict)) capability_generators = attr.ib(default=attr.Factory(dict)) def _generate_capability(self, kind): diff --git a/src/allmydata/util/dictutil.py b/src/allmydata/util/dictutil.py index 5971d26f6..0a7df0a38 100644 --- a/src/allmydata/util/dictutil.py +++ b/src/allmydata/util/dictutil.py @@ -1,21 +1,6 @@ """ Tools to mess with dicts. - -Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals - -from future.utils import PY2 -if PY2: - # IMPORTANT: We deliberately don't import dict. The issue is that we're - # subclassing dict, so we'd end up exposing Python 3 dict APIs to lots of - # code that doesn't support it. - from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, list, object, range, str, max, min # noqa: F401 -from six import ensure_str - class DictOfSets(dict): def add(self, key, value): @@ -104,7 +89,7 @@ def _make_enforcing_override(K, method_name): raise TypeError("{} must be of type {}".format( repr(key), self.KEY_TYPE)) return getattr(dict, method_name)(self, key, *args, **kwargs) - f.__name__ = ensure_str(method_name) + f.__name__ = method_name setattr(K, method_name, f) for _method_name in ["__setitem__", "__getitem__", "setdefault", "get", @@ -113,18 +98,13 @@ for _method_name in ["__setitem__", "__getitem__", "setdefault", "get", del _method_name -if PY2: - # No need for enforcement, can use either bytes or unicode as keys and it's - # fine. - BytesKeyDict = UnicodeKeyDict = dict -else: - class BytesKeyDict(_TypedKeyDict): - """Keys should be bytes.""" +class BytesKeyDict(_TypedKeyDict): + """Keys should be bytes.""" - KEY_TYPE = bytes + KEY_TYPE = bytes - class UnicodeKeyDict(_TypedKeyDict): - """Keys should be unicode strings.""" +class UnicodeKeyDict(_TypedKeyDict): + """Keys should be unicode strings.""" - KEY_TYPE = str + KEY_TYPE = str From a84b278ecdaf3263030ad9817961d70cdfdfd741 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 2 Dec 2022 08:26:15 -0500 Subject: [PATCH 5/8] support older pythons --- src/allmydata/testing/web.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index a687e5480..be7878d57 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -11,6 +11,8 @@ Test-helpers for clients that use the WebUI. """ +from __future__ import annotations + import hashlib from typing import Dict From b40d882fceb20fa102ca32540819555a08b2fdf1 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 2 Dec 2022 08:28:22 -0500 Subject: [PATCH 6/8] remove unused import --- src/allmydata/testing/web.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index be7878d57..4af2603a8 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -14,7 +14,6 @@ Test-helpers for clients that use the WebUI. from __future__ import annotations import hashlib -from typing import Dict import attr From c6cc3708f45c9e581a719852d4f1082528941701 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 2 Dec 2022 08:38:46 -0500 Subject: [PATCH 7/8] Fixup the annotations a bit --- src/allmydata/testing/web.py | 2 +- src/allmydata/util/dictutil.py | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 4af2603a8..72ecd7161 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -140,7 +140,7 @@ class _FakeTahoeUriHandler(Resource, object): isLeaf = True - data: BytesKeyDict[bytes, bytes] = attr.ib(default=attr.Factory(BytesKeyDict)) + data: BytesKeyDict[bytes] = attr.ib(default=attr.Factory(BytesKeyDict)) capability_generators = attr.ib(default=attr.Factory(dict)) def _generate_capability(self, kind): diff --git a/src/allmydata/util/dictutil.py b/src/allmydata/util/dictutil.py index 0a7df0a38..c436ab963 100644 --- a/src/allmydata/util/dictutil.py +++ b/src/allmydata/util/dictutil.py @@ -2,6 +2,10 @@ Tools to mess with dicts. """ +from __future__ import annotations + +from typing import TypeVar, Type + class DictOfSets(dict): def add(self, key, value): if key in self: @@ -64,7 +68,10 @@ class AuxValueDict(dict): self.auxilliary[key] = auxilliary -class _TypedKeyDict(dict): +K = TypeVar("K") +V = TypeVar("V") + +class _TypedKeyDict(dict[K, V]): """Dictionary that enforces key type. Doesn't override everything, but probably good enough to catch most @@ -73,7 +80,7 @@ class _TypedKeyDict(dict): Subclass and override KEY_TYPE. """ - KEY_TYPE = object + KEY_TYPE: Type[K] def __init__(self, *args, **kwargs): dict.__init__(self, *args, **kwargs) @@ -98,13 +105,13 @@ for _method_name in ["__setitem__", "__getitem__", "setdefault", "get", del _method_name -class BytesKeyDict(_TypedKeyDict): +class BytesKeyDict(_TypedKeyDict[bytes, V]): """Keys should be bytes.""" KEY_TYPE = bytes -class UnicodeKeyDict(_TypedKeyDict): +class UnicodeKeyDict(_TypedKeyDict[str, V]): """Keys should be unicode strings.""" KEY_TYPE = str From c542b8463707cd5720822de260f79b61c7582994 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 2 Dec 2022 08:47:07 -0500 Subject: [PATCH 8/8] remove the annotations everything is broken on older pythons --- src/allmydata/testing/web.py | 2 +- src/allmydata/util/dictutil.py | 15 ++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/allmydata/testing/web.py b/src/allmydata/testing/web.py index 72ecd7161..4f68b3774 100644 --- a/src/allmydata/testing/web.py +++ b/src/allmydata/testing/web.py @@ -140,7 +140,7 @@ class _FakeTahoeUriHandler(Resource, object): isLeaf = True - data: BytesKeyDict[bytes] = attr.ib(default=attr.Factory(BytesKeyDict)) + data: BytesKeyDict = attr.ib(default=attr.Factory(BytesKeyDict)) capability_generators = attr.ib(default=attr.Factory(dict)) def _generate_capability(self, kind): diff --git a/src/allmydata/util/dictutil.py b/src/allmydata/util/dictutil.py index c436ab963..0a7df0a38 100644 --- a/src/allmydata/util/dictutil.py +++ b/src/allmydata/util/dictutil.py @@ -2,10 +2,6 @@ Tools to mess with dicts. """ -from __future__ import annotations - -from typing import TypeVar, Type - class DictOfSets(dict): def add(self, key, value): if key in self: @@ -68,10 +64,7 @@ class AuxValueDict(dict): self.auxilliary[key] = auxilliary -K = TypeVar("K") -V = TypeVar("V") - -class _TypedKeyDict(dict[K, V]): +class _TypedKeyDict(dict): """Dictionary that enforces key type. Doesn't override everything, but probably good enough to catch most @@ -80,7 +73,7 @@ class _TypedKeyDict(dict[K, V]): Subclass and override KEY_TYPE. """ - KEY_TYPE: Type[K] + KEY_TYPE = object def __init__(self, *args, **kwargs): dict.__init__(self, *args, **kwargs) @@ -105,13 +98,13 @@ for _method_name in ["__setitem__", "__getitem__", "setdefault", "get", del _method_name -class BytesKeyDict(_TypedKeyDict[bytes, V]): +class BytesKeyDict(_TypedKeyDict): """Keys should be bytes.""" KEY_TYPE = bytes -class UnicodeKeyDict(_TypedKeyDict[str, V]): +class UnicodeKeyDict(_TypedKeyDict): """Keys should be unicode strings.""" KEY_TYPE = str