diff --git a/app/s3_client/s3_letter_upload_client.py b/app/s3_client/s3_letter_upload_client.py index e5ddad695..0f2e5a587 100644 --- a/app/s3_client/s3_letter_upload_client.py +++ b/app/s3_client/s3_letter_upload_client.py @@ -21,10 +21,12 @@ def upload_letter_to_s3( invalid_pages=None, recipient=None ): + # Use of urllib.parse.quote encodes metadata into ascii, which is required by s3. + # Making sure data for displaying to users is decoded is taken care of by LetterMetadata metadata = { 'status': status, 'page_count': str(page_count), - 'filename': filename, + 'filename': urllib.parse.quote(filename), } if message: metadata['message'] = message @@ -42,18 +44,8 @@ def upload_letter_to_s3( ) -def get_letter_pdf_and_metadata(service_id, file_id): - file_location = get_transient_letter_file_location(service_id, file_id) - s3 = resource('s3') - s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get() - - pdf = s3_object['Body'].read() - - return pdf, LetterMetadata(s3_object['Metadata']) - - class LetterMetadata: - KEYS_TO_DECODE = [] + KEYS_TO_DECODE = ["filename"] def __init__(self, metadata): self._metadata = metadata @@ -64,6 +56,16 @@ class LetterMetadata: return self._metadata.get(key, default) +def get_letter_pdf_and_metadata(service_id, file_id): + file_location = get_transient_letter_file_location(service_id, file_id) + s3 = resource('s3') + s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get() + + pdf = s3_object['Body'].read() + + return pdf, LetterMetadata(s3_object['Metadata']) + + def get_letter_metadata(service_id, file_id): file_location = get_transient_letter_file_location(service_id, file_id) s3 = resource('s3') diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index ad2cb440e..b9ac34222 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -457,9 +457,11 @@ def test_uploaded_letter_preview( mocker.patch('app.main.views.uploads.service_api_client') recipient = 'Bugs Bunny\n123 Big Hole\rLooney Town' mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({ - 'filename': 'my_letter.pdf', 'page_count': '1', 'status': 'valid', + 'filename': 'my_encoded_filename%C2%A3.pdf', + 'page_count': '1', + 'status': 'valid', 'recipient': urllib.parse.quote(recipient) - })) + })) service_one['restricted'] = False client_request.login(active_user_with_permissions, service=service_one) @@ -470,7 +472,7 @@ def test_uploaded_letter_preview( file_id=fake_uuid, ) - assert page.find('h1').text == 'my_letter.pdf' + assert page.find('h1').text == 'my_encoded_filename£.pdf' assert page.find('div', class_='letter-sent') assert not page.find("label", {"class": "file-upload-button"}) assert page.find('button', {'class': 'page-footer__button', 'type': 'submit'})