Merge pull request #4795 from overleaf/jel-project-title-regex

Escape project title for regular expression

GitOrigin-RevId: 495b96720de6d09cda905edc6464b55a5c85e21d

services/web/app/src/Features/Project/ProjectHelper.js

@@ -141,7 +141,8 @@ function _addNumericSuffixToProjectName(name, allProjectNames, maxLength) {
     n = parseInt(match[1])
   }
 
-  const prefixMatcher = new RegExp(`^${basename} \\(\\d+\\)$`)
+  const prefixMatcher = new RegExp(`^${_.escapeRegExp(basename)} \\(\\d+\\)$`)
+
   const projectNamesWithSamePrefix = Array.from(allProjectNames).filter(name =>
     prefixMatcher.test(name)
   )

services/web/test/acceptance/src/ProjectDuplicateNameTests.js

@@ -1,33 +1,10 @@
-/* eslint-disable
-    camelcase,
-    node/handle-callback-err,
-    max-len,
-    mocha/no-identical-title,
-    no-path-concat,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const async = require('async')
 const { expect } = require('chai')
 const sinon = require('sinon')
-const mkdirp = require('mkdirp')
 const Path = require('path')
 const fs = require('fs')
-const Settings = require('@overleaf/settings')
 const _ = require('underscore')
-
-const ProjectGetter = require('../../../app/src/Features/Project/ProjectGetter.js')
-
-const request = require('./helpers/request')
 const User = require('./helpers/User')
+const UserHelper = require('./helpers/UserHelper')
 
 const MockDocstoreApiClass = require('./mocks/MockDocstoreApi')
 const MockFilestoreApiClass = require('./mocks/MockFilestoreApi')
@@ -44,20 +21,20 @@ describe('ProjectDuplicateNames', function () {
     this.owner = new User()
     this.owner.login(done)
     this.project = {}
-    return (this.callback = sinon.stub())
+    this.callback = sinon.stub()
   })
 
   describe('creating a project from the example template', function () {
     beforeEach(function (done) {
-      return this.owner.createProject(
+      this.owner.createProject(
         'example-project',
         { template: 'example' },
-        (error, project_id) => {
-          if (error != null) {
+        (error, projectId) => {
+          if (error) {
             throw error
           }
-          this.example_project_id = project_id
-          return this.owner.getProject(project_id, (error, project) => {
+          this.example_project_id = projectId
+          this.owner.getProject(projectId, (error, project) => {
             this.project = project
             this.mainTexDoc = _.find(
               project.rootFolder[0].docs,
@@ -73,7 +50,7 @@ describe('ProjectDuplicateNames', function () {
             )
             this.rootFolderId = project.rootFolder[0]._id.toString()
             // create a folder called 'testfolder'
-            return this.owner.request.post(
+            this.owner.request.post(
               {
                 uri: `/project/${this.example_project_id}/folder`,
                 json: {
@@ -83,7 +60,7 @@ describe('ProjectDuplicateNames', function () {
               },
               (err, res, body) => {
                 this.testFolderId = body._id
-                return done()
+                done()
               }
             )
           })
@@ -93,23 +70,23 @@ describe('ProjectDuplicateNames', function () {
 
     it('should create a project', function () {
       expect(this.project.rootFolder[0].docs.length).to.equal(2)
-      return expect(this.project.rootFolder[0].fileRefs.length).to.equal(1)
+      expect(this.project.rootFolder[0].fileRefs.length).to.equal(1)
     })
 
     it('should create two docs in the docstore', function () {
       const docs = MockDocstoreApi.docs[this.example_project_id]
-      return expect(Object.keys(docs).length).to.equal(2)
+      expect(Object.keys(docs).length).to.equal(2)
     })
 
     it('should create one file in the filestore', function () {
       const files = MockFilestoreApi.files[this.example_project_id]
-      return expect(Object.keys(files).length).to.equal(1)
+      expect(Object.keys(files).length).to.equal(1)
     })
 
     describe('for an existing doc', function () {
       describe('trying to add a doc with the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/doc`,
               json: {
@@ -119,19 +96,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to add a folder with the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/folder`,
               json: {
@@ -141,35 +118,13 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
-        })
-      })
-
-      describe('trying to add a folder with the same name', function () {
-        beforeEach(function (done) {
-          return this.owner.request.post(
-            {
-              uri: `/project/${this.example_project_id}/folder`,
-              json: {
-                name: 'main.tex',
-                parent_folder_id: this.rootFolderId,
-              },
-            },
-            (err, res, body) => {
-              this.res = res
-              return done()
-            }
-          )
-        })
-
-        it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
     })
@@ -177,7 +132,7 @@ describe('ProjectDuplicateNames', function () {
     describe('for an existing file', function () {
       describe('trying to add a doc with the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/doc`,
               json: {
@@ -187,19 +142,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to add a folder with the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/folder`,
               json: {
@@ -209,19 +164,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to upload a file with the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/upload`,
               json: true,
@@ -245,13 +200,13 @@ describe('ProjectDuplicateNames', function () {
               this.body = body
               // update the image id because we have replaced the file
               this.imageFile._id = this.body.entity_id
-              return done()
+              done()
             }
           )
         })
 
         it('should succeed (overwriting the file)', function () {
-          return expect(this.body.success).to.equal(true)
+          expect(this.body.success).to.equal(true)
         })
       })
     })
@@ -260,7 +215,7 @@ describe('ProjectDuplicateNames', function () {
     describe('for an existing folder', function () {
       describe('trying to add a doc with the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/doc`,
               json: {
@@ -270,19 +225,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to add a folder with the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/folder`,
               json: {
@@ -292,19 +247,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to upload a file with the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/upload`,
               json: true,
@@ -326,21 +281,21 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.body = body
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with failure status', function () {
-          return expect(this.body.success).to.equal(false)
+          expect(this.body.success).to.equal(false)
         })
       })
     })
 
-    describe('for an existing doc', function () {
+    describe('rename for an existing doc', function () {
       describe('trying to rename a doc to the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/doc/${this.refBibDoc._id}/rename`,
               json: {
@@ -349,19 +304,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to rename a folder to the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/folder/${this.testFolderId}/rename`,
               json: {
@@ -370,19 +325,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to rename a file to the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/file/${this.imageFile._id}/rename`,
               json: {
@@ -391,21 +346,21 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with failure status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
     })
 
-    describe('for an existing file', function () {
+    describe('rename for an existing file', function () {
       describe('trying to rename a doc to the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/doc/${this.refBibDoc._id}/rename`,
               json: {
@@ -414,19 +369,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to rename a folder to the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/folder/${this.testFolderId}/rename`,
               json: {
@@ -435,19 +390,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to rename a file to the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/file/${this.imageFile._id}/rename`,
               json: {
@@ -456,21 +411,21 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with failure status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
     })
 
-    describe('for an existing folder', function () {
+    describe('rename for an existing folder', function () {
       describe('trying to rename a doc to the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/doc/${this.refBibDoc._id}/rename`,
               json: {
@@ -479,19 +434,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to rename a folder to the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/folder/${this.testFolderId}/rename`,
               json: {
@@ -500,19 +455,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to rename a file to the same name', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/file/${this.imageFile._id}/rename`,
               json: {
@@ -521,20 +476,20 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with failure status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
     })
 
     describe('for an existing folder with a file with the same name', function () {
       beforeEach(function (done) {
-        return this.owner.request.post(
+        this.owner.request.post(
           {
             uri: `/project/${this.example_project_id}/doc`,
             json: {
@@ -543,7 +498,7 @@ describe('ProjectDuplicateNames', function () {
             },
           },
           (err, res, body) => {
-            return this.owner.request.post(
+            this.owner.request.post(
               {
                 uri: `/project/${this.example_project_id}/doc`,
                 json: {
@@ -552,7 +507,7 @@ describe('ProjectDuplicateNames', function () {
                 },
               },
               (err, res, body) => {
-                return this.owner.request.post(
+                this.owner.request.post(
                   {
                     uri: `/project/${this.example_project_id}/folder`,
                     json: {
@@ -562,7 +517,7 @@ describe('ProjectDuplicateNames', function () {
                   },
                   (err, res, body) => {
                     this.subFolderId = body._id
-                    return this.owner.request.post(
+                    this.owner.request.post(
                       {
                         uri: `/project/${this.example_project_id}/folder`,
                         json: {
@@ -572,7 +527,7 @@ describe('ProjectDuplicateNames', function () {
                       },
                       (err, res, body) => {
                         this.otherFolderId = body._id
-                        return done()
+                        done()
                       }
                     )
                   }
@@ -585,7 +540,7 @@ describe('ProjectDuplicateNames', function () {
 
       describe('trying to move a doc into the folder', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/doc/${this.mainTexDoc._id}/move`,
               json: {
@@ -594,19 +549,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to move a file into the folder', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/file/${this.imageFile._id}/move`,
               json: {
@@ -615,19 +570,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to move a folder into the folder', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/folder/${this.otherFolderId}/move`,
               json: {
@@ -636,19 +591,19 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
 
       describe('trying to move a folder into a subfolder of itself', function () {
         beforeEach(function (done) {
-          return this.owner.request.post(
+          this.owner.request.post(
             {
               uri: `/project/${this.example_project_id}/folder/${this.testFolderId}/move`,
               json: {
@@ -657,15 +612,38 @@ describe('ProjectDuplicateNames', function () {
             },
             (err, res, body) => {
               this.res = res
-              return done()
+              done()
             }
           )
         })
 
         it('should respond with 400 error status', function () {
-          return expect(this.res.statusCode).to.equal(400)
+          expect(this.res.statusCode).to.equal(400)
         })
       })
     })
   })
+
+  describe('regex characters in title', function () {
+    let response, userHelper
+    beforeEach(async function () {
+      userHelper = new UserHelper()
+      userHelper = await UserHelper.createUser()
+      userHelper = await UserHelper.loginUser(
+        userHelper.getDefaultEmailPassword()
+      )
+    })
+    it('should handle characters that would cause an invalid regular expression', async function () {
+      const projectName = 'Example (test'
+      response = await userHelper.request.post('/project/new', {
+        simple: false,
+        form: { projectName },
+      })
+      expect(response.statusCode).to.equal(200) // can create project
+      response = await userHelper.request.get(
+        `/project/${JSON.parse(response.body).project_id}`
+      )
+      expect(response.statusCode).to.equal(200) // can open project
+    })
+  })
 })

services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js

@@ -261,6 +261,7 @@ describe('ProjectDetailsHandler', function () {
           { _id: 140, name: 'numeric (40)' },
           { _id: 141, name: 'Yearbook (2021)' },
           { _id: 142, name: 'Yearbook (2021) (1)' },
+          { _id: 142, name: 'Resume (2020' },
         ],
         readAndWrite: [
           { _id: 4, name: 'name2' },
@@ -384,6 +385,24 @@ describe('ProjectDetailsHandler', function () {
       )
       expect(name).to.equal('Yearbook (2021) (2)')
     })
+    describe('title with that causes invalid regex', function () {
+      it('should create the project with a suffix when project name exists', async function () {
+        const name = await this.handler.promises.generateUniqueName(
+          this.user._id,
+          'Resume (2020',
+          []
+        )
+        expect(name).to.equal('Resume (2020 (1)')
+      })
+      it('should create the project with the provided name', async function () {
+        const name = await this.handler.promises.generateUniqueName(
+          this.user._id,
+          'Yearbook (2021',
+          []
+        )
+        expect(name).to.equal('Yearbook (2021')
+      })
+    })
   })
 
   describe('fixProjectName', function () {