Merge pull request #643 from matrix-org/markjh/parse_json_II

Use parse_json_object_from_request to parse JSON out of request bodies

synapse/federation/transport/server.py

@@ -18,6 +18,7 @@ from twisted.internet import defer
 from synapse.api.urls import FEDERATION_PREFIX as PREFIX
 from synapse.api.errors import Codes, SynapseError
 from synapse.http.server import JsonResource
+from synapse.http.servlet import parse_json_object_from_request
 from synapse.util.ratelimitutils import FederationRateLimiter
 
 import functools
@@ -419,8 +420,7 @@ class On3pidBindServlet(BaseFederationServlet):
 
     @defer.inlineCallbacks
     def on_POST(self, request):
-        content_bytes = request.content.read()
+        content = parse_json_object_from_request(request)
-        content = json.loads(content_bytes)
         if "invites" in content:
             last_exception = None
             for invite in content["invites"]:

synapse/http/servlet.py

@@ -128,13 +128,20 @@ def parse_json_object_from_request(request):
             if it wasn't a JSON object.
     """
     try:
-        content = simplejson.loads(request.content.read())
+        content_bytes = request.content.read()
+    except:
+        raise SynapseError(400, "Error reading JSON content.")
+
+    try:
+        content = simplejson.loads(content_bytes)
+    except simplejson.JSONDecodeError:
+        raise SynapseError(400, "Content not JSON.", errcode=Codes.NOT_JSON)
+
     if type(content) != dict:
         message = "Content must be a JSON object."
         raise SynapseError(400, message, errcode=Codes.BAD_JSON)
+
     return content
-    except simplejson.JSONDecodeError:
-        raise SynapseError(400, "Content not JSON.", errcode=Codes.NOT_JSON)
 
 
 class RestServlet(object):

synapse/rest/client/v1/presence.py

@@ -19,9 +19,9 @@ from twisted.internet import defer
 
 from synapse.api.errors import SynapseError, AuthError
 from synapse.types import UserID
+from synapse.http.servlet import parse_json_object_from_request
 from .base import ClientV1RestServlet, client_path_patterns
 
-import simplejson as json
 import logging
 
 logger = logging.getLogger(__name__)
@@ -56,9 +56,10 @@ class PresenceStatusRestServlet(ClientV1RestServlet):
             raise AuthError(403, "Can only set your own presence state")
 
         state = {}
-        try:
-            content = json.loads(request.content.read())
 
+        content = parse_json_object_from_request(request)
+
+        try:
             state["presence"] = content.pop("presence")
 
             if "status_msg" in content:
@@ -113,11 +114,7 @@ class PresenceListRestServlet(ClientV1RestServlet):
             raise SynapseError(
                 400, "Cannot modify another user's presence list")
 
-        try:
+        content = parse_json_object_from_request(request)
-            content = json.loads(request.content.read())
-        except:
-            logger.exception("JSON parse error")
-            raise SynapseError(400, "Unable to parse content")
 
         if "invite" in content:
             for u in content["invite"]:

synapse/rest/client/v1/profile.py

@@ -18,8 +18,7 @@ from twisted.internet import defer
 
 from .base import ClientV1RestServlet, client_path_patterns
 from synapse.types import UserID
-
+from synapse.http.servlet import parse_json_object_from_request
-import simplejson as json
 
 
 class ProfileDisplaynameRestServlet(ClientV1RestServlet):
@@ -44,8 +43,9 @@ class ProfileDisplaynameRestServlet(ClientV1RestServlet):
         requester = yield self.auth.get_user_by_req(request, allow_guest=True)
         user = UserID.from_string(user_id)
 
+        content = parse_json_object_from_request(request)
+
         try:
-            content = json.loads(request.content.read())
             new_name = content["displayname"]
         except:
             defer.returnValue((400, "Unable to parse name"))
@@ -81,8 +81,8 @@ class ProfileAvatarURLRestServlet(ClientV1RestServlet):
         requester = yield self.auth.get_user_by_req(request)
         user = UserID.from_string(user_id)
 
+        content = parse_json_object_from_request(request)
         try:
-            content = json.loads(request.content.read())
             new_name = content["avatar_url"]
         except:
             defer.returnValue((400, "Unable to parse name"))

synapse/rest/client/v1/room.py

@@ -24,7 +24,6 @@ from synapse.types import UserID, RoomID, RoomAlias
 from synapse.events.utils import serialize_event
 from synapse.http.servlet import parse_json_object_from_request
 
-import simplejson as json
 import logging
 import urllib
 
@@ -72,15 +71,10 @@ class RoomCreateRestServlet(ClientV1RestServlet):
         defer.returnValue((200, info))
 
     def get_room_config(self, request):
-        try:
+        user_supplied_config = parse_json_object_from_request(request)
-            user_supplied_config = json.loads(request.content.read())
-            if "visibility" not in user_supplied_config:
         # default visibility
-                user_supplied_config["visibility"] = "public"
+        user_supplied_config.setdefault("visibility", "public")
         return user_supplied_config
-        except (ValueError, TypeError):
-            raise SynapseError(400, "Body must be JSON.",
-                               errcode=Codes.BAD_JSON)
 
     def on_OPTIONS(self, request):
         return (200, {})

synapse/rest/client/v2_alpha/account_data.py

@@ -15,15 +15,13 @@
 
 from ._base import client_v2_patterns
 
-from synapse.http.servlet import RestServlet
+from synapse.http.servlet import RestServlet, parse_json_object_from_request
-from synapse.api.errors import AuthError, SynapseError
+from synapse.api.errors import AuthError
 
 from twisted.internet import defer
 
 import logging
 
-import simplejson as json
-
 logger = logging.getLogger(__name__)
 
 
@@ -47,11 +45,7 @@ class AccountDataServlet(RestServlet):
         if user_id != requester.user.to_string():
             raise AuthError(403, "Cannot add account data for other users.")
 
-        try:
+        body = parse_json_object_from_request(request)
-            content_bytes = request.content.read()
-            body = json.loads(content_bytes)
-        except:
-            raise SynapseError(400, "Invalid JSON")
 
         max_id = yield self.store.add_account_data_for_user(
             user_id, account_data_type, body
@@ -86,14 +80,7 @@ class RoomAccountDataServlet(RestServlet):
         if user_id != requester.user.to_string():
             raise AuthError(403, "Cannot add account data for other users.")
 
-        try:
+        body = parse_json_object_from_request(request)
-            content_bytes = request.content.read()
-            body = json.loads(content_bytes)
-        except:
-            raise SynapseError(400, "Invalid JSON")
-
-        if not isinstance(body, dict):
-            raise ValueError("Expected a JSON object")
 
         max_id = yield self.store.add_account_data_to_room(
             user_id, room_id, account_data_type, body

synapse/rest/client/v2_alpha/filter.py

@@ -16,12 +16,11 @@
 from twisted.internet import defer
 
 from synapse.api.errors import AuthError, SynapseError
-from synapse.http.servlet import RestServlet
+from synapse.http.servlet import RestServlet, parse_json_object_from_request
 from synapse.types import UserID
 
 from ._base import client_v2_patterns
 
-import simplejson as json
 import logging
 
 
@@ -84,12 +83,7 @@ class CreateFilterRestServlet(RestServlet):
         if not self.hs.is_mine(target_user):
             raise SynapseError(400, "Can only create filters for local users")
 
-        try:
+        content = parse_json_object_from_request(request)
-            content = json.loads(request.content.read())
-
-            # TODO(paul): check for required keys and invalid keys
-        except:
-            raise SynapseError(400, "Invalid filter definition")
 
         filter_id = yield self.filtering.add_user_filter(
             user_localpart=target_user.localpart,

synapse/rest/client/v2_alpha/keys.py

@@ -15,16 +15,15 @@
 
 from twisted.internet import defer
 
-from synapse.api.errors import SynapseError
+from synapse.http.servlet import RestServlet, parse_json_object_from_request
-from synapse.http.servlet import RestServlet
 from synapse.types import UserID
 
 from canonicaljson import encode_canonical_json
 
 from ._base import client_v2_patterns
 
-import simplejson as json
 import logging
+import simplejson as json
 
 logger = logging.getLogger(__name__)
 
@@ -68,10 +67,9 @@ class KeyUploadServlet(RestServlet):
         user_id = requester.user.to_string()
         # TODO: Check that the device_id matches that in the authentication
         # or derive the device_id from the authentication instead.
-        try:
+
-            body = json.loads(request.content.read())
+        body = parse_json_object_from_request(request)
-        except:
+
-            raise SynapseError(400, "Invalid key JSON")
         time_now = self.clock.time_msec()
 
         # TODO: Validate the JSON to make sure it has the right keys.
@@ -173,10 +171,7 @@ class KeyQueryServlet(RestServlet):
     @defer.inlineCallbacks
     def on_POST(self, request, user_id, device_id):
         yield self.auth.get_user_by_req(request)
-        try:
+        body = parse_json_object_from_request(request)
-            body = json.loads(request.content.read())
-        except:
-            raise SynapseError(400, "Invalid key JSON")
         result = yield self.handle_request(body)
         defer.returnValue(result)
 
@@ -272,10 +267,7 @@ class OneTimeKeyServlet(RestServlet):
     @defer.inlineCallbacks
     def on_POST(self, request, user_id, device_id, algorithm):
         yield self.auth.get_user_by_req(request)
-        try:
+        body = parse_json_object_from_request(request)
-            body = json.loads(request.content.read())
-        except:
-            raise SynapseError(400, "Invalid key JSON")
         result = yield self.handle_request(body)
         defer.returnValue(result)
 

synapse/rest/client/v2_alpha/tags.py

@@ -15,15 +15,13 @@
 
 from ._base import client_v2_patterns
 
-from synapse.http.servlet import RestServlet
+from synapse.http.servlet import RestServlet, parse_json_object_from_request
-from synapse.api.errors import AuthError, SynapseError
+from synapse.api.errors import AuthError
 
 from twisted.internet import defer
 
 import logging
 
-import simplejson as json
-
 logger = logging.getLogger(__name__)
 
 
@@ -72,11 +70,7 @@ class TagServlet(RestServlet):
         if user_id != requester.user.to_string():
             raise AuthError(403, "Cannot add tags for other users.")
 
-        try:
+        body = parse_json_object_from_request(request)
-            content_bytes = request.content.read()
-            body = json.loads(content_bytes)
-        except:
-            raise SynapseError(400, "Invalid tag JSON")
 
         max_id = yield self.store.add_tag_to_room(user_id, room_id, tag, body)
 

synapse/rest/key/v2/remote_key_resource.py

@@ -13,7 +13,7 @@
 # limitations under the License.
 
 from synapse.http.server import request_handler, respond_with_json_bytes
-from synapse.http.servlet import parse_integer
+from synapse.http.servlet import parse_integer, parse_json_object_from_request
 from synapse.api.errors import SynapseError, Codes
 
 from twisted.web.resource import Resource
@@ -22,7 +22,6 @@ from twisted.internet import defer
 
 
 from io import BytesIO
-import json
 import logging
 logger = logging.getLogger(__name__)
 
@@ -126,14 +125,7 @@ class RemoteKey(Resource):
     @request_handler
     @defer.inlineCallbacks
     def async_render_POST(self, request):
-        try:
+        content = parse_json_object_from_request(request)
-            content = json.loads(request.content.read())
-            if type(content) != dict:
-                raise ValueError()
-        except ValueError:
-            raise SynapseError(
-                400, "Content must be JSON object.", errcode=Codes.NOT_JSON
-            )
 
         query = content["server_keys"]
 

tests/rest/client/v1/test_profile.py

@@ -95,7 +95,8 @@ class ProfileTestCase(unittest.TestCase):
         mocked_set.side_effect = AuthError(400, "message")
 
         (code, response) = yield self.mock_resource.trigger(
-            "PUT", "/profile/%s/displayname" % ("@4567:test"), '"Frank Jr."'
+            "PUT", "/profile/%s/displayname" % ("@4567:test"),
+            '{"displayname": "Frank Jr."}'
         )
 
         self.assertTrue(
@@ -121,7 +122,8 @@ class ProfileTestCase(unittest.TestCase):
         mocked_set.side_effect = SynapseError(400, "message")
 
         (code, response) = yield self.mock_resource.trigger(
-            "PUT", "/profile/%s/displayname" % ("@opaque:elsewhere"), None
+            "PUT", "/profile/%s/displayname" % ("@opaque:elsewhere"),
+            '{"displayname":"bob"}'
         )
 
         self.assertTrue(