Return 400 Bad Request if the XML-RPC request body is invalid

Author: mnaberez

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)
 ------------------
 

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 = '<methodName></methodName><junk></junk>'
+        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):

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 <methodName> 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