Skip to content

Commit

Permalink
Merge pull request mozilla-services#3287 from peterbe/sanitize-bad-re…
Browse files Browse the repository at this point in the history 
…quests-on-the-web-api

sanitize bad requests on the web api
  • Loading branch information
@willkg
willkg committed Apr 14, 2016
2 parents 01870cb + da2a25b commit e3895a3
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
16 changes: 16 additions & 0 deletions webapp-django/crashstats/api/tests/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ def mocked_get(url, params, **options):
'versions': ['10.0', '11.1'],
})
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['product'])
ok_('versions' not in dump['errors'])
Expand Down Expand Up @@ -276,6 +277,7 @@ def mocked_get(url, params, **options):
'from_date': '2012-01-xx', # invalid format
})
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['from_date'])

Expand Down Expand Up @@ -495,6 +497,7 @@ def mocked_get(url, params, **options):
}
response = self.client.get(url, data)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['limit'])
ok_(dump['errors']['duration'])
Expand All @@ -513,6 +516,7 @@ def test_ReportList(self, rget):
url = reverse('api:model_wrapper', args=('ReportList',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['signature'])

Expand Down Expand Up @@ -690,6 +694,7 @@ def test_ReportList_with_optional_parameters(self, rget):
url = reverse('api:model_wrapper', args=('ReportList',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['signature'])

Expand Down Expand Up @@ -758,6 +763,7 @@ def test_ProcessedCrash(self, rget):
url = reverse('api:model_wrapper', args=('ProcessedCrash',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['crash_id'])

Expand Down Expand Up @@ -838,6 +844,7 @@ def test_UnredactedCrash(self, rget):

response = self.client.get(url, HTTP_AUTH_TOKEN=token.key)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['crash_id'])

Expand Down Expand Up @@ -944,6 +951,7 @@ def mocked_get(url, params, **options):
url = reverse('api:model_wrapper', args=('RawCrash',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['crash_id'])

Expand Down Expand Up @@ -992,6 +1000,7 @@ def mocked_get(url, params, **options):
})
# invalid format
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')

user = self._login()
self._add_permission(user, 'view_pii')
Expand All @@ -1017,6 +1026,7 @@ def test_CommentsBySignature(self, rget):
url = reverse('api:model_wrapper', args=('CommentsBySignature',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['signature'])

Expand Down Expand Up @@ -1079,6 +1089,7 @@ def test_Bugs(self, rpost):
url = reverse('api:model_wrapper', args=('Bugs',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['signatures'])

Expand All @@ -1099,6 +1110,7 @@ def test_SignaturesForBugs(self, rpost):
url = reverse('api:model_wrapper', args=('SignaturesByBugs',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['bug_ids'])

Expand Down Expand Up @@ -1157,6 +1169,7 @@ def mocked_get(url, params, **options):
url = reverse('api:model_wrapper', args=('SignatureTrend',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['product'])
ok_(dump['errors']['version'])
Expand Down Expand Up @@ -1221,6 +1234,7 @@ def mocked_get(url, params, **options):
url = reverse('api:model_wrapper', args=('SignatureSummary',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['start_date'])
ok_(dump['errors']['end_date'])
Expand Down Expand Up @@ -1285,6 +1299,7 @@ def mocked_supersearch_get(**params):
'not_after': 'not a date',
})
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
res = json.loads(response.content)
ok_('errors' in res)
eq_(len(res['errors']), 3)
Expand Down Expand Up @@ -1392,6 +1407,7 @@ def mocked_get(url, params, **options):
url = reverse('api:model_wrapper', args=('SignatureURLs',))
response = self.client.get(url)
eq_(response.status_code, 400)
eq_(response['Content-Type'], 'application/json; charset=UTF-8')
dump = json.loads(response.content)
ok_(dump['errors']['products'])
ok_(dump['errors']['signature'])
Expand Down
19 changes: 13 additions & 6 deletions webapp-django/crashstats/api/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,11 @@ def model_wrapper(request, model_name):
error_code,
'UNKNOWN STATUS CODE'
)
return http.HttpResponse(reason, status=error_code)
return http.HttpResponse(
reason,
status=error_code,
content_type='text/plain; charset=UTF-8'
)
if error_code >= 500:
# special case
reason = REASON_PHRASES[424]
Expand All @@ -303,13 +307,16 @@ def model_wrapper(request, model_name):
# ujson module ValueError
'Expected object or value' in e
):
return http.HttpResponse(
'Not a valid JSON response',
status=400
return http.HttpResponseBadRequest(
json.dumps({'error': 'Not a valid JSON response'}),
content_type='application/json; charset=UTF-8'
)
raise
except BAD_REQUEST_EXCEPTIONS as e:
return http.HttpResponseBadRequest(e)
except BAD_REQUEST_EXCEPTIONS as exception:
return http.HttpResponseBadRequest(
json.dumps({'error': str(exception)}),
content_type='application/json; charset=UTF-8'
)

# Some models allows to return a binary reponse. It does so based on
# the models `BINARY_RESPONSE` dict in which all keys and values
Expand Down

0 comments on commit e3895a3

Please sign in to comment.