From 849f4ed2a57da1e2dd19b668dccba5967534224c Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 28 Jul 2023 11:14:09 -0400 Subject: [PATCH 01/10] More annotations. --- src/allmydata/storage/http_client.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index 765e94319..79f6cfa89 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -72,7 +72,7 @@ except ImportError: pass -def _encode_si(si): # type: (bytes) -> str +def _encode_si(si: bytes) -> str: """Encode the storage index into Unicode string.""" return str(si_b2a(si), "ascii") @@ -80,7 +80,7 @@ def _encode_si(si): # type: (bytes) -> str class ClientException(Exception): """An unexpected response code from the server.""" - def __init__(self, code, *additional_args): + def __init__(self, code: int, *additional_args): Exception.__init__(self, code, *additional_args) self.code = code @@ -93,7 +93,7 @@ register_exception_extractor(ClientException, lambda e: {"response_code": e.code # Tags are of the form #6.nnn, where the number is documented at # https://www.iana.org/assignments/cbor-tags/cbor-tags.xhtml. Notably, #6.258 # indicates a set. -_SCHEMAS = { +_SCHEMAS : Mapping[str,Schema] = { "get_version": Schema( # Note that the single-quoted (`'`) string keys in this schema # represent *byte* strings - per the CDDL specification. Text strings @@ -155,7 +155,7 @@ class _LengthLimitedCollector: timeout_on_silence: IDelayedCall f: BytesIO = field(factory=BytesIO) - def __call__(self, data: bytes): + def __call__(self, data: bytes) -> None: self.timeout_on_silence.reset(60) self.remaining_length -= len(data) if self.remaining_length < 0: @@ -164,7 +164,7 @@ class _LengthLimitedCollector: def limited_content( - response, + response: IResponse, clock: IReactorTime, max_length: int = 30 * 1024 * 1024, ) -> Deferred[BinaryIO]: @@ -300,11 +300,11 @@ class _StorageClientHTTPSPolicy: expected_spki_hash: bytes # IPolicyForHTTPS - def creatorForNetloc(self, hostname, port): + def creatorForNetloc(self, hostname: str, port: int) -> _StorageClientHTTPSPolicy: return self # IOpenSSLClientConnectionCreator - def clientConnectionForTLS(self, tlsProtocol): + def clientConnectionForTLS(self, tlsProtocol: object) -> SSL.Connection: return SSL.Connection( _TLSContextFactory(self.expected_spki_hash).getContext(), None ) @@ -344,7 +344,7 @@ class StorageClientFactory: cls.TEST_MODE_REGISTER_HTTP_POOL = callback @classmethod - def stop_test_mode(cls): + def stop_test_mode(cls) -> None: """Stop testing mode.""" cls.TEST_MODE_REGISTER_HTTP_POOL = None @@ -437,7 +437,7 @@ class StorageClient(object): """Get a URL relative to the base URL.""" return self._base_url.click(path) - def _get_headers(self, headers): # type: (Optional[Headers]) -> Headers + def _get_headers(self, headers: Optional[Headers]) -> Headers: """Return the basic headers to be used by default.""" if headers is None: headers = Headers() @@ -565,7 +565,7 @@ class StorageClient(object): ).read() raise ClientException(response.code, response.phrase, data) - def shutdown(self) -> Deferred: + def shutdown(self) -> Deferred[object]: """Shutdown any connections.""" return self._pool.closeCachedConnections() From 2b7f3d1707b6e91ea8e517bcfa3a6cf892d1715e Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 28 Jul 2023 11:28:13 -0400 Subject: [PATCH 02/10] Add type annotations to `_authorization_decorator`. --- src/allmydata/storage/http_server.py | 44 +++++++++++++++++++------ src/allmydata/test/test_storage_http.py | 3 +- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index 3ff98e933..78ed1a974 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -4,7 +4,8 @@ HTTP server for storage. from __future__ import annotations -from typing import Any, Callable, Union, cast, Optional +from typing import Any, Callable, Union, cast, Optional, TypeVar, Sequence +from typing_extensions import ParamSpec, Concatenate from functools import wraps from base64 import b64decode import binascii @@ -27,6 +28,7 @@ from twisted.internet.defer import Deferred from twisted.internet.ssl import CertificateOptions, Certificate, PrivateCertificate from twisted.internet.interfaces import IReactorFromThreads from twisted.web.server import Site, Request +from twisted.web.iweb import IRequest from twisted.protocols.tls import TLSMemoryBIOFactory from twisted.python.filepath import FilePath @@ -68,7 +70,7 @@ class ClientSecretsException(Exception): def _extract_secrets( - header_values: list[str], required_secrets: set[Secrets] + header_values: Sequence[str], required_secrets: set[Secrets] ) -> dict[Secrets, bytes]: """ Given list of values of ``X-Tahoe-Authorization`` headers, and required @@ -102,18 +104,32 @@ def _extract_secrets( return result -def _authorization_decorator(required_secrets): +P = ParamSpec("P") +T = TypeVar("T") + + +def _authorization_decorator( + required_secrets: set[Secrets], +) -> Callable[ + [Callable[Concatenate[BaseApp, Request, dict[Secrets, bytes], P], T]], + Callable[Concatenate[BaseApp, Request, P], T], +]: """ 1. Check the ``Authorization`` header matches server swissnum. 2. Extract ``X-Tahoe-Authorization`` headers and pass them in. 3. Log the request and response. """ - def decorator(f): + def decorator( + f: Callable[Concatenate[BaseApp, Request, dict[Secrets, bytes], P], T] + ) -> Callable[Concatenate[BaseApp, Request, P], T]: @wraps(f) - def route(self, request, *args, **kwargs): - # Don't set text/html content type by default: - request.defaultContentType = None + def route( + self: BaseApp, request: Request, *args: P.args, **kwargs: P.kwargs + ) -> T: + # Don't set text/html content type by default. + # None is actually supported, see https://github.com/twisted/twisted/issues/11902 + request.defaultContentType = None # type: ignore[assignment] with start_action( action_type="allmydata:storage:http-server:handle-request", @@ -584,7 +600,13 @@ async def read_encoded( return cbor2.load(request.content) -class HTTPServer(object): +class BaseApp: + """Base class for ``HTTPServer`` and testing equivalent.""" + + _swissnum: bytes + + +class HTTPServer(BaseApp): """ A HTTP interface to the storage server. """ @@ -641,7 +663,6 @@ class HTTPServer(object): # https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3861 raise _HTTPError(http.NOT_ACCEPTABLE) - ##### Generic APIs ##### @_authorized_route(_app, set(), "/storage/v1/version", methods=["GET"]) @@ -874,7 +895,10 @@ class HTTPServer(object): async def mutable_read_test_write(self, request, authorization, storage_index): """Read/test/write combined operation for mutables.""" rtw_request = await read_encoded( - self._reactor, request, _SCHEMAS["mutable_read_test_write"], max_size=2**48 + self._reactor, + request, + _SCHEMAS["mutable_read_test_write"], + max_size=2**48, ) secrets = ( authorization[Secrets.WRITE_ENABLER], diff --git a/src/allmydata/test/test_storage_http.py b/src/allmydata/test/test_storage_http.py index 48ca072bc..1a334034d 100644 --- a/src/allmydata/test/test_storage_http.py +++ b/src/allmydata/test/test_storage_http.py @@ -62,6 +62,7 @@ from ..storage.http_server import ( _add_error_handling, read_encoded, _SCHEMAS as SERVER_SCHEMAS, + BaseApp, ) from ..storage.http_client import ( StorageClient, @@ -257,7 +258,7 @@ def gen_bytes(length: int) -> bytes: return result -class TestApp(object): +class TestApp(BaseApp): """HTTP API for testing purposes.""" clock: IReactorTime From 919e6b339d0eaf7019f231ad916b2f7ac25cef48 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 28 Jul 2023 12:58:22 -0400 Subject: [PATCH 03/10] Add type annotation to _authorized_route --- src/allmydata/storage/http_server.py | 77 ++++++++++++++++++------- src/allmydata/test/test_storage_http.py | 2 +- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index 78ed1a974..7ceb8328c 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -4,7 +4,7 @@ HTTP server for storage. from __future__ import annotations -from typing import Any, Callable, Union, cast, Optional, TypeVar, Sequence +from typing import Any, Callable, Union, cast, Optional, TypeVar, Sequence, Protocol from typing_extensions import ParamSpec, Concatenate from functools import wraps from base64 import b64decode @@ -16,7 +16,7 @@ import mmap from eliot import start_action from cryptography.x509 import Certificate as CryptoCertificate from zope.interface import implementer -from klein import Klein +from klein import Klein, KleinRenderable from twisted.web import http from twisted.internet.interfaces import ( IListeningPort, @@ -104,15 +104,23 @@ def _extract_secrets( return result +class BaseApp(Protocol): + """Protocol for ``HTTPServer`` and testing equivalent.""" + + _swissnum: bytes + + P = ParamSpec("P") T = TypeVar("T") +SecretsDict = dict[Secrets, bytes] +App = TypeVar("App", bound=BaseApp) def _authorization_decorator( required_secrets: set[Secrets], ) -> Callable[ - [Callable[Concatenate[BaseApp, Request, dict[Secrets, bytes], P], T]], - Callable[Concatenate[BaseApp, Request, P], T], + [Callable[Concatenate[App, Request, SecretsDict, P], T]], + Callable[Concatenate[App, Request, P], T], ]: """ 1. Check the ``Authorization`` header matches server swissnum. @@ -121,11 +129,14 @@ def _authorization_decorator( """ def decorator( - f: Callable[Concatenate[BaseApp, Request, dict[Secrets, bytes], P], T] - ) -> Callable[Concatenate[BaseApp, Request, P], T]: + f: Callable[Concatenate[App, Request, SecretsDict, P], T] + ) -> Callable[Concatenate[App, Request, P], T]: @wraps(f) def route( - self: BaseApp, request: Request, *args: P.args, **kwargs: P.kwargs + self: App, + request: Request, + *args: P.args, + **kwargs: P.kwargs, ) -> T: # Don't set text/html content type by default. # None is actually supported, see https://github.com/twisted/twisted/issues/11902 @@ -179,7 +190,22 @@ def _authorization_decorator( return decorator -def _authorized_route(app, required_secrets, *route_args, **route_kwargs): +def _authorized_route( + klein_app: Klein, + required_secrets: set[Secrets], + url: str, + *route_args: Any, + branch: bool = False, + **route_kwargs: Any, +) -> Callable[ + [ + Callable[ + Concatenate[App, Request, SecretsDict, P], + KleinRenderable, + ] + ], + Callable[..., KleinRenderable], +]: """ Like Klein's @route, but with additional support for checking the ``Authorization`` header as well as ``X-Tahoe-Authorization`` headers. The @@ -189,12 +215,23 @@ def _authorized_route(app, required_secrets, *route_args, **route_kwargs): :param required_secrets: Set of required ``Secret`` types. """ - def decorator(f): - @app.route(*route_args, **route_kwargs) + def decorator( + f: Callable[ + Concatenate[App, Request, SecretsDict, P], + KleinRenderable, + ] + ) -> Callable[..., KleinRenderable]: + @klein_app.route(url, *route_args, branch=branch, **route_kwargs) # type: ignore[arg-type] @_authorization_decorator(required_secrets) @wraps(f) - def handle_route(*args, **kwargs): - return f(*args, **kwargs) + def handle_route( + app: App, + request: Request, + secrets: SecretsDict, + *args: P.args, + **kwargs: P.kwargs, + ) -> KleinRenderable: + return f(app, request, secrets, *args, **kwargs) return handle_route @@ -367,7 +404,7 @@ class _ReadAllProducer: start: int = field(default=0) @classmethod - def produce_to(cls, request: Request, read_data: ReadData) -> Deferred: + def produce_to(cls, request: Request, read_data: ReadData) -> Deferred[bytes]: """ Create and register the producer, returning ``Deferred`` that should be returned from a HTTP server endpoint. @@ -600,12 +637,6 @@ async def read_encoded( return cbor2.load(request.content) -class BaseApp: - """Base class for ``HTTPServer`` and testing equivalent.""" - - _swissnum: bytes - - class HTTPServer(BaseApp): """ A HTTP interface to the storage server. @@ -637,7 +668,7 @@ class HTTPServer(BaseApp): """Return twisted.web ``Resource`` for this object.""" return self._app.resource() - def _send_encoded(self, request, data): + def _send_encoded(self, request: Request, data: object) -> Deferred[bytes]: """ Return encoded data suitable for writing as the HTTP body response, by default using CBOR. @@ -666,7 +697,7 @@ class HTTPServer(BaseApp): ##### Generic APIs ##### @_authorized_route(_app, set(), "/storage/v1/version", methods=["GET"]) - def version(self, request, authorization): + def version(self, request: Request, authorization: SecretsDict) -> KleinRenderable: """Return version information.""" return self._send_encoded(request, self._get_version()) @@ -698,7 +729,9 @@ class HTTPServer(BaseApp): methods=["POST"], ) @async_to_deferred - async def allocate_buckets(self, request, authorization, storage_index): + async def allocate_buckets( + self, request: Request, authorization: SecretsDict, storage_index: bytes + ) -> KleinRenderable: """Allocate buckets.""" upload_secret = authorization[Secrets.UPLOAD] # It's just a list of up to ~256 shares, shouldn't use many bytes. diff --git a/src/allmydata/test/test_storage_http.py b/src/allmydata/test/test_storage_http.py index 1a334034d..e7b8059ee 100644 --- a/src/allmydata/test/test_storage_http.py +++ b/src/allmydata/test/test_storage_http.py @@ -266,7 +266,7 @@ class TestApp(BaseApp): _add_error_handling(_app) _swissnum = SWISSNUM_FOR_TEST # Match what the test client is using - @_authorized_route(_app, {}, "/noop", methods=["GET"]) + @_authorized_route(_app, set(), "/noop", methods=["GET"]) def noop(self, request, authorization): return "noop" From d669099a3515b4ff8c1524bc43f8fcd74782560a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 28 Jul 2023 13:28:02 -0400 Subject: [PATCH 04/10] Add more type annotations. --- src/allmydata/storage/http_server.py | 69 ++++++++++++++++++++++------ 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index 7ceb8328c..0cf3d25f4 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -770,7 +770,13 @@ class HTTPServer(BaseApp): "/storage/v1/immutable///abort", methods=["PUT"], ) - def abort_share_upload(self, request, authorization, storage_index, share_number): + def abort_share_upload( + self, + request: Request, + authorization: SecretsDict, + storage_index: bytes, + share_number: int, + ) -> KleinRenderable: """Abort an in-progress immutable share upload.""" try: bucket = self._uploads.get_write_bucket( @@ -801,7 +807,13 @@ class HTTPServer(BaseApp): "/storage/v1/immutable//", methods=["PATCH"], ) - def write_share_data(self, request, authorization, storage_index, share_number): + def write_share_data( + self, + request: Request, + authorization: SecretsDict, + storage_index: bytes, + share_number: int, + ) -> KleinRenderable: """Write data to an in-progress immutable upload.""" content_range = parse_content_range_header(request.getHeader("content-range")) if content_range is None or content_range.units != "bytes": @@ -811,14 +823,17 @@ class HTTPServer(BaseApp): bucket = self._uploads.get_write_bucket( storage_index, share_number, authorization[Secrets.UPLOAD] ) - offset = content_range.start - remaining = content_range.stop - content_range.start + offset = content_range.start or 0 + # We don't support an unspecified stop for the range: + assert content_range.stop is not None + # Missing body makes no sense: + assert request.content is not None + remaining = content_range.stop - offset finished = False while remaining > 0: data = request.content.read(min(remaining, 65536)) assert data, "uploaded data length doesn't match range" - try: finished = bucket.write(offset, data) except ConflictingWriteError: @@ -844,7 +859,9 @@ class HTTPServer(BaseApp): "/storage/v1/immutable//shares", methods=["GET"], ) - def list_shares(self, request, authorization, storage_index): + def list_shares( + self, request: Request, authorization: SecretsDict, storage_index: bytes + ) -> KleinRenderable: """ List shares for the given storage index. """ @@ -857,7 +874,13 @@ class HTTPServer(BaseApp): "/storage/v1/immutable//", methods=["GET"], ) - def read_share_chunk(self, request, authorization, storage_index, share_number): + def read_share_chunk( + self, + request: Request, + authorization: SecretsDict, + storage_index: bytes, + share_number: int, + ) -> KleinRenderable: """Read a chunk for an already uploaded immutable.""" request.setHeader("content-type", "application/octet-stream") try: @@ -874,7 +897,9 @@ class HTTPServer(BaseApp): "/storage/v1/lease/", methods=["PUT"], ) - def add_or_renew_lease(self, request, authorization, storage_index): + def add_or_renew_lease( + self, request: Request, authorization: SecretsDict, storage_index: bytes + ) -> KleinRenderable: """Update the lease for an immutable or mutable share.""" if not list(self._storage_server.get_shares(storage_index)): raise _HTTPError(http.NOT_FOUND) @@ -897,8 +922,12 @@ class HTTPServer(BaseApp): ) @async_to_deferred async def advise_corrupt_share_immutable( - self, request, authorization, storage_index, share_number - ): + self, + request: Request, + authorization: SecretsDict, + storage_index: bytes, + share_number: int, + ) -> KleinRenderable: """Indicate that given share is corrupt, with a text reason.""" try: bucket = self._storage_server.get_buckets(storage_index)[share_number] @@ -925,7 +954,9 @@ class HTTPServer(BaseApp): methods=["POST"], ) @async_to_deferred - async def mutable_read_test_write(self, request, authorization, storage_index): + async def mutable_read_test_write( + self, request: Request, authorization: SecretsDict, storage_index: bytes + ) -> KleinRenderable: """Read/test/write combined operation for mutables.""" rtw_request = await read_encoded( self._reactor, @@ -967,7 +998,13 @@ class HTTPServer(BaseApp): "/storage/v1/mutable//", methods=["GET"], ) - def read_mutable_chunk(self, request, authorization, storage_index, share_number): + def read_mutable_chunk( + self, + request: Request, + authorization: SecretsDict, + storage_index: bytes, + share_number: int, + ) -> KleinRenderable: """Read a chunk from a mutable.""" request.setHeader("content-type", "application/octet-stream") @@ -1007,8 +1044,12 @@ class HTTPServer(BaseApp): ) @async_to_deferred async def advise_corrupt_share_mutable( - self, request, authorization, storage_index, share_number - ): + self, + request: Request, + authorization: SecretsDict, + storage_index: bytes, + share_number: int, + ) -> KleinRenderable: """Indicate that given share is corrupt, with a text reason.""" if share_number not in { shnum for (shnum, _) in self._storage_server.get_shares(storage_index) From 0d0e32646fe305637d4cebedd8c9e4427db9fedd Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 28 Jul 2023 13:42:00 -0400 Subject: [PATCH 05/10] More type annotations. --- src/allmydata/storage/http_server.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index 0cf3d25f4..ce07d8f2e 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -31,6 +31,7 @@ from twisted.web.server import Site, Request from twisted.web.iweb import IRequest from twisted.protocols.tls import TLSMemoryBIOFactory from twisted.python.filepath import FilePath +from twisted.python.failure import Failure from attrs import define, field, Factory from werkzeug.http import ( @@ -287,7 +288,7 @@ class UploadsInProgress(object): except (KeyError, IndexError): raise _HTTPError(http.NOT_FOUND) - def remove_write_bucket(self, bucket: BucketWriter): + def remove_write_bucket(self, bucket: BucketWriter) -> None: """Stop tracking the given ``BucketWriter``.""" try: storage_index, share_number = self._bucketwriters.pop(bucket) @@ -303,7 +304,7 @@ class UploadsInProgress(object): def validate_upload_secret( self, storage_index: bytes, share_number: int, upload_secret: bytes - ): + ) -> None: """ Raise an unauthorized-HTTP-response exception if the given storage_index+share_number have a different upload secret than the @@ -325,7 +326,7 @@ class StorageIndexConverter(BaseConverter): regex = "[" + str(rfc3548_alphabet, "ascii") + "]{26}" - def to_python(self, value): + def to_python(self, value: str) -> bytes: try: return si_a2b(value.encode("ascii")) except (AssertionError, binascii.Error, ValueError): @@ -413,7 +414,7 @@ class _ReadAllProducer: request.registerProducer(producer, False) return producer.result - def resumeProducing(self): + def resumeProducing(self) -> None: data = self.read_data(self.start, 65536) if not data: self.request.unregisterProducer() @@ -424,10 +425,10 @@ class _ReadAllProducer: self.request.write(data) self.start += len(data) - def pauseProducing(self): + def pauseProducing(self) -> None: pass - def stopProducing(self): + def stopProducing(self) -> None: pass @@ -445,7 +446,7 @@ class _ReadRangeProducer: start: int remaining: int - def resumeProducing(self): + def resumeProducing(self) -> None: if self.result is None or self.request is None: return @@ -482,10 +483,10 @@ class _ReadRangeProducer: if self.remaining == 0: self.stopProducing() - def pauseProducing(self): + def pauseProducing(self) -> None: pass - def stopProducing(self): + def stopProducing(self) -> None: if self.request is not None: self.request.unregisterProducer() self.request = None @@ -564,12 +565,13 @@ def read_range( return d -def _add_error_handling(app: Klein): +def _add_error_handling(app: Klein) -> None: """Add exception handlers to a Klein app.""" @app.handle_errors(_HTTPError) - def _http_error(_, request, failure): + def _http_error(self: Any, request: IRequest, failure: Failure) -> KleinRenderable: """Handle ``_HTTPError`` exceptions.""" + assert isinstance(failure.value, _HTTPError) request.setResponseCode(failure.value.code) if failure.value.body is not None: return failure.value.body @@ -577,7 +579,9 @@ def _add_error_handling(app: Klein): return b"" @app.handle_errors(CDDLValidationError) - def _cddl_validation_error(_, request, failure): + def _cddl_validation_error( + self: Any, request: IRequest, failure: Failure + ) -> KleinRenderable: """Handle CDDL validation errors.""" request.setResponseCode(http.BAD_REQUEST) return str(failure.value).encode("utf-8") From 00b7e7e17862335edd08b0b38b28f30f64b8f993 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 28 Jul 2023 13:48:43 -0400 Subject: [PATCH 06/10] More type annotations. --- src/allmydata/storage/http_server.py | 11 ++++++++--- src/allmydata/test/test_storage_https.py | 6 ++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index ce07d8f2e..cf0e6dbb4 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -17,11 +17,13 @@ from eliot import start_action from cryptography.x509 import Certificate as CryptoCertificate from zope.interface import implementer from klein import Klein, KleinRenderable +from klein.resource import KleinResource from twisted.web import http from twisted.internet.interfaces import ( IListeningPort, IStreamServerEndpoint, IPullProducer, + IProtocolFactory, ) from twisted.internet.address import IPv4Address, IPv6Address from twisted.internet.defer import Deferred @@ -668,7 +670,7 @@ class HTTPServer(BaseApp): self._uploads.remove_write_bucket ) - def get_resource(self): + def get_resource(self) -> KleinResource: """Return twisted.web ``Resource`` for this object.""" return self._app.resource() @@ -1085,7 +1087,10 @@ class _TLSEndpointWrapper(object): @classmethod def from_paths( - cls, endpoint, private_key_path: FilePath, cert_path: FilePath + cls: type[_TLSEndpointWrapper], + endpoint: IStreamServerEndpoint, + private_key_path: FilePath, + cert_path: FilePath, ) -> "_TLSEndpointWrapper": """ Create an endpoint with the given private key and certificate paths on @@ -1100,7 +1105,7 @@ class _TLSEndpointWrapper(object): ) return cls(endpoint=endpoint, context_factory=certificate_options) - def listen(self, factory): + def listen(self, factory: IProtocolFactory) -> Deferred[IListeningPort]: return self.endpoint.listen( TLSMemoryBIOFactory(self.context_factory, False, factory) ) diff --git a/src/allmydata/test/test_storage_https.py b/src/allmydata/test/test_storage_https.py index a11b0eed5..0e0bbcc95 100644 --- a/src/allmydata/test/test_storage_https.py +++ b/src/allmydata/test/test_storage_https.py @@ -109,9 +109,11 @@ class PinningHTTPSValidation(AsyncTestCase): root.isLeaf = True listening_port = await endpoint.listen(Site(root)) try: - yield f"https://127.0.0.1:{listening_port.getHost().port}/" + yield f"https://127.0.0.1:{listening_port.getHost().port}/" # type: ignore[attr-defined] finally: - await listening_port.stopListening() + result = listening_port.stopListening() + if result is not None: + await result def request(self, url: str, expected_certificate: x509.Certificate): """ From f8e9631f532da65c46ef3d04039b34469b6ab11a Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 28 Jul 2023 13:49:21 -0400 Subject: [PATCH 07/10] News fragment. --- newsfragments/4052.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/4052.minor diff --git a/newsfragments/4052.minor b/newsfragments/4052.minor new file mode 100644 index 000000000..e69de29bb From 176fac7360f3797cb637d8fd9d90ba05c3fbe548 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Fri, 28 Jul 2023 14:20:05 -0400 Subject: [PATCH 08/10] Work in Python 3.8. --- src/allmydata/storage/http_server.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/allmydata/storage/http_server.py b/src/allmydata/storage/http_server.py index cf0e6dbb4..66b0dd6de 100644 --- a/src/allmydata/storage/http_server.py +++ b/src/allmydata/storage/http_server.py @@ -4,7 +4,17 @@ HTTP server for storage. from __future__ import annotations -from typing import Any, Callable, Union, cast, Optional, TypeVar, Sequence, Protocol +from typing import ( + Any, + Callable, + Union, + cast, + Optional, + TypeVar, + Sequence, + Protocol, + Dict, +) from typing_extensions import ParamSpec, Concatenate from functools import wraps from base64 import b64decode @@ -115,7 +125,7 @@ class BaseApp(Protocol): P = ParamSpec("P") T = TypeVar("T") -SecretsDict = dict[Secrets, bytes] +SecretsDict = Dict[Secrets, bytes] App = TypeVar("App", bound=BaseApp) From e545ab4a8022c52ee3a450ab501eedc470491d50 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 1 Aug 2023 15:31:38 -0400 Subject: [PATCH 09/10] More accurate type --- src/allmydata/storage/http_client.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index 79f6cfa89..75b6eab22 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -41,6 +41,7 @@ from twisted.internet.interfaces import ( IDelayedCall, ) from twisted.internet.ssl import CertificateOptions +from twisted.protocols.tls import TLSMemoryBIOProtocol from twisted.web.client import Agent, HTTPConnectionPool from zope.interface import implementer from hyperlink import DecodedURL @@ -304,7 +305,7 @@ class _StorageClientHTTPSPolicy: return self # IOpenSSLClientConnectionCreator - def clientConnectionForTLS(self, tlsProtocol: object) -> SSL.Connection: + def clientConnectionForTLS(self, tlsProtocol: TLSMemoryBIOProtocol) -> SSL.Connection: return SSL.Connection( _TLSContextFactory(self.expected_spki_hash).getContext(), None ) From 009f063067a156ddab95bb3f554c43244cc05fc1 Mon Sep 17 00:00:00 2001 From: Itamar Turner-Trauring Date: Tue, 1 Aug 2023 15:34:40 -0400 Subject: [PATCH 10/10] Stricter type checking --- src/allmydata/storage/http_client.py | 14 ++++++++++---- src/allmydata/storage_client.py | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/allmydata/storage/http_client.py b/src/allmydata/storage/http_client.py index 75b6eab22..b508c07fd 100644 --- a/src/allmydata/storage/http_client.py +++ b/src/allmydata/storage/http_client.py @@ -81,9 +81,13 @@ def _encode_si(si: bytes) -> str: class ClientException(Exception): """An unexpected response code from the server.""" - def __init__(self, code: int, *additional_args): - Exception.__init__(self, code, *additional_args) + def __init__( + self, code: int, message: Optional[str] = None, body: Optional[bytes] = None + ): + Exception.__init__(self, code, message, body) self.code = code + self.message = message + self.body = body register_exception_extractor(ClientException, lambda e: {"response_code": e.code}) @@ -94,7 +98,7 @@ register_exception_extractor(ClientException, lambda e: {"response_code": e.code # Tags are of the form #6.nnn, where the number is documented at # https://www.iana.org/assignments/cbor-tags/cbor-tags.xhtml. Notably, #6.258 # indicates a set. -_SCHEMAS : Mapping[str,Schema] = { +_SCHEMAS: Mapping[str, Schema] = { "get_version": Schema( # Note that the single-quoted (`'`) string keys in this schema # represent *byte* strings - per the CDDL specification. Text strings @@ -305,7 +309,9 @@ class _StorageClientHTTPSPolicy: return self # IOpenSSLClientConnectionCreator - def clientConnectionForTLS(self, tlsProtocol: TLSMemoryBIOProtocol) -> SSL.Connection: + def clientConnectionForTLS( + self, tlsProtocol: TLSMemoryBIOProtocol + ) -> SSL.Connection: return SSL.Connection( _TLSContextFactory(self.expected_spki_hash).getContext(), None ) diff --git a/src/allmydata/storage_client.py b/src/allmydata/storage_client.py index 4efc845b4..69ae2c22b 100644 --- a/src/allmydata/storage_client.py +++ b/src/allmydata/storage_client.py @@ -1428,7 +1428,7 @@ class _FakeRemoteReference(object): result = yield getattr(self.local_object, action)(*args, **kwargs) defer.returnValue(result) except HTTPClientException as e: - raise RemoteException(e.args) + raise RemoteException((e.code, e.message, e.body)) @attr.s