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

Author: mnaberez

CHANGES.txt

@@ -31,6 +31,10 @@
 - Files included via the ``[include]`` section are now logged at the ``INFO``
   level instead of ``WARN``.  Patch by Daniel Hahler.
 
+- ``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

@@ -153,9 +153,37 @@ def test_continue_request_400_if_method_name_is_empty(self):
         handler.continue_request(data, request)
         logdata = supervisor.options.logger.data
         self.assertEqual(len(logdata), 1)
-        self.assertEqual(logdata[0],
-               'XML-RPC request received with no method name')
-        self.assertEqual(len(request.producers), 0)
+        self.assertTrue(logdata[0].startswith('XML-RPC request data'))
+        self.assertTrue(repr(data) in logdata[0])
+        self.assertTrue(logdata[0].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
+        self.assertEqual(len(logdata), 1)
+        self.assertTrue(logdata[0].startswith('XML-RPC request data'))
+        self.assertTrue(repr(data) in logdata[0])
+        self.assertTrue(logdata[0].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
+        self.assertEqual(len(logdata), 1)
+        self.assertTrue(logdata[0].startswith('XML-RPC request data'))
+        self.assertTrue(repr(data) in logdata[0])
+        self.assertTrue(logdata[0].endswith('is invalid: unmarshallable'))
         self.assertEqual(request._error, 400)
 
     def test_continue_request_500_if_rpcinterface_method_call_raises(self):
@@ -173,6 +201,7 @@ def test_continue_request_500_if_rpcinterface_method_call_raises(self):
         self.assertTrue(repr(data) in logdata[1])
         self.assertTrue("Traceback" in logdata[1])
         self.assertTrue("ValueError: error" in logdata[1])
+        self.assertEqual(request._error, 500)
 
     def test_continue_request_500_if_xmlrpc_dumps_raises(self):
         supervisor = DummySupervisor()
@@ -192,7 +221,7 @@ def test_continue_request_500_if_xmlrpc_dumps_raises(self):
         self.assertTrue(repr(data) in logdata[2])
         self.assertTrue("Traceback" in logdata[2])
         self.assertTrue("TypeError: cannot marshal" in logdata[2])
-
+        self.assertEqual(request._error, 500)
 
     def test_continue_request_value_is_function(self):
         class DummyRPCNamespace(object):

supervisor/xmlrpc.py

@@ -377,15 +377,26 @@ def loads(self, data):
     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