mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-08-20 22:40:31 -04:00
Reject CSV / Spreadsheet files larger than 10Mb
This is a quick additional check to protect the user: - From getting a CloudFront 502 error if the file takes too long to upload. I was surprised to find it takes about 1 minute to upload a 70Mb file to S3.* - From getting a CloudFront 502 error when we follow the redirect and run through the slow processing code in utils that builds a RecipientCSV [1]. For context, a CSV with 100K rows and a few columns is around 5Mb, so a 10Mb limit should be enough. Analysis over the past week shows that the vast majority of CSV uploads are actually < 2.5Mb. I haven't added any tests for this because: - The check isn't critical, as the worst case scenario is the user gets a worse error than this in-app one. - There's no easy way to mock the validation, and I didn't want to have a test that depends on a 10Mb+ file. *We're using "key.put" to upload the file, when we could be doing a multipart upload [2]. However, I tried this myself with a chunk size of 1000 bytes and found it only led to a marginal improvement. [1]: https://github.com/alphagov/notifications-utils/pull/930 [2]: https://boto3.amazonaws.com/v1/documentation/api/latest/guide/s3-uploading-files.html
This commit is contained in:
@@ -5,6 +5,7 @@ from glob import glob
|
||||
from io import BytesIO
|
||||
from itertools import repeat
|
||||
from os import path
|
||||
from random import randbytes
|
||||
from unittest.mock import ANY
|
||||
from uuid import uuid4
|
||||
from zipfile import BadZipFile
|
||||
@@ -966,6 +967,25 @@ def test_upload_csv_invalid_extension(
|
||||
assert "invalid.txt is not a spreadsheet that Notify can read" in resp.get_data(as_text=True)
|
||||
|
||||
|
||||
def test_upload_csv_size_too_big(
|
||||
logged_in_client,
|
||||
mock_login,
|
||||
service_one,
|
||||
mock_get_service_template,
|
||||
fake_uuid,
|
||||
):
|
||||
|
||||
resp = logged_in_client.post(
|
||||
url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid),
|
||||
data={'file': (BytesIO(randbytes(11_000_000)), 'invalid.csv')},
|
||||
content_type='multipart/form-data',
|
||||
follow_redirects=True
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert "File must be smaller than 10Mb" in resp.get_data(as_text=True)
|
||||
|
||||
|
||||
def test_upload_valid_csv_redirects_to_check_page(
|
||||
client_request,
|
||||
mock_get_service_template_with_placeholders,
|
||||
|
||||
Reference in New Issue
Block a user