txAWS

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

  1. catch-all-server-error
  2. Merge into trunk
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
1=== modified file 'txaws/server/resource.py'
2--- txaws/server/resource.py 2013-08-22 14:03:25 +0000
3+++ txaws/server/resource.py 2013-08-22 14:03:25 +0000
4@@ -115,8 +115,12 @@
5 if body is None:
6 body = self.dump_error(failure.value, request)
7 else:
8+ # If the error is a generic one (not an APIError), log the
9+ # message , but don't send it back to the client, as it could
10+ # contain sensitive information. Send a generic server error
11+ # message instead.
12 log.err(failure)
13- body = safe_str(failure.value)
14+ body = "Server error"
15 status = 500
16 request.setResponseCode(status)
17 write_response(body)
18
19=== modified file 'txaws/server/tests/test_resource.py'
20--- txaws/server/tests/test_resource.py 2013-08-22 14:03:25 +0000
21+++ txaws/server/tests/test_resource.py 2013-08-22 14:03:25 +0000
22@@ -375,8 +375,7 @@
23 errors = self.flushLoggedErrors()
24 self.assertEquals(1, len(errors))
25 self.assertTrue(request.finished)
26- self.assertEqual("integer division or modulo by zero",
27- request.response)
28+ self.assertEqual("Server error", request.response)
29 self.assertEqual(500, request.code)
30
31 self.api.principal = TestPrincipal(creds)
32@@ -495,10 +494,10 @@
33 self.api.execute = fail_execute
34
35 def check(ignored):
36- errors = self.flushLoggedErrors()
37- self.assertEquals(1, len(errors))
38+ [error] = self.flushLoggedErrors()
39+ self.assertIsInstance(error.value, ValueError)
40 self.assertTrue(request.finished)
41- self.assertIn("ValueError", request.response)
42+ self.assertEqual("Server error", request.response)
43 self.assertEqual(500, request.code)
44
45 self.api.principal = TestPrincipal(creds)

Subscribers

People subscribed via source and target branches