launchpad-reviewers team mailing list archive

[Kit Randel <kit.randel@xxxxxxxxxxxxx>]

Kit Randel has proposed merging lp:~blr/rutabaga/bad-request-crash into lp:rutabaga.

Commit message:
Teach rutabaga to return a 400 rather than crash on invalid json post data.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~blr/rutabaga/bad-request-crash/+merge/299492

Rutabaga would crash if unable to decode request.json_body
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~blr/rutabaga/bad-request-crash into lp:rutabaga.

=== modified file 'rutabaga/tests/test_views.py'
--- rutabaga/tests/test_views.py	2015-11-12 03:23:20 +0000
+++ rutabaga/tests/test_views.py	2016-07-08 00:46:10 +0000
@@ -108,6 +108,15 @@
                       resp.json['errors'][0]['description'])
         self.assertEqual(400, resp.status_code)
 
+    def test_invalid_post_non_json(self):
+        self.app.authorization = ('Basic', (self.admin_username,
+                                            self.admin_secret))
+        resp = self.app.post('/tokens', {'junk': 'junk'},
+                             expect_errors=True)
+        self.assertIn('Invalid json.',
+                      resp.json['errors'][0]['description'])
+        self.assertEqual(400, resp.status_code)
+
     def test_create_token_with_reserved_admin_prefix(self):
         # This _should_ be # BUILD-1234 etc.
         self.app.authorization = ('Basic', (self.admin_username,

=== modified file 'rutabaga/views.py'
--- rutabaga/views.py	2015-11-19 21:44:39 +0000
+++ rutabaga/views.py	2016-07-08 00:46:10 +0000
@@ -1,6 +1,7 @@
 # Copyright 2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from json.decoder import JSONDecodeError
 import sqlite3
 
 from cornice.resource import (
@@ -53,10 +54,16 @@
         if 'admin' not in request.effective_principals:
             raise exc.HTTPUnauthorized()
         admin_prefix = self.config.get('admin_api_username_prefix')
-        if 'username' not in request.json_body:
-            request.errors.add('body', 'username', 'username is missing.')
-        elif request.json_body['username'].startswith(admin_prefix):
-            request.errors.add('body', 'username', 'invalid username.')
+        data = {}
+        try:
+            data = request.json_body
+        except JSONDecodeError:
+            request.errors.add('body', 'json', 'Invalid json.')
+        finally:
+            if 'username' not in data:
+                request.errors.add('body', 'username', 'username is missing.')
+            elif data['username'].startswith(admin_prefix):
+                request.errors.add('body', 'username', 'invalid username.')
 
     def validate_delete_token(self, request):
         if 'admin' in request.effective_principals: