diff --git a/CHANGES.txt b/CHANGES.txt index 1a93621bf..072e278bd 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,10 @@ +Next 3.x Release +---------------- + +- 400 Bad Request is now returned if an XML-RPC request is received with + invalid body data. In previous versions, 500 Internal Server Error + was returned. + 3.2.2 (2016-03-04) ------------------ diff --git a/supervisor/tests/test_xmlrpc.py b/supervisor/tests/test_xmlrpc.py index b18f6abab..8e5de21ee 100644 --- a/supervisor/tests/test_xmlrpc.py +++ b/supervisor/tests/test_xmlrpc.py @@ -175,9 +175,47 @@ def test_continue_request_400_if_method_name_is_empty(self): else: expected = 2 self.assertEqual(len(logdata), expected) - self.assertEqual(logdata[-1], - u'XML-RPC request received with no method name') - self.assertEqual(len(request.producers), 0) + self.assertTrue(logdata[-1].startswith('XML-RPC request data')) + self.assertTrue(repr(data) in logdata[-1]) + self.assertTrue(logdata[-1].endswith('is invalid: no method name')) + self.assertEqual(request._error, 400) + + def test_continue_request_400_if_loads_raises_not_xml(self): + supervisor = DummySupervisor() + subinterfaces = [('supervisor', DummySupervisorRPCNamespace())] + handler = self._makeOne(supervisor, subinterfaces) + data = 'this is not an xml-rpc request body' + request = DummyRequest('/what/ever', None, None, None) + handler.continue_request(data, request) + logdata = supervisor.options.logger.data + from supervisor.xmlrpc import loads + if loads: + expected = 1 + else: + expected = 2 + self.assertEqual(len(logdata), expected) + self.assertTrue(logdata[-1].startswith('XML-RPC request data')) + self.assertTrue(repr(data) in logdata[-1]) + self.assertTrue(logdata[-1].endswith('is invalid: unmarshallable')) + self.assertEqual(request._error, 400) + + def test_continue_request_400_if_loads_raises_weird_xml(self): + supervisor = DummySupervisor() + subinterfaces = [('supervisor', DummySupervisorRPCNamespace())] + handler = self._makeOne(supervisor, subinterfaces) + data = '' + request = DummyRequest('/what/ever', None, None, None) + handler.continue_request(data, request) + logdata = supervisor.options.logger.data + from supervisor.xmlrpc import loads + if loads: + expected = 1 + else: + expected = 2 + self.assertEqual(len(logdata), expected) + self.assertTrue(logdata[-1].startswith('XML-RPC request data')) + self.assertTrue(repr(data) in logdata[-1]) + self.assertTrue(logdata[-1].endswith('is invalid: unmarshallable')) self.assertEqual(request._error, 400) def test_continue_request_500_if_rpcinterface_method_call_raises(self): @@ -228,6 +266,7 @@ def test_continue_request_500_if_xmlrpc_dumps_raises(self): self.assertTrue(repr(data) in logdata[-1]) self.assertTrue("Traceback" in logdata[-1]) self.assertTrue("TypeError: cannot marshal" in logdata[-1]) + self.assertEqual(request._error, 500) class TraverseTests(unittest.TestCase): def test_underscore(self): diff --git a/supervisor/xmlrpc.py b/supervisor/xmlrpc.py index aaf759b78..339e19e48 100644 --- a/supervisor/xmlrpc.py +++ b/supervisor/xmlrpc.py @@ -359,15 +359,26 @@ def __init__(self, supervisord, subinterfaces): def match(self, request): return request.uri.startswith(self.path) - def continue_request (self, data, request): + def continue_request(self, data, request): logger = self.supervisord.options.logger try: - params, method = self.loads(data) + try: + params, method = self.loads(data) + except: + logger.error( + 'XML-RPC request data %r is invalid: unmarshallable' % + (data,) + ) + request.error(400) + return # no in the request or name is an empty string if not method: - logger.trace('XML-RPC request received with no method name') + logger.error( + 'XML-RPC request data %r is invalid: no method name' % + (data,) + ) request.error(400) return