From 66a4af8d9627719a875c405c8c0f49b0056811b2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 10 Jul 2020 14:30:08 -0400 Subject: Do not use canonicaljson to magically handle decoding bytes from JSON. (#7802) --- synapse/api/errors.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'synapse/api/errors.py') diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 5305038c21..d5d4522336 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -15,13 +15,11 @@ # limitations under the License. """Contains exceptions and error codes.""" - +import json import logging from http import HTTPStatus from typing import Dict, List -from canonicaljson import json - from twisted.web import http logger = logging.getLogger(__name__) @@ -573,7 +571,7 @@ class HttpResponseException(CodeMessageException): # try to parse the body as json, to get better errcode/msg, but # default to M_UNKNOWN with the HTTP status as the error text try: - j = json.loads(self.response) + j = json.loads(self.response.decode("utf-8")) except ValueError: j = {} -- cgit 1.5.1 From 4db1509516eecb24db54e2f3cb479d01dde27b0d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Jul 2020 07:03:58 -0400 Subject: Improve the type hints of synapse.api.errors. (#7820) --- changelog.d/7820.misc | 1 + synapse/api/errors.py | 128 ++++++++++++++++++++++++++++---------------------- 2 files changed, 74 insertions(+), 55 deletions(-) create mode 100644 changelog.d/7820.misc (limited to 'synapse/api/errors.py') diff --git a/changelog.d/7820.misc b/changelog.d/7820.misc new file mode 100644 index 0000000000..b77b5672e3 --- /dev/null +++ b/changelog.d/7820.misc @@ -0,0 +1 @@ +Add type hints to synapse.api.errors module. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index d5d4522336..cc5edb5118 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -17,11 +17,15 @@ """Contains exceptions and error codes.""" import json import logging +import typing from http import HTTPStatus -from typing import Dict, List +from typing import Dict, List, Optional, Union from twisted.web import http +if typing.TYPE_CHECKING: + from synapse.types import JsonDict + logger = logging.getLogger(__name__) @@ -76,11 +80,11 @@ class CodeMessageException(RuntimeError): """An exception with integer code and message string attributes. Attributes: - code (int): HTTP error code - msg (str): string describing the error + code: HTTP error code + msg: string describing the error """ - def __init__(self, code, msg): + def __init__(self, code: Union[int, HTTPStatus], msg: str): super(CodeMessageException, self).__init__("%d: %s" % (code, msg)) # Some calls to this method pass instances of http.HTTPStatus for `code`. @@ -121,16 +125,16 @@ class SynapseError(CodeMessageException): message (as well as an HTTP status code). Attributes: - errcode (str): Matrix error code e.g 'M_FORBIDDEN' + errcode: Matrix error code e.g 'M_FORBIDDEN' """ - def __init__(self, code, msg, errcode=Codes.UNKNOWN): + def __init__(self, code: int, msg: str, errcode: str = Codes.UNKNOWN): """Constructs a synapse error. Args: - code (int): The integer error code (an HTTP response code) - msg (str): The human-readable error message. - errcode (str): The matrix error code e.g 'M_FORBIDDEN' + code: The integer error code (an HTTP response code) + msg: The human-readable error message. + errcode: The matrix error code e.g 'M_FORBIDDEN' """ super(SynapseError, self).__init__(code, msg) self.errcode = errcode @@ -143,10 +147,16 @@ class ProxiedRequestError(SynapseError): """An error from a general matrix endpoint, eg. from a proxied Matrix API call. Attributes: - errcode (str): Matrix error code e.g 'M_FORBIDDEN' + errcode: Matrix error code e.g 'M_FORBIDDEN' """ - def __init__(self, code, msg, errcode=Codes.UNKNOWN, additional_fields=None): + def __init__( + self, + code: int, + msg: str, + errcode: str = Codes.UNKNOWN, + additional_fields: Optional[Dict] = None, + ): super(ProxiedRequestError, self).__init__(code, msg, errcode) if additional_fields is None: self._additional_fields = {} # type: Dict @@ -162,12 +172,12 @@ class ConsentNotGivenError(SynapseError): privacy policy. """ - def __init__(self, msg, consent_uri): + def __init__(self, msg: str, consent_uri: str): """Constructs a ConsentNotGivenError Args: - msg (str): The human-readable error message - consent_url (str): The URL where the user can give their consent + msg: The human-readable error message + consent_url: The URL where the user can give their consent """ super(ConsentNotGivenError, self).__init__( code=HTTPStatus.FORBIDDEN, msg=msg, errcode=Codes.CONSENT_NOT_GIVEN @@ -183,11 +193,11 @@ class UserDeactivatedError(SynapseError): authenticated endpoint, but the account has been deactivated. """ - def __init__(self, msg): + def __init__(self, msg: str): """Constructs a UserDeactivatedError Args: - msg (str): The human-readable error message + msg: The human-readable error message """ super(UserDeactivatedError, self).__init__( code=HTTPStatus.FORBIDDEN, msg=msg, errcode=Codes.USER_DEACTIVATED @@ -199,16 +209,16 @@ class FederationDeniedError(SynapseError): is not on its federation whitelist. Attributes: - destination (str): The destination which has been denied + destination: The destination which has been denied """ - def __init__(self, destination): + def __init__(self, destination: Optional[str]): """Raised by federation client or server to indicate that we are are deliberately not attempting to contact a given server because it is not on our federation whitelist. Args: - destination (str): the domain in question + destination: the domain in question """ self.destination = destination @@ -226,11 +236,11 @@ class InteractiveAuthIncompleteError(Exception): (This indicates we should return a 401 with 'result' as the body) Attributes: - result (dict): the server response to the request, which should be + result: the server response to the request, which should be passed back to the client """ - def __init__(self, result): + def __init__(self, result: "JsonDict"): super(InteractiveAuthIncompleteError, self).__init__( "Interactive auth not yet complete" ) @@ -243,7 +253,6 @@ class UnrecognizedRequestError(SynapseError): def __init__(self, *args, **kwargs): if "errcode" not in kwargs: kwargs["errcode"] = Codes.UNRECOGNIZED - message = None if len(args) == 0: message = "Unrecognized request" else: @@ -254,7 +263,7 @@ class UnrecognizedRequestError(SynapseError): class NotFoundError(SynapseError): """An error indicating we can't find the thing you asked for""" - def __init__(self, msg="Not found", errcode=Codes.NOT_FOUND): + def __init__(self, msg: str = "Not found", errcode: str = Codes.NOT_FOUND): super(NotFoundError, self).__init__(404, msg, errcode=errcode) @@ -280,21 +289,23 @@ class InvalidClientCredentialsError(SynapseError): M_UNKNOWN_TOKEN respectively. """ - def __init__(self, msg, errcode): + def __init__(self, msg: str, errcode: str): super().__init__(code=401, msg=msg, errcode=errcode) class MissingClientTokenError(InvalidClientCredentialsError): """Raised when we couldn't find the access token in a request""" - def __init__(self, msg="Missing access token"): + def __init__(self, msg: str = "Missing access token"): super().__init__(msg=msg, errcode="M_MISSING_TOKEN") class InvalidClientTokenError(InvalidClientCredentialsError): """Raised when we didn't understand the access token in a request""" - def __init__(self, msg="Unrecognised access token", soft_logout=False): + def __init__( + self, msg: str = "Unrecognised access token", soft_logout: bool = False + ): super().__init__(msg=msg, errcode="M_UNKNOWN_TOKEN") self._soft_logout = soft_logout @@ -312,11 +323,11 @@ class ResourceLimitError(SynapseError): def __init__( self, - code, - msg, - errcode=Codes.RESOURCE_LIMIT_EXCEEDED, - admin_contact=None, - limit_type=None, + code: int, + msg: str, + errcode: str = Codes.RESOURCE_LIMIT_EXCEEDED, + admin_contact: Optional[str] = None, + limit_type: Optional[str] = None, ): self.admin_contact = admin_contact self.limit_type = limit_type @@ -364,10 +375,10 @@ class StoreError(SynapseError): class InvalidCaptchaError(SynapseError): def __init__( self, - code=400, - msg="Invalid captcha.", - error_url=None, - errcode=Codes.CAPTCHA_INVALID, + code: int = 400, + msg: str = "Invalid captcha.", + error_url: Optional[str] = None, + errcode: str = Codes.CAPTCHA_INVALID, ): super(InvalidCaptchaError, self).__init__(code, msg, errcode) self.error_url = error_url @@ -382,10 +393,10 @@ class LimitExceededError(SynapseError): def __init__( self, - code=429, - msg="Too Many Requests", - retry_after_ms=None, - errcode=Codes.LIMIT_EXCEEDED, + code: int = 429, + msg: str = "Too Many Requests", + retry_after_ms: Optional[int] = None, + errcode: str = Codes.LIMIT_EXCEEDED, ): super(LimitExceededError, self).__init__(code, msg, errcode) self.retry_after_ms = retry_after_ms @@ -398,10 +409,10 @@ class RoomKeysVersionError(SynapseError): """A client has tried to upload to a non-current version of the room_keys store """ - def __init__(self, current_version): + def __init__(self, current_version: str): """ Args: - current_version (str): the current version of the store they should have used + current_version: the current version of the store they should have used """ super(RoomKeysVersionError, self).__init__( 403, "Wrong room_keys version", Codes.WRONG_ROOM_KEYS_VERSION @@ -413,7 +424,7 @@ class UnsupportedRoomVersionError(SynapseError): """The client's request to create a room used a room version that the server does not support.""" - def __init__(self, msg="Homeserver does not support this room version"): + def __init__(self, msg: str = "Homeserver does not support this room version"): super(UnsupportedRoomVersionError, self).__init__( code=400, msg=msg, errcode=Codes.UNSUPPORTED_ROOM_VERSION, ) @@ -435,7 +446,7 @@ class IncompatibleRoomVersionError(SynapseError): failing. """ - def __init__(self, room_version): + def __init__(self, room_version: str): super(IncompatibleRoomVersionError, self).__init__( code=400, msg="Your homeserver does not support the features required to " @@ -455,8 +466,8 @@ class PasswordRefusedError(SynapseError): def __init__( self, - msg="This password doesn't comply with the server's policy", - errcode=Codes.WEAK_PASSWORD, + msg: str = "This password doesn't comply with the server's policy", + errcode: str = Codes.WEAK_PASSWORD, ): super(PasswordRefusedError, self).__init__( code=400, msg=msg, errcode=errcode, @@ -481,14 +492,14 @@ class RequestSendFailed(RuntimeError): self.can_retry = can_retry -def cs_error(msg, code=Codes.UNKNOWN, **kwargs): +def cs_error(msg: str, code: str = Codes.UNKNOWN, **kwargs): """ Utility method for constructing an error response for client-server interactions. Args: - msg (str): The error message. - code (str): The error code. - kwargs : Additional keys to add to the response. + msg: The error message. + code: The error code. + kwargs: Additional keys to add to the response. Returns: A dict representing the error response JSON. """ @@ -510,7 +521,14 @@ class FederationError(RuntimeError): is wrong (e.g., it referred to an invalid event) """ - def __init__(self, level, code, reason, affected, source=None): + def __init__( + self, + level: str, + code: int, + reason: str, + affected: str, + source: Optional[str] = None, + ): if level not in ["FATAL", "ERROR", "WARN"]: raise ValueError("Level is not valid: %s" % (level,)) self.level = level @@ -537,16 +555,16 @@ class HttpResponseException(CodeMessageException): Represents an HTTP-level failure of an outbound request Attributes: - response (bytes): body of response + response: body of response """ - def __init__(self, code, msg, response): + def __init__(self, code: int, msg: str, response: bytes): """ Args: - code (int): HTTP status code - msg (str): reason phrase from HTTP response status line - response (bytes): body of response + code: HTTP status code + msg: reason phrase from HTTP response status line + response: body of response """ super(HttpResponseException, self).__init__(code, msg) self.response = response -- cgit 1.5.1 From 35450519dee869e5641cff94ed35acd3bdaa8a42 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Jul 2020 13:40:54 -0400 Subject: Ensure that calls to `json.dumps` are compatible with the standard library json. (#7836) --- changelog.d/7836.misc | 1 + synapse/api/errors.py | 4 +++- synapse/federation/federation_server.py | 2 +- synapse/handlers/ui_auth/checkers.py | 3 ++- synapse/http/client.py | 4 ++-- synapse/http/servlet.py | 4 +++- synapse/rest/client/v1/room.py | 13 +++++++------ synapse/rest/key/v2/remote_key_resource.py | 4 +++- 8 files changed, 22 insertions(+), 13 deletions(-) create mode 100644 changelog.d/7836.misc (limited to 'synapse/api/errors.py') diff --git a/changelog.d/7836.misc b/changelog.d/7836.misc new file mode 100644 index 0000000000..a3a97c7590 --- /dev/null +++ b/changelog.d/7836.misc @@ -0,0 +1 @@ +Ensure that calls to `json.dumps` are compatible with the standard library json. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index cc5edb5118..b3bab1aa52 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -15,12 +15,14 @@ # limitations under the License. """Contains exceptions and error codes.""" -import json + import logging import typing from http import HTTPStatus from typing import Dict, List, Optional, Union +from canonicaljson import json + from twisted.web import http if typing.TYPE_CHECKING: diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 2aab9c5f55..8c53330c49 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -14,10 +14,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import json import logging from typing import Any, Callable, Dict, List, Match, Optional, Tuple, Union +from canonicaljson import json from prometheus_client import Counter, Histogram from twisted.internet import defer diff --git a/synapse/handlers/ui_auth/checkers.py b/synapse/handlers/ui_auth/checkers.py index 8b24a73319..a140e9391e 100644 --- a/synapse/handlers/ui_auth/checkers.py +++ b/synapse/handlers/ui_auth/checkers.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import logging from canonicaljson import json @@ -117,7 +118,7 @@ class RecaptchaAuthChecker(UserInteractiveAuthChecker): except PartialDownloadError as pde: # Twisted is silly data = pde.response - resp_body = json.loads(data) + resp_body = json.loads(data.decode("utf-8")) if "success" in resp_body: # Note that we do NOT check the hostname here: we explicitly diff --git a/synapse/http/client.py b/synapse/http/client.py index b80681135e..6bc51202cd 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -13,13 +13,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -import json + import logging import urllib from io import BytesIO import treq -from canonicaljson import encode_canonical_json +from canonicaljson import encode_canonical_json, json from netaddr import IPAddress from prometheus_client import Counter from zope.interface import implementer, provider diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py index 3cabe9d02e..a34e5ead88 100644 --- a/synapse/http/servlet.py +++ b/synapse/http/servlet.py @@ -14,9 +14,11 @@ # limitations under the License. """ This module contains base REST classes for constructing REST servlets. """ -import json + import logging +from canonicaljson import json + from synapse.api.errors import Codes, SynapseError logger = logging.getLogger(__name__) diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index f40ed82142..ea5912d4e4 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -15,6 +15,7 @@ # limitations under the License. """ This module contains REST servlets to do with rooms: /rooms/ """ + import logging import re from typing import List, Optional @@ -515,9 +516,9 @@ class RoomMessageListRestServlet(RestServlet): requester = await self.auth.get_user_by_req(request, allow_guest=True) pagination_config = PaginationConfig.from_request(request, default_limit=10) as_client_event = b"raw" not in request.args - filter_bytes = parse_string(request, b"filter", encoding=None) - if filter_bytes: - filter_json = urlparse.unquote(filter_bytes.decode("UTF-8")) + filter_str = parse_string(request, b"filter", encoding="utf-8") + if filter_str: + filter_json = urlparse.unquote(filter_str) event_filter = Filter(json.loads(filter_json)) # type: Optional[Filter] if ( event_filter @@ -627,9 +628,9 @@ class RoomEventContextServlet(RestServlet): limit = parse_integer(request, "limit", default=10) # picking the API shape for symmetry with /messages - filter_bytes = parse_string(request, "filter") - if filter_bytes: - filter_json = urlparse.unquote(filter_bytes) + filter_str = parse_string(request, b"filter", encoding="utf-8") + if filter_str: + filter_json = urlparse.unquote(filter_str) event_filter = Filter(json.loads(filter_json)) # type: Optional[Filter] else: event_filter = None diff --git a/synapse/rest/key/v2/remote_key_resource.py b/synapse/rest/key/v2/remote_key_resource.py index e149ac1733..9b3f85b306 100644 --- a/synapse/rest/key/v2/remote_key_resource.py +++ b/synapse/rest/key/v2/remote_key_resource.py @@ -202,9 +202,11 @@ class RemoteKey(DirectServeJsonResource): if miss: cache_misses.setdefault(server_name, set()).add(key_id) + # Cast to bytes since postgresql returns a memoryview. json_results.add(bytes(most_recent_result["key_json"])) else: for ts_added, result in results: + # Cast to bytes since postgresql returns a memoryview. json_results.add(bytes(result["key_json"])) if cache_misses and query_remote_on_cache_miss: @@ -213,7 +215,7 @@ class RemoteKey(DirectServeJsonResource): else: signed_keys = [] for key_json in json_results: - key_json = json.loads(key_json) + key_json = json.loads(key_json.decode("utf-8")) for signing_key in self.config.key_server_signing_keys: key_json = sign_json(key_json, self.config.server_name, signing_key) -- cgit 1.5.1