From ece2f1f346ccf1821ce8afef9b6269159d29306c Mon Sep 17 00:00:00 2001 From: Tom Richards Date: Tue, 17 Dec 2024 17:19:39 +0000 Subject: [PATCH 1/5] limit file size on ingest to 500MB --- .../app/controllers/ImageLoaderController.scala | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/image-loader/app/controllers/ImageLoaderController.scala b/image-loader/app/controllers/ImageLoaderController.scala index 44bfb88bdb..725023183f 100644 --- a/image-loader/app/controllers/ImageLoaderController.scala +++ b/image-loader/app/controllers/ImageLoaderController.scala @@ -141,7 +141,18 @@ class ImageLoaderController(auth: Authentication, val approximateReceiveCount = getApproximateReceiveCount(sqsMessage) - if (approximateReceiveCount > 2) { + if(s3IngestObject.contentLength > 500000000){ // 500MB + val errorMessage = s"File size exceeds the maximum allowed size (500MB). Moving to fail bucket." + logger.warn(logMarker, errorMessage) + store.moveObjectToFailedBucket(s3IngestObject.key) + s3IngestObject.maybeMediaIdFromUiUpload foreach { imageId => + uploadStatusTable.updateStatus( // fire & forget, since there's nothing else we can do + imageId, UploadStatus(StatusType.Failed, Some(errorMessage)) + ) + } + Future.unit + } + else if (approximateReceiveCount > 2) { metrics.abandonedMessagesFromQueue.incrementBothWithAndWithoutDimensions(metricDimensions) val errorMessage = s"File processing has been attempted $approximateReceiveCount times. Moving to fail bucket." logger.warn(logMarker, errorMessage) From 7e98a00ffe054192f6dc0a37ca1788f034a578ba Mon Sep 17 00:00:00 2001 From: Tom Richards Date: Tue, 17 Dec 2024 22:32:59 +0000 Subject: [PATCH 2/5] skip files on UI upload which are bigger than 500MB (with alert to user) --- kahuna/public/js/upload/manager.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/kahuna/public/js/upload/manager.js b/kahuna/public/js/upload/manager.js index df45761ca6..e4baff2a8b 100644 --- a/kahuna/public/js/upload/manager.js +++ b/kahuna/public/js/upload/manager.js @@ -31,7 +31,18 @@ upload.factory('uploadManager', }; } - async function createJobItems(files){ + async function createJobItems(_files){ + + const filesAboveSizeLimit = _files.filter(file => file.size > 500000000); // 500MB + + if (filesAboveSizeLimit.length > 0){ + alert(`The following files will be skipped as they are above the size limit of 500MB:\n ${ + filesAboveSizeLimit.map(file => file.name).join("\n") + }`); + } + + const files = _files.filter(file => !filesAboveSizeLimit.includes(file)); + if (window._clientConfig.shouldUploadStraightToBucket) { const mediaIdToFileMap = Object.fromEntries( await Promise.all( From d815827999b687ccecb3fe823d3b3e9fcc24e979 Mon Sep 17 00:00:00 2001 From: Tom Richards Date: Wed, 18 Dec 2024 22:42:41 +0000 Subject: [PATCH 3/5] make upload size limit optional and configurable --- .../gu/mediaservice/lib/config/CommonConfig.scala | 2 ++ .../app/controllers/ImageLoaderController.scala | 4 ++-- kahuna/app/views/main.scala.html | 1 + kahuna/public/js/upload/manager.js | 13 ++++++++----- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index 7d72097b7d..ce25eec81e 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -54,6 +54,8 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientV1B val maybeIngestBucket: Option[String] = stringOpt("s3.ingest.bucket") val maybeFailBucket: Option[String] = stringOpt("s3.fail.bucket") + val maybeUploadLimitInBytes: Option[Int] = intOpt("upload.limit.mb").map(_ * 1_000_000) + // Note: had to make these lazy to avoid init order problems ;_; val domainRoot: String = string("domain.root") val domainRootOverride: Option[String] = stringOpt("domain.root-override") diff --git a/image-loader/app/controllers/ImageLoaderController.scala b/image-loader/app/controllers/ImageLoaderController.scala index 725023183f..14f311c62f 100644 --- a/image-loader/app/controllers/ImageLoaderController.scala +++ b/image-loader/app/controllers/ImageLoaderController.scala @@ -141,8 +141,8 @@ class ImageLoaderController(auth: Authentication, val approximateReceiveCount = getApproximateReceiveCount(sqsMessage) - if(s3IngestObject.contentLength > 500000000){ // 500MB - val errorMessage = s"File size exceeds the maximum allowed size (500MB). Moving to fail bucket." + if(config.maybeUploadLimitInBytes.exists(_ < s3IngestObject.contentLength)){ + val errorMessage = s"File size exceeds the maximum allowed size (${config.maybeUploadLimitInBytes.get / 1_000_000}MB). Moving to fail bucket." logger.warn(logMarker, errorMessage) store.moveObjectToFailedBucket(s3IngestObject.key) s3IngestObject.maybeMediaIdFromUiUpload foreach { imageId => diff --git a/kahuna/app/views/main.scala.html b/kahuna/app/views/main.scala.html index d579854688..c9878299b0 100644 --- a/kahuna/app/views/main.scala.html +++ b/kahuna/app/views/main.scala.html @@ -85,6 +85,7 @@ permissionsDefault: "@kahunaConfig.permissionsDefault", defaultShouldBlurGraphicImages: @kahunaConfig.defaultShouldBlurGraphicImages, shouldUploadStraightToBucket: @kahunaConfig.shouldUploadStraightToBucket, + maybeUploadLimitInBytes: @kahunaConfig.maybeUploadLimitInBytes, announcements: @Html(announcements), imageTypes: @Html(imageTypes), } diff --git a/kahuna/public/js/upload/manager.js b/kahuna/public/js/upload/manager.js index e4baff2a8b..568f87e807 100644 --- a/kahuna/public/js/upload/manager.js +++ b/kahuna/public/js/upload/manager.js @@ -33,15 +33,18 @@ upload.factory('uploadManager', async function createJobItems(_files){ - const filesAboveSizeLimit = _files.filter(file => file.size > 500000000); // 500MB + const maybeUploadLimitInBytes = window._clientConfig.maybeUploadLimitInBytes; + const maybeFilesAboveSizeLimit = maybeUploadLimitInBytes && _files.filter(file => file.size > maybeUploadLimitInBytes); - if (filesAboveSizeLimit.length > 0){ - alert(`The following files will be skipped as they are above the size limit of 500MB:\n ${ - filesAboveSizeLimit.map(file => file.name).join("\n") + if (maybeFilesAboveSizeLimit && maybeFilesAboveSizeLimit.length > 0){ + alert(`The following files will be skipped as they are above the size limit of ${maybeUploadLimitInBytes / 1_000_000}MB:\n${ + maybeFilesAboveSizeLimit.map(file => file.name).join("\n") }`); } - const files = _files.filter(file => !filesAboveSizeLimit.includes(file)); + const files = maybeFilesAboveSizeLimit && maybeFilesAboveSizeLimit.length > 0 + ? _files.filter(file => !maybeFilesAboveSizeLimit.includes(file)) + : _files; if (window._clientConfig.shouldUploadStraightToBucket) { const mediaIdToFileMap = Object.fromEntries( From 064daab40a8782c6f0dd08e27f31945614d242ff Mon Sep 17 00:00:00 2001 From: Tom Richards Date: Mon, 23 Dec 2024 15:04:15 +0000 Subject: [PATCH 4/5] ensure failure metric for oversized images which are rejected Co-authored-by: Pete F <37048459+bryophyta@users.noreply.github.com> --- image-loader/app/controllers/ImageLoaderController.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/image-loader/app/controllers/ImageLoaderController.scala b/image-loader/app/controllers/ImageLoaderController.scala index 14f311c62f..b43f47e1e0 100644 --- a/image-loader/app/controllers/ImageLoaderController.scala +++ b/image-loader/app/controllers/ImageLoaderController.scala @@ -150,6 +150,7 @@ class ImageLoaderController(auth: Authentication, imageId, UploadStatus(StatusType.Failed, Some(errorMessage)) ) } + metrics.failedIngestsFromQueue.incrementBothWithAndWithoutDimensions(metricDimensions) Future.unit } else if (approximateReceiveCount > 2) { From 8ab87d915f90a763b5c766a5f118e7eebb982065 Mon Sep 17 00:00:00 2001 From: Tom Richards Date: Mon, 23 Dec 2024 15:05:08 +0000 Subject: [PATCH 5/5] cleaner type for `maybeFilesAboveSizeLimit` Co-authored-by: Pete F <37048459+bryophyta@users.noreply.github.com> --- kahuna/public/js/upload/manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kahuna/public/js/upload/manager.js b/kahuna/public/js/upload/manager.js index 568f87e807..18b40deeb4 100644 --- a/kahuna/public/js/upload/manager.js +++ b/kahuna/public/js/upload/manager.js @@ -34,7 +34,7 @@ upload.factory('uploadManager', async function createJobItems(_files){ const maybeUploadLimitInBytes = window._clientConfig.maybeUploadLimitInBytes; - const maybeFilesAboveSizeLimit = maybeUploadLimitInBytes && _files.filter(file => file.size > maybeUploadLimitInBytes); + const maybeFilesAboveSizeLimit = !!maybeUploadLimitInBytes && _files.filter(file => file.size > maybeUploadLimitInBytes); if (maybeFilesAboveSizeLimit && maybeFilesAboveSizeLimit.length > 0){ alert(`The following files will be skipped as they are above the size limit of ${maybeUploadLimitInBytes / 1_000_000}MB:\n${