Merge pull request #1608 from sharelatex/bg-allow-reserved-names-safely

allow reserved names safely

GitOrigin-RevId: d0c5f287411f5cd6d0639f57ef925409571acf8a

services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee

@@ -122,6 +122,9 @@ module.exports = ProjectEntityMongoUpdateHandler = self =
 			return callback(err) if err?
 			ProjectLocator.findElement {project, element_id: entity_id, type: entityType}, (err, entity, entityPath)->
 				return callback(err) if err?
+				# Prevent top-level docs/files with reserved names (to match v1 behaviour)
+				if self._blockedFilename entityPath, entityType
+					return callback new Errors.InvalidNameError("blocked element name")
 				self._checkValidMove project, entityType, entity, entityPath, destFolderId, (error) ->
 					return callback(error) if error?
 					ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) ->
@@ -153,10 +156,13 @@ module.exports = ProjectEntityMongoUpdateHandler = self =
 				return callback(error) if error?
 				ProjectLocator.findElement {project:project, element_id:entity_id, type:entityType}, (error, entity, entPath, parentFolder)=>
 					return callback(error) if error?
+					endPath = path.join(path.dirname(entPath.fileSystem), newName)
+					# Prevent top-level docs/files with reserved names (to match v1 behaviour)
+					if self._blockedFilename {fileSystem: endPath}, entityType
+						return callback new Errors.InvalidNameError("blocked element name")
 					# check if the new name already exists in the current folder
 					self._checkValidElementName parentFolder, newName, (error) =>
 						return callback(error) if error?
-						endPath = path.join(path.dirname(entPath.fileSystem), newName)
 						conditions = {_id:project_id}
 						update = "$set":{}, "$inc":{}
 						namePath = entPath.mongo+".name"
@@ -257,6 +263,9 @@ module.exports = ProjectEntityMongoUpdateHandler = self =
 			# check if the path would be too long
 			if not SafePath.isAllowedLength newPath.fileSystem
 				return callback new Errors.InvalidNameError("path too long")
+			# Prevent top-level docs/files with reserved names (to match v1 behaviour)
+			if self._blockedFilename newPath, type
+				return callback new Errors.InvalidNameError("blocked element name")
 			self._checkValidElementName folder, element.name, (err) =>
 				return callback(err) if err?
 				id = element._id+''
@@ -276,6 +285,18 @@ module.exports = ProjectEntityMongoUpdateHandler = self =
 						return callback(err)
 					callback(err, {path:newPath}, newProject)
 
+	_blockedFilename: (entityPath, entityType) ->
+		# check if name would be blocked in v1
+		# javascript reserved names are forbidden for docs and files
+		# at the top-level (but folders with reserved names are allowed).
+		isFolder = (entityType in ['folder', 'folders'])
+		[dir, file] = [path.dirname(entityPath.fileSystem), path.basename(entityPath.fileSystem)]
+		isTopLevel = dir is '/'
+		if isTopLevel and !isFolder && SafePath.isBlockedFilename file
+			return true
+		else
+			return false
+
 	_checkValidElementName: (folder, name, callback = (err) ->) ->
 		# check if the name is already taken by a doc, file or
 		# folder. If so, return an error "file already exists".

services/web/app/coffee/Features/Project/SafePath.coffee

@@ -19,7 +19,7 @@ load = () ->
 
 	BADFILE_RX = ///
 		(^\.$)      # reject . as a filename
-		|	(^\.\.$)  # reject .. as a filename
+		| (^\.\.$)  # reject .. as a filename
 		| (^\s+)    # reject leading space
 		| (\s+$)    # reject trailing space
 		///g
@@ -67,8 +67,10 @@ load = () ->
 		isCleanFilename: (filename) ->
 			return SafePath.isAllowedLength(filename) &&
 				!BADCHAR_RX.test(filename) &&
-				!BADFILE_RX.test(filename) &&
-				!BLOCKEDFILE_RX.test(filename)
+				!BADFILE_RX.test(filename)
+
+		isBlockedFilename: (filename) ->
+			return BLOCKEDFILE_RX.test(filename)
 
 		# returns whether a full path is 'clean' - e.g. is a full or relative path
 		# that points to a file, and each element passes the rules in 'isCleanFilename'
