txaws-dev team mailing list archive

[Alberto Donato <alberto.donato@xxxxxxxxxxxxx>]

Alberto Donato has proposed merging lp:~ack/txaws/catch-all-server-error into lp:txaws with lp:~ack/txaws/xss-hardening as a prerequisite.

Requested reviews:
  txAWS Committers (txaws-dev)
  txAWS Committers (txaws-dev)

For more details, see:
https://code.launchpad.net/~ack/txaws/catch-all-server-error/+merge/181563

This returns a generic "Server error" message for exceptions that are not APIError (the code is already 500 for those responses).
This avoids potential disclosure of server-side information (such as from tracebacks).
-- 
https://code.launchpad.net/~ack/txaws/catch-all-server-error/+merge/181563
Your team txAWS Committers is requested to review the proposed merge of lp:~ack/txaws/catch-all-server-error into lp:txaws.

=== modified file 'txaws/server/resource.py'
--- txaws/server/resource.py	2013-08-22 14:03:25 +0000
+++ txaws/server/resource.py	2013-08-22 14:03:25 +0000
@@ -115,8 +115,12 @@
                 if body is None:
                     body = self.dump_error(failure.value, request)
             else:
+                # If the error is a generic one (not an APIError), log the
+                # message , but don't send it back to the client, as it could
+                # contain sensitive information. Send a generic server error
+                # message instead.
                 log.err(failure)
-                body = safe_str(failure.value)
+                body = "Server error"
                 status = 500
             request.setResponseCode(status)
             write_response(body)

=== modified file 'txaws/server/tests/test_resource.py'
--- txaws/server/tests/test_resource.py	2013-08-22 14:03:25 +0000
+++ txaws/server/tests/test_resource.py	2013-08-22 14:03:25 +0000
@@ -375,8 +375,7 @@
             errors = self.flushLoggedErrors()
             self.assertEquals(1, len(errors))
             self.assertTrue(request.finished)
-            self.assertEqual("integer division or modulo by zero",
-                             request.response)
+            self.assertEqual("Server error", request.response)
             self.assertEqual(500, request.code)
 
         self.api.principal = TestPrincipal(creds)
@@ -495,10 +494,10 @@
         self.api.execute = fail_execute
 
         def check(ignored):
-            errors = self.flushLoggedErrors()
-            self.assertEquals(1, len(errors))
+            [error] = self.flushLoggedErrors()
+            self.assertIsInstance(error.value, ValueError)
             self.assertTrue(request.finished)
-            self.assertIn("ValueError", request.response)
+            self.assertEqual("Server error", request.response)
             self.assertEqual(500, request.code)
 
         self.api.principal = TestPrincipal(creds)