From a5124f4af1d1a850ae3a56050f1433404790cbc7 Mon Sep 17 00:00:00 2001
From: Katie Smith <katie.smith@digital.cabinet-office.gov.uk>
Date: Thu, 12 Jul 2018 15:14:58 +0100
Subject: [PATCH] Sanitise csv filenames before saving to S3

Commit 58cc1604a73be583f9d96513dbe67e77f326e010 sanitises any non-ascii
characters in the headers. CSV filenames get used as a header value, so
this fixed a bug that occurred when non-ascii characters were used.

The CSV filename also gets used as part of the metadata when uploading
the file to S3. Since the S3 metadata can only contain ASII characters,
we also need to sanitise the filename before uploading it to S3.
---
 app/main/views/send.py            |  7 +++--
 tests/app/main/views/test_send.py | 43 ++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/app/main/views/send.py b/app/main/views/send.py
index 9158b2a0e..90c75d01b 100644
--- a/app/main/views/send.py
+++ b/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,
         ),
     )
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
index 93f2bb78b..6f0ff50bb 100644
--- a/tests/app/main/views/test_send.py
+++ b/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(