From c14437d2dd53bc5af32c3f9cd012ee11929ef360 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Mon, 23 Aug 2021 08:26:36 -0500 Subject: [PATCH] Merge pull request #4795 from overleaf/jel-project-title-regex Escape project title for regular expression GitOrigin-RevId: 495b96720de6d09cda905edc6464b55a5c85e21d --- .../app/src/Features/Project/ProjectHelper.js | 3 +- .../src/ProjectDuplicateNameTests.js | 234 ++++++++---------- .../src/Project/ProjectDetailsHandlerTests.js | 19 ++ 3 files changed, 127 insertions(+), 129 deletions(-) diff --git a/services/web/app/src/Features/Project/ProjectHelper.js b/services/web/app/src/Features/Project/ProjectHelper.js index eee36944da..a7463b3986 100644 --- a/services/web/app/src/Features/Project/ProjectHelper.js +++ b/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) ) diff --git a/services/web/test/acceptance/src/ProjectDuplicateNameTests.js b/services/web/test/acceptance/src/ProjectDuplicateNameTests.js index 5c58dbee48..be9e8f513e 100644 --- a/services/web/test/acceptance/src/ProjectDuplicateNameTests.js +++ b/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 + }) + }) }) diff --git a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js b/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js index 8ac4286eeb..dd71d7ce65 100644 --- a/services/web/test/unit/src/Project/ProjectDetailsHandlerTests.js +++ b/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 () {