Sanitise csv filenames before saving to S3

Commit 58cc1604a7 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.
This commit is contained in:
Katie Smith
2018-07-12 15:14:58 +01:00
parent 03b6bbdfc6
commit a5124f4af1
2 changed files with 47 additions and 3 deletions

View File

@@ -21,6 +21,7 @@ from notifications_utils.recipients import (
first_column_headings, first_column_headings,
optional_address_columns, optional_address_columns,
) )
from notifications_utils.sanitise_text import SanitiseASCII
from orderedset import OrderedSet from orderedset import OrderedSet
from werkzeug.routing import RequestRedirect from werkzeug.routing import RequestRedirect
from xlrd.biffh import XLRDError 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), row_errors=get_errors_for_csv(recipients, template.template_type),
count_of_recipients=len(recipients), count_of_recipients=len(recipients),
count_of_displayed_recipients=len(list(recipients.displayed_rows)), 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, upload_id=upload_id,
form=CsvUploadForm(), form=CsvUploadForm(),
remaining_messages=remaining_messages, 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) 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( set_metadata_on_csv_upload(
service_id, service_id,
upload_id, upload_id,
@@ -626,7 +629,7 @@ def check_messages(service_id, template_id, upload_id, row_index=2):
template_id=str(template_id), template_id=str(template_id),
valid=True, valid=True,
original_file_name=unicode_truncate( original_file_name=unicode_truncate(
request.args.get('original_file_name', ''), data['original_file_name'],
1600, 1600,
), ),
) )

View File

@@ -765,7 +765,48 @@ def test_file_name_truncated_to_fit_in_s3_metadata(
assert sys.getsizeof(''.join(( assert sys.getsizeof(''.join((
'{}{}'.format(key, value) for key, value in '{}{}'.format(key, value) for key, value in
mock_s3_set_metadata.call_args_list[0][1].items() 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( def test_show_all_columns_if_there_are_duplicate_recipient_columns(