@@ -81,6 +83,9 @@ load = () ->
 			for element in elements
 				return false if element.length > 0 && !SafePath.isCleanFilename element
 
+			# check for a top-level reserved name
+			return false if BLOCKEDFILE_RX.test(path.replace(/^\/?/,''))  # remove leading slash if present
+
 			return true
 
 		isAllowedLength: (pathname) ->

services/web/public/src/ide/directives/SafePath.js

@@ -87,8 +87,7 @@ prototype\
       return (
         SafePath.isAllowedLength(filename) &&
         !BADCHAR_RX.test(filename) &&
-        !BADFILE_RX.test(filename) &&
-        !BLOCKEDFILE_RX.test(filename)
+        !BADFILE_RX.test(filename)
       )
     },
 

services/web/public/src/ide/file-tree/FileTreeManager.js

@@ -168,16 +168,18 @@ define([
     }
 
     getMultiSelectedEntityChildNodes() {
+      // use pathnames with a leading slash to avoid
+      // problems with reserved Object properties
       const entities = this.getMultiSelectedEntities()
       const paths = {}
       for (var entity of Array.from(entities)) {
-        paths[this.getEntityPath(entity)] = entity
+        paths["/" + this.getEntityPath(entity)] = entity
       }
       const prefixes = {}
       for (var path in paths) {
         entity = paths[path]
         const parts = path.split('/')
-        if (parts.length <= 1) {
+        if (parts.length <= 2) {
           continue
         } else {
           // Record prefixes a/b/c.tex -> 'a' and 'a/b'
@@ -186,7 +188,7 @@ define([
             asc ? i <= end : i >= end;
             asc ? i++ : i--
           ) {
-            prefixes[parts.slice(0, i).join('/')] = true
+            prefixes["/" + parts.slice(0, i).join('/')] = true
           }
         }
       }

services/web/test/unit/coffee/Project/SafePathTests.coffee

@@ -63,17 +63,18 @@ describe 'SafePath', ->
 			result = @SafePath.isCleanFilename 'foo\uD800\uDFFFbar'
 			result.should.equal false
 
-		it 'should not accept javascript property names', ->
+		it 'should accept javascript property names', ->
 			result = @SafePath.isCleanFilename 'prototype'
-			result.should.equal false
+			result.should.equal true
 
-		it 'should not accept javascript property names in the prototype', ->
+		it 'should accept javascript property names in the prototype', ->
 			result = @SafePath.isCleanFilename 'hasOwnProperty'
-			result.should.equal false
+			result.should.equal true
 
-		it 'should not accept javascript property names resulting from substitutions', ->
-			result = @SafePath.isCleanFilename '  proto  '
-			result.should.equal false
+		# this test never worked correctly because the spaces are not replaced by underscores in isCleanFilename
+		# it 'should not accept javascript property names resulting from substitutions', ->
+		# 	result = @SafePath.isCleanFilename '  proto  '
+		# 	result.should.equal false
 
 		# it 'should not accept a trailing .', ->
 		# 	result = @SafePath.isCleanFilename 'hello.'
@@ -136,6 +137,18 @@ describe 'SafePath', ->
 			result = @SafePath.isCleanPath 'foo//*/bar'
 			result.should.equal false
 
+		it 'should not accept javascript property names', ->
+			result = @SafePath.isCleanPath 'prototype'
+			result.should.equal false
+
+		it 'should not accept javascript property names in the prototype', ->
+			result = @SafePath.isCleanPath 'hasOwnProperty'
+			result.should.equal false
+
+		it 'should not accept javascript property names resulting from substitutions', ->
+			result = @SafePath.isCleanPath '  proto  '
+			result.should.equal false
+
 	describe 'isAllowedLength', ->
 		it 'should accept a valid path "main.tex"', ->
 			result = @SafePath.isAllowedLength 'main.tex'