Fix the exception that is raised when invalid JSON is encountered. (#8291)

2 files changed, 128 insertions, 0 deletions

diff --git a/tests/http/test_fedclient.py b/tests/http/test_fedclient.py
index 7f1dfb2128..5604af3795 100644
--- a/tests/http/test_fedclient.py
+++ b/tests/http/test_fedclient.py
@@ -16,6 +16,7 @@
 from mock import Mock
 
 from netaddr import IPSet
+from parameterized import parameterized
 
 from twisted.internet import defer
 from twisted.internet.defer import TimeoutError
@@ -511,3 +512,50 @@ class FederationClientTests(HomeserverTestCase):
         self.reactor.advance(120)
 
         self.assertTrue(conn.disconnecting)
+
+    @parameterized.expand([(b"",), (b"foo",), (b'{"a": Infinity}',)])
+    def test_json_error(self, return_value):
+        """
+        Test what happens if invalid JSON is returned from the remote endpoint.
+        """
+
+        test_d = defer.ensureDeferred(self.cl.get_json("testserv:8008", "foo/bar"))
+
+        self.pump()
+
+        # Nothing happened yet
+        self.assertNoResult(test_d)
+
+        # Make sure treq is trying to connect
+        clients = self.reactor.tcpClients
+        self.assertEqual(len(clients), 1)
+        (host, port, factory, _timeout, _bindAddress) = clients[0]
+        self.assertEqual(host, "1.2.3.4")
+        self.assertEqual(port, 8008)
+
+        # complete the connection and wire it up to a fake transport
+        protocol = factory.buildProtocol(None)
+        transport = StringTransport()
+        protocol.makeConnection(transport)
+
+        # that should have made it send the request to the transport
+        self.assertRegex(transport.value(), b"^GET /foo/bar")
+        self.assertRegex(transport.value(), b"Host: testserv:8008")
+
+        # Deferred is still without a result
+        self.assertNoResult(test_d)
+
+        # Send it the HTTP response
+        protocol.dataReceived(
+            b"HTTP/1.1 200 OK\r\n"
+            b"Server: Fake\r\n"
+            b"Content-Type: application/json\r\n"
+            b"Content-Length: %i\r\n"
+            b"\r\n"
+            b"%s" % (len(return_value), return_value)
+        )
+
+        self.pump()
+
+        f = self.failureResultOf(test_d)
+        self.assertIsInstance(f.value, ValueError)
diff --git a/tests/http/test_servlet.py b/tests/http/test_servlet.py
new file mode 100644
index 0000000000..45089158ce
--- /dev/null
+++ b/tests/http/test_servlet.py
@@ -0,0 +1,80 @@
+# -*- coding: utf-8 -*-
+# Copyright 2020 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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
+from io import BytesIO
+
+from mock import Mock
+
+from synapse.api.errors import SynapseError
+from synapse.http.servlet import (
+    parse_json_object_from_request,
+    parse_json_value_from_request,
+)
+
+from tests import unittest
+
+
+def make_request(content):
+    """Make an object that acts enough like a request."""
+    request = Mock(spec=["content"])
+
+    if isinstance(content, dict):
+        content = json.dumps(content).encode("utf8")
+
+    request.content = BytesIO(content)
+    return request
+
+
+class TestServletUtils(unittest.TestCase):
+    def test_parse_json_value(self):
+        """Basic tests for parse_json_value_from_request."""
+        # Test round-tripping.
+        obj = {"foo": 1}
+        result = parse_json_value_from_request(make_request(obj))
+        self.assertEqual(result, obj)
+
+        # Results don't have to be objects.
+        result = parse_json_value_from_request(make_request(b'["foo"]'))
+        self.assertEqual(result, ["foo"])
+
+        # Test empty.
+        with self.assertRaises(SynapseError):
+            parse_json_value_from_request(make_request(b""))
+
+        result = parse_json_value_from_request(make_request(b""), allow_empty_body=True)
+        self.assertIsNone(result)
+
+        # Invalid UTF-8.
+        with self.assertRaises(SynapseError):
+            parse_json_value_from_request(make_request(b"\xFF\x00"))
+
+        # Invalid JSON.
+        with self.assertRaises(SynapseError):
+            parse_json_value_from_request(make_request(b"foo"))
+
+        with self.assertRaises(SynapseError):
+            parse_json_value_from_request(make_request(b'{"foo": Infinity}'))
+
+    def test_parse_json_object(self):
+        """Basic tests for parse_json_object_from_request."""
+        # Test empty.
+        result = parse_json_object_from_request(
+            make_request(b""), allow_empty_body=True
+        )
+        self.assertEqual(result, {})
+
+        # Test not an object
+        with self.assertRaises(SynapseError):
+            parse_json_object_from_request(make_request(b'["foo"]'))