From 1a9255c12eb73245bdbb626a1a0cad2fbe967caa Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 25 Apr 2017 19:30:55 +0100 Subject: [PATCH] Use CodeMessageException subclass instead Parse json errors from get_json client methods and throw special errors. --- synapse/api/errors.py | 11 +++++++ synapse/handlers/identity.py | 29 ++++++++++------ synapse/http/client.py | 64 +++++++++++------------------------- synapse/server.py | 8 +---- 4 files changed, 51 insertions(+), 61 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 6fbd5d6876..d0dfa959dc 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -66,6 +66,17 @@ class CodeMessageException(RuntimeError): return cs_error(self.msg) +class MatrixCodeMessageException(CodeMessageException): + """An error from a general matrix endpoint, eg. from a proxied Matrix API call. + + Attributes: + errcode (str): Matrix error code e.g 'M_FORBIDDEN' + """ + def __init__(self, code, msg, errcode=Codes.UNKNOWN): + super(MatrixCodeMessageException, self).__init__(code, msg) + self.errcode = errcode + + class SynapseError(CodeMessageException): """A base exception type for matrix errors which have an errcode and error message (as well as an HTTP status code). diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index 41f978d990..8b407a307c 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -18,7 +18,7 @@ from twisted.internet import defer from synapse.api.errors import ( - CodeMessageException + MatrixCodeMessageException, CodeMessageException ) from ._base import BaseHandler from synapse.util.async import run_on_reactor @@ -35,7 +35,7 @@ class IdentityHandler(BaseHandler): def __init__(self, hs): super(IdentityHandler, self).__init__(hs) - self.proxy_client = hs.get_matrix_proxy_client() + self.http_client = hs.get_simple_http_client() self.trusted_id_servers = set(hs.config.trusted_third_party_id_servers) self.trust_any_id_server_just_for_testing_do_not_use = ( @@ -83,13 +83,16 @@ class IdentityHandler(BaseHandler): data = {} try: - data = yield self.proxy_client.get_json( - "https://%s%s" % ( + data = yield self.http_client.get_json( + "http://%s%s" % ( id_server, "/_matrix/identity/api/v1/3pid/getValidated3pid" ), {'sid': creds['sid'], 'client_secret': client_secret} ) + except MatrixCodeMessageException as e: + logger.info("getValidated3pid failed with Matrix error: %r", e) + raise SynapseError(e.code, e.msg, e.errcode) except CodeMessageException as e: data = json.loads(e.msg) @@ -118,8 +121,8 @@ class IdentityHandler(BaseHandler): raise SynapseError(400, "No client_secret in creds") try: - data = yield self.proxy_client.post_urlencoded_get_json( - "https://%s%s" % ( + data = yield self.http_client.post_urlencoded_get_json( + "http://%s%s" % ( id_server, "/_matrix/identity/api/v1/3pid/bind" ), { @@ -151,14 +154,17 @@ class IdentityHandler(BaseHandler): params.update(kwargs) try: - data = yield self.proxy_client.post_json_get_json( - "https://%s%s" % ( + data = yield self.http_client.post_json_get_json( + "http://%s%s" % ( id_server, "/_matrix/identity/api/v1/validate/email/requestToken" ), params ) defer.returnValue(data) + except MatrixCodeMessageException as e: + logger.info("Proxied requestToken failed with Matrix error: %r", e) + raise SynapseError(e.code, e.msg, e.errcode) except CodeMessageException as e: logger.info("Proxied requestToken failed: %r", e) raise e @@ -185,14 +191,17 @@ class IdentityHandler(BaseHandler): params.update(kwargs) try: - data = yield self.proxy_client.post_json_get_json( - "https://%s%s" % ( + data = yield self.http_client.post_json_get_json( + "http://%s%s" % ( id_server, "/_matrix/identity/api/v1/validate/msisdn/requestToken" ), params ) defer.returnValue(data) + except MatrixCodeMessageException as e: + logger.info("Proxied requestToken failed with Matrix error: %r", e) + raise SynapseError(e.code, e.msg, e.errcode) except CodeMessageException as e: logger.info("Proxied requestToken failed: %r", e) raise e diff --git a/synapse/http/client.py b/synapse/http/client.py index df8c3e3c2c..57a49b2827 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -16,7 +16,7 @@ from OpenSSL import SSL from OpenSSL.SSL import VERIFY_NONE from synapse.api.errors import ( - CodeMessageException, SynapseError, Codes, + CodeMessageException, MatrixCodeMessageException, SynapseError, Codes, ) from synapse.util.logcontext import preserve_context_over_fn import synapse.metrics @@ -145,8 +145,10 @@ class SimpleHttpClient(object): body = yield preserve_context_over_fn(readBody, response) - if response.code / 100 >= 4: - raise CodeMessageException(response.code, body) + if 200 <= response.code < 300: + defer.returnValue(json.loads(body)) + else: + raise self._exceptionFromFailedRequest(response, body) defer.returnValue(json.loads(body)) @@ -168,7 +170,11 @@ class SimpleHttpClient(object): error message. """ body = yield self.get_raw(uri, args) - defer.returnValue(json.loads(body)) + + if 200 <= response.code < 300: + defer.returnValue(json.loads(body)) + else: + raise self._exceptionFromFailedRequest(response, body) @defer.inlineCallbacks def put_json(self, uri, json_body, args={}): @@ -249,6 +255,16 @@ class SimpleHttpClient(object): else: raise CodeMessageException(response.code, body) + def _exceptionFromFailedRequest(self, response, body): + try: + jsonBody = json.loads(body) + errcode = jsonBody['errcode'] + error = jsonBody['error'] + return MatrixCodeMessageException(response.code, error, errcode) + except e: + print e + return CodeMessageException(response.code, body) + # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. # The two should be factored out. @@ -309,46 +325,6 @@ class SimpleHttpClient(object): defer.returnValue((length, headers, response.request.absoluteURI, response.code)) -class MatrixProxyClient(object): - """ - An HTTP client that proxies other Matrix endpoints, ie. if the remote endpoint - returns Matrix-style error response, this will raise the appropriate SynapseError - """ - def __init__(self, hs): - self.simpleHttpClient = SimpleHttpClient(hs) - - @defer.inlineCallbacks - def post_json_get_json(self, uri, post_json): - try: - result = yield self.simpleHttpClient.post_json_get_json(uri, post_json) - defer.returnValue(result) - except CodeMessageException as cme: - ex = self._tryGetMatrixError(cme) - if ex is not None: - raise ex - raise cme - - @defer.inlineCallbacks - def get_json(self, uri, args={}): - try: - result = yield self.simpleHttpClient.get_json(uri, args) - defer.returnValue(result) - except CodeMessageException as cme: - ex = self._tryGetMatrixError(cme) - if ex is not None: - raise ex - raise cme - - def _tryGetMatrixError(self, codeMessageException): - try: - errbody = json.loads(codeMessageException.msg) - errcode = errbody['errcode'] - errtext = errbody['error'] - return SynapseError(codeMessageException.code, errtext, errcode) - except: - return None - - # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient. # The two should be factored out. diff --git a/synapse/server.py b/synapse/server.py index 8325d77a7a..12754c89ae 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -49,9 +49,7 @@ from synapse.handlers.events import EventHandler, EventStreamHandler from synapse.handlers.initial_sync import InitialSyncHandler from synapse.handlers.receipts import ReceiptsHandler from synapse.handlers.read_marker import ReadMarkerHandler -from synapse.http.client import ( - SimpleHttpClient, InsecureInterceptableContextFactory, MatrixProxyClient -) +from synapse.http.client import SimpleHttpClient, InsecureInterceptableContextFactory from synapse.http.matrixfederationclient import MatrixFederationHttpClient from synapse.notifier import Notifier from synapse.push.pusherpool import PusherPool @@ -130,7 +128,6 @@ class HomeServer(object): 'filtering', 'http_client_context_factory', 'simple_http_client', - 'matrix_proxy_client', 'media_repository', 'federation_transport_client', 'federation_sender', @@ -193,9 +190,6 @@ class HomeServer(object): def build_simple_http_client(self): return SimpleHttpClient(self) - def build_matrix_proxy_client(self): - return MatrixProxyClient(self) - def build_v1auth(self): orf = Auth(self) # Matrix spec makes no reference to what HTTP status code is returned,