Merge lp:~ack/txaws/catch-all-server-error into lp:txaws

Proposed by Alberto Donato
Status: Merged
Merged at revision: 157
Proposed branch: lp:~ack/txaws/catch-all-server-error
Merge into: lp:txaws
Prerequisite: lp:~ack/txaws/xss-hardening
Diff against target: 45 lines (+9/-6)
2 files modified
txaws/server/resource.py (+5/-1)
txaws/server/tests/test_resource.py (+4/-5)
To merge this branch: bzr merge lp:~ack/txaws/catch-all-server-error
Reviewer Review Type Date Requested Status
Fernando Correa Neto (community) Approve
txAWS Committers Pending
Review via email: mp+181563@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Fernando Correa Neto (fcorrea) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
115 if body is None:115 if body is None:
116 body = self.dump_error(failure.value, request)116 body = self.dump_error(failure.value, request)
117 else:117 else:
118 # If the error is a generic one (not an APIError), log the
119 # message , but don't send it back to the client, as it could
120 # contain sensitive information. Send a generic server error
121 # message instead.
118 log.err(failure)122 log.err(failure)
119 body = safe_str(failure.value)123 body = "Server error"
120 status = 500124 status = 500
121 request.setResponseCode(status)125 request.setResponseCode(status)
122 write_response(body)126 write_response(body)
123127
=== 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 @@
375 errors = self.flushLoggedErrors()375 errors = self.flushLoggedErrors()
376 self.assertEquals(1, len(errors))376 self.assertEquals(1, len(errors))
377 self.assertTrue(request.finished)377 self.assertTrue(request.finished)
378 self.assertEqual("integer division or modulo by zero",378 self.assertEqual("Server error", request.response)
379 request.response)
380 self.assertEqual(500, request.code)379 self.assertEqual(500, request.code)
381380
382 self.api.principal = TestPrincipal(creds)381 self.api.principal = TestPrincipal(creds)
@@ -495,10 +494,10 @@
495 self.api.execute = fail_execute494 self.api.execute = fail_execute
496495
497 def check(ignored):496 def check(ignored):
498 errors = self.flushLoggedErrors()497 [error] = self.flushLoggedErrors()
499 self.assertEquals(1, len(errors))498 self.assertIsInstance(error.value, ValueError)
500 self.assertTrue(request.finished)499 self.assertTrue(request.finished)
501 self.assertIn("ValueError", request.response)500 self.assertEqual("Server error", request.response)
502 self.assertEqual(500, request.code)501 self.assertEqual(500, request.code)
503502
504 self.api.principal = TestPrincipal(creds)503 self.api.principal = TestPrincipal(creds)

Subscribers

People subscribed via source and target branches