From 7c6ae4003439a8478c64f87005fab394b875b05c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 22 Apr 2021 14:42:54 +0100 Subject: [PATCH] Normalise broadcast content before validating length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This changes the content length validation of the internal API to match the validation of the public broadcast API[1]. This removes the length check from JSONSchema, which isn’t sophisticated enough to deal with things like normalising newlines or handling different encodings. The admin app should catch these errors before they’re raised here, but it’s best to be belt and braces. 1.https://github.com/alphagov/notifications-api/blob/7ab0403ae7ab9c9a17b3fb6595c8c8d2f8b32cbb/app/v2/broadcast/post_broadcast.py#L53-L63 --- .../broadcast_message_schema.py | 2 +- app/broadcast_message/rest.py | 14 ++++++++ tests/app/broadcast_message/test_rest.py | 34 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/app/broadcast_message/broadcast_message_schema.py b/app/broadcast_message/broadcast_message_schema.py index 09eff203b..fc4527511 100644 --- a/app/broadcast_message/broadcast_message_schema.py +++ b/app/broadcast_message/broadcast_message_schema.py @@ -15,7 +15,7 @@ create_broadcast_message_schema = { 'finishes_at': {'type': 'string', 'format': 'datetime'}, 'areas': {"type": "array", "items": {"type": "string"}}, 'simple_polygons': {"type": "array", "items": {"type": "array"}}, - 'content': {'type': 'string', 'minLength': 1, 'maxLength': 1395}, + 'content': {'type': 'string', 'minLength': 1}, 'reference': {'type': 'string', 'minLength': 1, 'maxLength': 255}, }, 'required': ['service_id', 'created_by'], diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py index 31e3a5a4d..26221b89a 100644 --- a/app/broadcast_message/rest.py +++ b/app/broadcast_message/rest.py @@ -2,6 +2,7 @@ from datetime import datetime import iso8601 from flask import Blueprint, current_app, jsonify, request +from notifications_utils.template import BroadcastMessageTemplate from app.broadcast_message.broadcast_message_schema import ( create_broadcast_message_schema, @@ -116,6 +117,19 @@ def create_broadcast_message(service_id): reference = None else: template, content, reference = None, data['content'], data['reference'] + temporary_template = BroadcastMessageTemplate.from_content(content) + if temporary_template.content_too_long: + raise InvalidRequest( + ( + f'Content must be ' + f'{temporary_template.max_content_count:,.0f} ' + f'characters or fewer' + ) + ( + ' (because it could not be GSM7 encoded)' + if temporary_template.non_gsm_characters else '' + ), + status_code=400, + ) broadcast_message = BroadcastMessage( service_id=service.id, diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py index 49550c515..438aedaf4 100644 --- a/tests/app/broadcast_message/test_rest.py +++ b/tests/app/broadcast_message/test_rest.py @@ -192,6 +192,40 @@ def test_create_broadcast_message_400s_if_json_schema_fails_validation( assert response['errors'] == expected_errors +@pytest.mark.parametrize('content, expected_status, expected_errors', ( + ('a', 201, None), + ('a' * 1_395, 201, None), + ('a\r\n' * 697, 201, None), # 1,394 chars – new lines normalised to \n + ('a' * 1_396, 400, ( + 'Content must be 1,395 characters or fewer' + )), + ('ŵ' * 615, 201, None), + ('ŵ' * 616, 400, ( + 'Content must be 615 characters or fewer ' + '(because it could not be GSM7 encoded)' + )), +)) +def test_create_broadcast_message_400s_if_content_too_long( + admin_request, + sample_broadcast_service, + content, + expected_status, + expected_errors, +): + response = admin_request.post( + 'broadcast_message.create_broadcast_message', + service_id=sample_broadcast_service.id, + _data={ + 'service_id': str(sample_broadcast_service.id), + 'created_by': str(sample_broadcast_service.created_by_id), + 'reference': 'abc123', + 'content': content, + }, + _expected_status=expected_status, + ) + assert response.get('message') == expected_errors + + @freeze_time('2020-01-01') def test_create_broadcast_message_can_be_created_from_content(admin_request, sample_broadcast_service): response = admin_request.post(