Merge pull request #4320 from overleaf/hb-handle-invalid-filenames-upload

Handle invalid filenames in upload modal

GitOrigin-RevId: f3438d8653cf29ef9536a71692c03b5802d90a6d

services/web/app/src/Features/Uploads/ProjectUploadController.js

@@ -77,7 +77,10 @@ module.exports = ProjectUploadController = {
         { projectId: project_id, fileName: name },
         'bad name when trying to upload file'
       )
-      return res.send({ success: false })
+      return res.status(422).send({
+        success: false,
+        error: 'invalid_filename',
+      })
     }
     const user_id = AuthenticationController.getLoggedInUserId(req)
 
@@ -103,17 +106,17 @@ module.exports = ProjectUploadController = {
             'error uploading file'
           )
           if (error.name === 'InvalidNameError') {
-            return res.send({
+            return res.status(422).send({
               success: false,
-              error: req.i18n.translate('invalid_filename'),
+              error: 'invalid_filename',
             })
           } else if (error.message === 'project_has_too_many_files') {
-            return res.send({
+            return res.status(422).send({
               success: false,
-              error: req.i18n.translate('project_has_too_many_files'),
+              error: 'project_has_too_many_files',
             })
           } else {
-            return res.send({ success: false })
+            return res.status(422).send({ success: false })
           }
         } else {
           return res.send({

services/web/frontend/extracted-translations.json

@@ -158,6 +158,7 @@
   "importing_and_merging_changes_in_github": "",
   "invalid_email": "",
   "invalid_file_name": "",
+  "invalid_filename": "",
   "invalid_request": "",
   "invite_not_accepted": "",
   "learn_how_to_make_documents_compile_quickly": "",
@@ -204,9 +205,9 @@
   "no_new_commits_in_github": "",
   "no_other_projects_found": "",
   "no_pdf_error_explanation": "",
-  "no_pdf_error_reason_unrecoverable_error": "",
   "no_pdf_error_reason_no_content": "",
   "no_pdf_error_reason_output_pdf_already_exists": "",
+  "no_pdf_error_reason_unrecoverable_error": "",
   "no_pdf_error_title": "",
   "no_preview_available": "",
   "no_search_results": "",
@@ -347,4 +348,4 @@
   "zotero_reference_loading_error_expired": "",
   "zotero_reference_loading_error_forbidden": "",
   "zotero_sync_description": ""
-}
+}

services/web/frontend/js/features/file-tree/components/file-tree-create/error-message.js

@@ -1,5 +1,5 @@
 import PropTypes from 'prop-types'
-import { useTranslation } from 'react-i18next'
+import { useTranslation, Trans } from 'react-i18next'
 import { FetchError } from '../../../../infrastructure/fetch-json'
 import RedirectToLogin from './redirect-to-login'
 import {
@@ -11,6 +11,7 @@ import DangerMessage from './danger-message'
 
 export default function ErrorMessage({ error }) {
   const { t } = useTranslation()
+  const fileNameLimit = 150
 
   // the error is a string
   // TODO: translate? always? is this a key or a message?
@@ -25,6 +26,18 @@ export default function ErrorMessage({ error }) {
       case 'remote-service-error':
         return <DangerMessage>{t('remote_service_error')}</DangerMessage>
 
+      case 'invalid_filename':
+        return (
+          <DangerMessage>
+            <Trans
+              i18nKey="invalid_filename"
+              values={{
+                nameLimit: fileNameLimit,
+              }}
+            />
+          </DangerMessage>
+        )
+
       case 'rate-limit-hit':
         return (
           <DangerMessage>

services/web/frontend/js/features/file-tree/components/file-tree-create/modes/file-tree-upload-doc.js

@@ -105,7 +105,7 @@ export default function FileTreeUploadDoc() {
               break
 
             default:
-              // TODO
+              setError(response.body.error)
               break
           }
         })

services/web/locales/en.json

@@ -65,7 +65,7 @@
   "you_can_opt_in_and_out_of_the_program_at_any_time_on_this_page": "You can opt in and out of the program at any time on this page",
   "give_feedback": "Give feedback",
   "beta_feature_badge": "Beta feature badge",
-  "invalid_filename": "Upload failed: check that the file name doesn't contain special characters or trailing/leading whitespace",
+  "invalid_filename": "Upload failed: check that the file name doesn't contain special characters, trailing/leading whitespace or more than __nameLimit__ characters",
   "clsi_unavailable": "Sorry, the compile server for your project was temporarily unavailable. Please try again in a few moments.",
   "x_price_per_month": "<0>__price__</0> per month",
   "x_price_per_year": "<0>__price__</0> per year",

services/web/test/frontend/features/file-tree/components/file-tree-create/file-tree-modal-create-file.test.js

@@ -18,7 +18,12 @@ import { useFileTreeActionable } from '../../../../../../frontend/js/features/fi
 import { useFileTreeMutable } from '../../../../../../frontend/js/features/file-tree/contexts/file-tree-mutable'
 
 describe('<FileTreeModalCreateFile/>', function () {
+  beforeEach(function () {
+    window.csrfToken = 'token'
+  })
+
   afterEach(function () {
+    delete window.csrfToken
     fetchMock.restore()
     cleanup()
   })
@@ -400,6 +405,51 @@ describe('<FileTreeModalCreateFile/>', function () {
 
     xhr.restore()
   })
+
+  it('displays upload errors', async function () {
+    const xhr = sinon.useFakeXMLHttpRequest()
+    const requests = []
+    xhr.onCreate = request => {
+      requests.push(request)
+    }
+
+    render(
+      <FileTreeContext {...contextProps}>
+        <OpenWithMode mode="upload" />
+      </FileTreeContext>
+    )
+
+    // the submit button should not be present
+    expect(screen.queryByRole('button', { name: 'Create' })).to.be.null
+
+    const dropzone = screen.getByLabelText('File Uploader')
+
+    expect(dropzone).not.to.be.null
+
+    fireEvent.paste(dropzone, {
+      clipboardData: {
+        files: [new File(['test'], 'tes!t.tex', { type: 'text/plain' })],
+      },
+    })
+
+    await waitFor(() => expect(requests).to.have.length(1))
+
+    const [request] = requests
+    expect(request.url).to.equal('/project/test-project/upload')
+    expect(request.method).to.equal('POST')
+
+    request.respond(
+      422,
+      { 'Content-Type': 'application/json' },
+      '{ "success": false, "error": "invalid_filename" }'
+    )
+
+    await screen.findByText(
+      `Upload failed: check that the file name doesn't contain special characters, trailing/leading whitespace or more than 150 characters`
+    )
+
+    xhr.restore()
+  })
 })
 
 function OpenWithMode({ mode }) {

services/web/test/unit/src/Uploads/ProjectUploadControllerTests.js

@@ -248,7 +248,7 @@ describe('ProjectUploadController', function () {
       })
     })
 
-    describe('with a bad request', function () {
+    describe('with an invalid filename', function () {
       beforeEach(function () {
         this.req.file.originalname = ''
         return this.ProjectUploadController.uploadFile(this.req, this.res)
@@ -257,6 +257,7 @@ describe('ProjectUploadController', function () {
       it('should return a a non success response', function () {
         return expect(this.res.body).to.deep.equal({
           success: false,
+          error: 'invalid_filename',
         })
       })
     })