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

This commit is contained in:
Patrick Cloke 2020-09-10 14:55:25 -04:00 committed by GitHub
parent 192e98111d
commit b86764662b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 167 additions and 2 deletions

1
changelog.d/8291.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a bug introduced in v1.20.0rc1 that the wrong exception was raised when invalid JSON data is encountered.

View File

@ -54,6 +54,7 @@ from synapse.logging.opentracing import (
start_active_span, start_active_span,
tags, tags,
) )
from synapse.util import json_decoder
from synapse.util.async_helpers import timeout_deferred from synapse.util.async_helpers import timeout_deferred
from synapse.util.metrics import Measure from synapse.util.metrics import Measure
@ -164,7 +165,9 @@ async def _handle_json_response(
try: try:
check_content_type_is_json(response.headers) check_content_type_is_json(response.headers)
d = treq.json_content(response) # Use the custom JSON decoder (partially re-implements treq.json_content).
d = treq.text_content(response, encoding="utf-8")
d.addCallback(json_decoder.decode)
d = timeout_deferred(d, timeout=timeout_sec, reactor=reactor) d = timeout_deferred(d, timeout=timeout_sec, reactor=reactor)
body = await make_deferred_yieldable(d) body = await make_deferred_yieldable(d)

View File

@ -28,7 +28,7 @@ logger = logging.getLogger(__name__)
def _reject_invalid_json(val): def _reject_invalid_json(val):
"""Do not allow Infinity, -Infinity, or NaN values in JSON.""" """Do not allow Infinity, -Infinity, or NaN values in JSON."""
raise json.JSONDecodeError("Invalid JSON value: '%s'" % val) raise ValueError("Invalid JSON value: '%s'" % val)
# Create a custom encoder to reduce the whitespace produced by JSON encoding and # Create a custom encoder to reduce the whitespace produced by JSON encoding and

View File

@ -15,6 +15,8 @@
# limitations under the License. # limitations under the License.
import logging import logging
from parameterized import parameterized
from synapse.events import make_event_from_dict from synapse.events import make_event_from_dict
from synapse.federation.federation_server import server_matches_acl_event from synapse.federation.federation_server import server_matches_acl_event
from synapse.rest import admin from synapse.rest import admin
@ -23,6 +25,37 @@ from synapse.rest.client.v1 import login, room
from tests import unittest from tests import unittest
class FederationServerTests(unittest.FederatingHomeserverTestCase):
servlets = [
admin.register_servlets,
room.register_servlets,
login.register_servlets,
]
@parameterized.expand([(b"",), (b"foo",), (b'{"limit": Infinity}',)])
def test_bad_request(self, query_content):
"""
Querying with bad data returns a reasonable error code.
"""
u1 = self.register_user("u1", "pass")
u1_token = self.login("u1", "pass")
room_1 = self.helper.create_room_as(u1, tok=u1_token)
self.inject_room_member(room_1, "@user:other.example.com", "join")
"/get_missing_events/(?P<room_id>[^/]*)/?"
request, channel = self.make_request(
"POST",
"/_matrix/federation/v1/get_missing_events/%s" % (room_1,),
query_content,
)
self.render(request)
self.assertEquals(400, channel.code, channel.result)
self.assertEqual(channel.json_body["errcode"], "M_NOT_JSON")
class ServerACLsTestCase(unittest.TestCase): class ServerACLsTestCase(unittest.TestCase):
def test_blacklisted_server(self): def test_blacklisted_server(self):
e = _create_acl_event({"allow": ["*"], "deny": ["evil.com"]}) e = _create_acl_event({"allow": ["*"], "deny": ["evil.com"]})

View File

@ -16,6 +16,7 @@
from mock import Mock from mock import Mock
from netaddr import IPSet from netaddr import IPSet
from parameterized import parameterized
from twisted.internet import defer from twisted.internet import defer
from twisted.internet.defer import TimeoutError from twisted.internet.defer import TimeoutError
@ -511,3 +512,50 @@ class FederationClientTests(HomeserverTestCase):
self.reactor.advance(120) self.reactor.advance(120)
self.assertTrue(conn.disconnecting) 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)

View File

@ -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"]'))