From 736055caeeb0e301e416b87269bdba0cfa50a368 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Thu, 8 Oct 2020 12:18:29 +0200 Subject: [PATCH] apps: do not crash when request body is not json (#47452) --- combo/apps/dashboard/views.py | 6 +++++- combo/apps/lingo/views.py | 11 +++++++++-- combo/apps/pwa/views.py | 9 +++++++-- tests/test_dashboard.py | 5 +++++ tests/test_lingo_payment.py | 13 +++++++++++++ tests/test_pwa.py | 4 ++++ 6 files changed, 43 insertions(+), 5 deletions(-) diff --git a/combo/apps/dashboard/views.py b/combo/apps/dashboard/views.py index b7c6ad2b..428b297d 100644 --- a/combo/apps/dashboard/views.py +++ b/combo/apps/dashboard/views.py @@ -114,13 +114,17 @@ def dashboard_auto_tile(request, *args, **kwargs): if request.method != 'POST': return HttpResponseNotAllowed(['post']) + try: + request_body = json.loads(force_text(request.body)) + except json.JSONDecodeError: + return HttpResponseBadRequest('bad json request: "%s"' % request.body) + dashboard = DashboardCell.objects.filter(page__snapshot__isnull=True)[0] cell = ConfigJsonCell(key=kwargs.get('key'), order=1, page_id=dashboard.page_id, placeholder='_auto_tile') # only keep parameters that are actually defined for this cell type. cell.parameters = {} - request_body = json.loads(force_text(request.body)) for field in settings.JSON_CELL_TYPES[cell.key].get('form') or []: key = field['varname'] cell.parameters[key] = request_body.get(key) diff --git a/combo/apps/lingo/views.py b/combo/apps/lingo/views.py index 9a1b191c..b19d77cf 100644 --- a/combo/apps/lingo/views.py +++ b/combo/apps/lingo/views.py @@ -136,7 +136,11 @@ class AddBasketItemApiView(View): if not lingo_check_request_signature(request): return HttpResponseForbidden() - request_body = json.loads(force_text(self.request.body)) + try: + request_body = json.loads(force_text(request.body)) + except json.JSONDecodeError: + return BadRequestJsonResponse('bad json request: "%s"' % request.body) + extra = request_body.get('extra', {}) if 'amount' not in request.GET and not 'amount' in request_body and \ @@ -237,7 +241,10 @@ class RemoveBasketItemApiView(View): if not lingo_check_request_signature(request): return HttpResponseForbidden() - request_body = json.loads(force_text(self.request.body)) + try: + request_body = json.loads(force_text(request.body)) + except json.JSONDecodeError: + return BadRequestJsonResponse('bad json request: "%s"' % request.body) if 'basket_item_id' not in request_body: return BadRequestJsonResponse('missing basket_item_id parameter') diff --git a/combo/apps/pwa/views.py b/combo/apps/pwa/views.py index 0d0493b6..db95ded8 100644 --- a/combo/apps/pwa/views.py +++ b/combo/apps/pwa/views.py @@ -19,7 +19,7 @@ import json from django.conf import settings -from django.http import HttpResponse, HttpResponseForbidden, Http404, JsonResponse +from django.http import HttpResponse, HttpResponseForbidden, Http404, JsonResponse, HttpResponseBadRequest from django.template.loader import get_template, TemplateDoesNotExist from django.utils.encoding import force_text, force_bytes from django.views.decorators.csrf import csrf_exempt @@ -87,7 +87,12 @@ def subscribe_push(request, *args, **kwargs): return HttpResponseForbidden() if request.method != 'POST': return HttpResponseForbidden() - subscription_data = json.loads(force_text(request.body)) + + try: + subscription_data = json.loads(force_text(request.body)) + except json.JSONDecodeError: + return HttpResponseBadRequest('bad json request: "%s"' % request.body) + if subscription_data is None: PushSubscription.objects.filter(user=request.user).delete() else: diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index 636e7260..df1a0eb2 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -217,6 +217,11 @@ def test_auto_tile(app, site): # and with a GET instead of POST resp = app.get(reverse('combo-dashboard-auto-tile', kwargs={'key': 'test-config-json-cell'}), status=405) + # bad json + resp = app.post( + reverse('combo-dashboard-auto-tile', kwargs={'key': 'test-config-json-cell'}), + params='', + status=400) def test_clean_autotiles(app, site): diff --git a/tests/test_lingo_payment.py b/tests/test_lingo_payment.py index 48c2645a..2ad12d6f 100644 --- a/tests/test_lingo_payment.py +++ b/tests/test_lingo_payment.py @@ -274,6 +274,13 @@ def test_add_amount_to_basket(app, key, regie, user_name_id): resp = app.post_json(url, params=data, status=400) assert 'unknown user' in resp.text + amount = 42 + data['amount'] = amount + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), 'unknown_id') + url = sign_url(url, key) + resp = app.post(url, params='', status=400) + assert 'bad json' in resp.text + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) resp = app.post_json(url, params=data) @@ -659,6 +666,12 @@ def test_cancel_basket_item(app, key, regie, user_name_id): resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'missing basket_item_id parameter' + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_name_id) + url = sign_url(url, key) + data = {'notify': 'true'} + resp = app.post(url, params='', status=400) + assert 'bad json' in resp.json['err_desc'] + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_name_id) url = sign_url(url, key) data = {'basket_item_id': 'eggs', 'notify': 'true'} diff --git a/tests/test_pwa.py b/tests/test_pwa.py index 35baabcb..86a06d87 100644 --- a/tests/test_pwa.py +++ b/tests/test_pwa.py @@ -66,6 +66,10 @@ def test_webpush_subscription(app, john_doe, jane_doe): app.post_json(reverse('pwa-subscribe-push'), params=None, status=200) assert PushSubscription.objects.count() == 1 + app = login(app, john_doe.username, john_doe.username) + resp = app.post(reverse('pwa-subscribe-push'), params='', status=400) + assert 'bad json' in resp.text + def test_webpush_notification(app, john_doe): PushSubscription.objects.all().delete() app = login(app, john_doe.username, john_doe.username)