Merge pull request #2162 from alphagov/sanitize-csv-filenames

Sanitise csv filenames before saving to S3

app/main/views/send.py

@@ -21,6 +21,7 @@ from notifications_utils.recipients import (
     first_column_headings,
     optional_address_columns,
 )
+from notifications_utils.sanitise_text import SanitiseASCII
 from orderedset import OrderedSet
 from werkzeug.routing import RequestRedirect
 from xlrd.biffh import XLRDError
@@ -576,7 +577,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_
         row_errors=get_errors_for_csv(recipients, template.template_type),
         count_of_recipients=len(recipients),
         count_of_displayed_recipients=len(list(recipients.displayed_rows)),
-        original_file_name=request.args.get('original_file_name'),
+        original_file_name=request.args.get('original_file_name', ''),
         upload_id=upload_id,
         form=CsvUploadForm(),
         remaining_messages=remaining_messages,
@@ -619,6 +620,8 @@ def check_messages(service_id, template_id, upload_id, row_index=2):
     ):
         return render_template('views/check/column-errors.html', **data)
 
+    data['original_file_name'] = SanitiseASCII.encode(data.get('original_file_name', ''))
+
     set_metadata_on_csv_upload(
         service_id,
         upload_id,
@@ -626,7 +629,7 @@ def check_messages(service_id, template_id, upload_id, row_index=2):
         template_id=str(template_id),
         valid=True,
         original_file_name=unicode_truncate(
-            request.args.get('original_file_name', ''),
+            data['original_file_name'],
             1600,
         ),
     )

tests/app/main/views/test_send.py

@@ -765,7 +765,48 @@ def test_file_name_truncated_to_fit_in_s3_metadata(
     assert sys.getsizeof(''.join((
         '{}{}'.format(key, value) for key, value in
         mock_s3_set_metadata.call_args_list[0][1].items()
-    )).encode('utf-8')) == 1724
+    )).encode('utf-8')) == 1726
+
+
+def test_check_messages_replaces_invalid_characters_in_file_name(
+    client_request,
+    mocker,
+    mock_get_live_service,
+    mock_get_service_template_with_placeholders,
+    mock_get_users_by_service,
+    mock_get_service_statistics,
+    mock_get_job_doesnt_exist,
+    mock_s3_set_metadata,
+    fake_uuid,
+):
+    with client_request.session_transaction() as session:
+        session['file_uploads'] = {
+            fake_uuid: {'template_id': fake_uuid}
+        }
+
+    mocker.patch('app.main.views.send.s3download', return_value="""
+        phone number,name,thing,thing,thing
+        07700900001, A,   foo,  foo,  foo
+    """)
+
+    file_name = 'ü😁’€'
+
+    client_request.get(
+        'main.check_messages',
+        service_id=SERVICE_ONE_ID,
+        template_id=fake_uuid,
+        upload_id=fake_uuid,
+        original_file_name=file_name,
+    )
+
+    mock_s3_set_metadata.assert_called_once_with(
+        SERVICE_ONE_ID,
+        fake_uuid,
+        notification_count=1,
+        original_file_name="u?'?",
+        template_id=fake_uuid,
+        valid=True
+    )
 
 
 def test_show_all_columns_if_there_are_duplicate_recipient_columns(