From 082121d3dacd5e8da9f9c972cc7cce32f335d3c1 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 21 Jul 2025 16:02:43 +0200 Subject: [PATCH] [web] reject upload requests without a file path (#27156) * [web] reject upload requests without a file path * [web] update copy on error message and link to contact form Co-authored-by: Kamal Arkinstall * [web] update copy: move dot to the end --------- Co-authored-by: Kamal Arkinstall GitOrigin-RevId: ba1ee81a91b046540caeb2f3f3da0e305611b35f --- .../Uploads/ProjectUploadController.mjs | 12 ++++-- .../web/frontend/extracted-translations.json | 1 + .../file-tree-create/error-message.tsx | 20 ++++++++- services/web/locales/en.json | 1 + .../acceptance/src/ProjectStructureTests.mjs | 41 +++++++++++++++++++ 5 files changed, 71 insertions(+), 4 deletions(-) diff --git a/services/web/app/src/Features/Uploads/ProjectUploadController.mjs b/services/web/app/src/Features/Uploads/ProjectUploadController.mjs index a3bc434ed7..84b8738af3 100644 --- a/services/web/app/src/Features/Uploads/ProjectUploadController.mjs +++ b/services/web/app/src/Features/Uploads/ProjectUploadController.mjs @@ -66,7 +66,7 @@ function uploadProject(req, res, next) { async function uploadFile(req, res, next) { const timer = new metrics.Timer('file-upload') const name = req.body.name - const path = req.file?.path + const { path } = req.file const projectId = req.params.Project_id const userId = SessionManager.getLoggedInUserId(req.session) let { folder_id: folderId } = req.query @@ -162,8 +162,14 @@ function multerMiddleware(req, res, next) { .status(422) .json({ success: false, error: req.i18n.translate('file_too_large') }) } - - return next(err) + if (err) return next(err) + if (!req.file?.path) { + logger.info({ req }, 'missing req.file.path on upload') + return res + .status(400) + .json({ success: false, error: 'invalid_upload_request' }) + } + next() }) } diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 2775c04601..639c9fcdfc 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -867,6 +867,7 @@ "invalid_password_too_similar": "", "invalid_regular_expression": "", "invalid_request": "", + "invalid_upload_request": "", "invite": "", "invite_expired": "", "invite_more_collabs": "", diff --git a/services/web/frontend/js/features/file-tree/components/file-tree-create/error-message.tsx b/services/web/frontend/js/features/file-tree/components/file-tree-create/error-message.tsx index 02cc083928..244ef1a76b 100644 --- a/services/web/frontend/js/features/file-tree/components/file-tree-create/error-message.tsx +++ b/services/web/frontend/js/features/file-tree/components/file-tree-create/error-message.tsx @@ -1,4 +1,4 @@ -import { useTranslation } from 'react-i18next' +import { useTranslation, Trans } from 'react-i18next' import { FetchError } from '../../../../infrastructure/fetch-json' import RedirectToLogin from './redirect-to-login' import { @@ -7,6 +7,7 @@ import { InvalidFilenameError, } from '../../errors' import DangerMessage from './danger-message' +import getMeta from '@/utils/meta' // TODO: Update the error type when we properly type FileTreeActionableContext export default function ErrorMessage({ @@ -15,6 +16,7 @@ export default function ErrorMessage({ error: string | Record }) { const { t } = useTranslation() + const { isOverleaf } = getMeta('ol-ExposedSettings') const fileNameLimit = 150 // the error is a string @@ -46,6 +48,22 @@ export default function ErrorMessage({ ) + case 'invalid_upload_request': + if (!isOverleaf) { + return ( + {t('generic_something_went_wrong')} + ) + } + return ( + + ]} + /> + + ) + case 'duplicate_file_name': return ( diff --git a/services/web/locales/en.json b/services/web/locales/en.json index ddaeb8e79f..4df2e389cd 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -1111,6 +1111,7 @@ "invalid_password_too_similar": "Password is too similar to parts of email address", "invalid_regular_expression": "Invalid regular expression", "invalid_request": "Invalid Request. Please correct the data and try again.", + "invalid_upload_request": "The upload failed. If the problem persists, <0>let us know.", "invalid_zip_file": "Invalid zip file", "invite": "Invite", "invite_expired": "The invite may have expired", diff --git a/services/web/test/acceptance/src/ProjectStructureTests.mjs b/services/web/test/acceptance/src/ProjectStructureTests.mjs index a1f48a8448..d87daba5f5 100644 --- a/services/web/test/acceptance/src/ProjectStructureTests.mjs +++ b/services/web/test/acceptance/src/ProjectStructureTests.mjs @@ -138,6 +138,47 @@ describe('ProjectStructureChanges', function () { }) }) + describe('when sending an upload request without a file', function () { + describe('project', function () { + it('should reject the request with status 400', async function () { + const { response, body } = await owner.doRequest('POST', { + uri: 'project/new/upload', + json: true, + formData: { + name: 'foo', + }, + }) + + expect(response.statusCode).to.equal(400) + expect(body).to.deep.equal({ + success: false, + error: 'invalid_upload_request', + }) + }) + }) + + describe('file', function () { + it('should reject the request with status 400', async function () { + const projectId = await owner.createProject('foo', { + template: 'blank', + }) + const { response, body } = await owner.doRequest('POST', { + uri: `project/${projectId}/upload`, + json: true, + formData: { + name: 'foo.txt', + }, + }) + + expect(response.statusCode).to.equal(400) + expect(body).to.deep.equal({ + success: false, + error: 'invalid_upload_request', + }) + }) + }) + }) + describe('uploading an empty zipfile', function () { let res