From e624f697d2926e2e4a6f812285500b9ccadb3105 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 7 Jan 2019 11:18:10 +0000 Subject: [PATCH] Merge pull request #1352 from sharelatex/spd-zip-files-with-backslashes Handle import of zip files that include filenames with backslashes GitOrigin-RevId: 9f84cf6e0a648ee04bac89fe385931d603709a41 --- .../Features/Uploads/ArchiveManager.coffee | 8 +- services/web/npm-shrinkwrap.json | 106 +++++++++--------- services/web/package.json | 2 +- .../coffee/ProjectStructureTests.coffee | 48 ++++++++ ...est_project_with_backslash_in_filename.zip | Bin 0 -> 445 bytes ...t_project_with_shared_top_level_folder.zip | Bin 0 -> 470 bytes .../coffee/Uploads/ArchiveManagerTests.coffee | 32 +++++- 7 files changed, 133 insertions(+), 63 deletions(-) create mode 100644 services/web/test/acceptance/files/test_project_with_backslash_in_filename.zip create mode 100644 services/web/test/acceptance/files/test_project_with_shared_top_level_folder.zip diff --git a/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee b/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee index 3db074b13f..b923431f41 100644 --- a/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee +++ b/services/web/app/coffee/Features/Uploads/ArchiveManager.coffee @@ -38,16 +38,18 @@ module.exports = ArchiveManager = callback(null, isTooLarge) _checkFilePath: (entry, destination, callback = (err, destFile) ->) -> + # transform backslashes to forwardslashes to accommodate badly-behaved zip archives + transformedFilename = entry.fileName.replace(/\\/g, '/') # check if the entry is a directory endsWithSlash = /\/$/ - if endsWithSlash.test(entry.fileName) + if endsWithSlash.test(transformedFilename) return callback() # don't give a destfile for directory # check that the file does not use a relative path - for dir in entry.fileName.split('/') + for dir in transformedFilename.split('/') if dir == '..' return callback(new Error("relative path")) # check that the destination file path is normalized - dest = "#{destination}/#{entry.fileName}" + dest = "#{destination}/#{transformedFilename}" if dest != Path.normalize(dest) return callback(new Error("unnormalized path")) else diff --git a/services/web/npm-shrinkwrap.json b/services/web/npm-shrinkwrap.json index 542498aae1..ac8e28d48f 100644 --- a/services/web/npm-shrinkwrap.json +++ b/services/web/npm-shrinkwrap.json @@ -1936,7 +1936,7 @@ "readable-stream": { "version": "2.3.6", "from": "readable-stream@>=2.2.2 <3.0.0", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz" + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz" }, "string_decoder": { "version": "1.1.1", @@ -2107,9 +2107,9 @@ "resolved": "https://registry.npmjs.org/cookie-signature/-/cookie-signature-1.0.6.tgz" }, "cookies": { - "version": "0.7.1", + "version": "0.7.3", "from": "cookies@>=0.2.2", - "resolved": "https://registry.npmjs.org/cookies/-/cookies-0.7.1.tgz", + "resolved": "https://registry.npmjs.org/cookies/-/cookies-0.7.3.tgz", "dev": true }, "copy-descriptor": { @@ -2629,9 +2629,9 @@ "dev": true }, "domelementtype": { - "version": "1.3.0", + "version": "1.3.1", "from": "domelementtype@>=1.3.0 <2.0.0", - "resolved": "https://registry.npmjs.org/domelementtype/-/domelementtype-1.3.0.tgz", + "resolved": "https://registry.npmjs.org/domelementtype/-/domelementtype-1.3.1.tgz", "dev": true }, "domhandler": { @@ -2835,9 +2835,9 @@ "dev": true }, "entities": { - "version": "1.1.1", + "version": "1.1.2", "from": "entities@>=1.1.1 <2.0.0", - "resolved": "https://registry.npmjs.org/entities/-/entities-1.1.1.tgz", + "resolved": "https://registry.npmjs.org/entities/-/entities-1.1.2.tgz", "dev": true }, "errno": { @@ -3603,9 +3603,9 @@ } }, "fd-slicer": { - "version": "1.0.1", - "from": "fd-slicer@>=1.0.1 <1.1.0", - "resolved": "https://registry.npmjs.org/fd-slicer/-/fd-slicer-1.0.1.tgz" + "version": "1.1.0", + "from": "fd-slicer@>=1.1.0 <1.2.0", + "resolved": "https://registry.npmjs.org/fd-slicer/-/fd-slicer-1.1.0.tgz" }, "figures": { "version": "2.0.0", @@ -4798,7 +4798,7 @@ "readable-stream": { "version": "2.3.6", "from": "readable-stream@^2.0.1", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "dev": true }, "string_decoder": { @@ -4826,33 +4826,21 @@ "dev": true }, "htmlparser2": { - "version": "3.9.2", - "from": "htmlparser2@>=3.9.0 <4.0.0", - "resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-3.9.2.tgz", + "version": "3.10.0", + "from": "htmlparser2@>=3.10.0 <4.0.0", + "resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-3.10.0.tgz", "dev": true, "dependencies": { - "isarray": { - "version": "1.0.0", - "from": "isarray@~1.0.0", - "resolved": "https://registry.npmjs.org/isarray/-/isarray-1.0.0.tgz", - "dev": true - }, - "process-nextick-args": { - "version": "2.0.0", - "from": "process-nextick-args@>=2.0.0 <2.1.0", - "resolved": "https://registry.npmjs.org/process-nextick-args/-/process-nextick-args-2.0.0.tgz", - "dev": true - }, "readable-stream": { - "version": "2.3.6", - "from": "readable-stream@>=2.0.2 <3.0.0", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "version": "3.1.1", + "from": "readable-stream@>=3.0.6 <4.0.0", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-3.1.1.tgz", "dev": true }, "string_decoder": { - "version": "1.1.1", - "from": "string_decoder@>=1.1.1 <1.2.0", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.1.1.tgz", + "version": "1.2.0", + "from": "string_decoder@>=1.1.1 <2.0.0", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.2.0.tgz", "dev": true } } @@ -5844,9 +5832,9 @@ } }, "keygrip": { - "version": "1.0.2", - "from": "keygrip@>=1.0.2 <1.1.0", - "resolved": "https://registry.npmjs.org/keygrip/-/keygrip-1.0.2.tgz", + "version": "1.0.3", + "from": "keygrip@>=1.0.3 <1.1.0", + "resolved": "https://registry.npmjs.org/keygrip/-/keygrip-1.0.3.tgz", "dev": true }, "killable": { @@ -7160,7 +7148,7 @@ "readable-stream": { "version": "2.3.6", "from": "readable-stream@^2.0.1", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "dev": true }, "string_decoder": { @@ -7811,7 +7799,7 @@ "readable-stream": { "version": "2.3.6", "from": "readable-stream@^2.3.3", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "dev": true }, "string_decoder": { @@ -8168,7 +8156,7 @@ "readable-stream": { "version": "2.3.6", "from": "readable-stream@^2.0.2", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "dev": true }, "readdirp": { @@ -10065,9 +10053,9 @@ } }, "sanitize-html": { - "version": "1.18.2", + "version": "1.20.0", "from": "sanitize-html@>=1.14.1 <2.0.0", - "resolved": "http://registry.npmjs.org/sanitize-html/-/sanitize-html-1.18.2.tgz", + "resolved": "https://registry.npmjs.org/sanitize-html/-/sanitize-html-1.20.0.tgz", "dev": true, "dependencies": { "ansi-styles": { @@ -10089,9 +10077,9 @@ "dev": true }, "postcss": { - "version": "6.0.22", - "from": "postcss@>=6.0.14 <7.0.0", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-6.0.22.tgz", + "version": "7.0.7", + "from": "postcss@>=7.0.5 <8.0.0", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-7.0.7.tgz", "dev": true }, "source-map": { @@ -10101,9 +10089,9 @@ "dev": true }, "supports-color": { - "version": "5.4.0", + "version": "5.5.0", "from": "supports-color@>=5.3.0 <6.0.0", - "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.4.0.tgz", + "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.5.0.tgz", "dev": true } } @@ -10814,7 +10802,7 @@ "readable-stream": { "version": "2.3.6", "from": "readable-stream@^2.0.2", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "dev": true }, "string_decoder": { @@ -10890,7 +10878,7 @@ "readable-stream": { "version": "2.3.6", "from": "readable-stream@^2.3.0", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "dev": true }, "string_decoder": { @@ -11278,13 +11266,19 @@ "translations-sharelatex": { "version": "0.1.4", "from": "git+https://github.com/sharelatex/translations-sharelatex.git#master", - "resolved": "git+https://github.com/sharelatex/translations-sharelatex.git#c3dbe7869c594cb91cd366020f8b69e50a9405c9", + "resolved": "git+https://github.com/sharelatex/translations-sharelatex.git#beea1036cdf3adf41cd41e73fcfd6a5a70f83763", "dev": true, "dependencies": { "async": { - "version": "2.6.0", + "version": "2.6.1", "from": "async@>=2.1.4 <3.0.0", - "resolved": "https://registry.npmjs.org/async/-/async-2.6.0.tgz", + "resolved": "https://registry.npmjs.org/async/-/async-2.6.1.tgz", + "dev": true + }, + "lodash": { + "version": "4.17.11", + "from": "lodash@>=4.17.10 <5.0.0", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.11.tgz", "dev": true } } @@ -12209,13 +12203,13 @@ "readable-stream": { "version": "2.3.6", "from": "readable-stream@^2.0.2", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "dev": true }, "readdirp": { "version": "2.1.0", "from": "readdirp@>=2.0.0 <3.0.0", - "resolved": "http://registry.npmjs.org/readdirp/-/readdirp-2.1.0.tgz", + "resolved": "https://registry.npmjs.org/readdirp/-/readdirp-2.1.0.tgz", "dev": true }, "string_decoder": { @@ -12413,7 +12407,7 @@ "chokidar": { "version": "2.0.3", "from": "chokidar@>=2.0.0 <3.0.0", - "resolved": "https://registry.npmjs.org/chokidar/-/chokidar-2.0.3.tgz", + "resolved": "http://registry.npmjs.org/chokidar/-/chokidar-2.0.3.tgz", "dev": true }, "cliui": { @@ -13228,9 +13222,9 @@ } }, "yauzl": { - "version": "2.9.1", - "from": "yauzl@>=2.8.0 <3.0.0", - "resolved": "https://registry.npmjs.org/yauzl/-/yauzl-2.9.1.tgz" + "version": "2.10.0", + "from": "yauzl@2.10.0", + "resolved": "https://registry.npmjs.org/yauzl/-/yauzl-2.10.0.tgz" }, "yeast": { "version": "0.1.2", diff --git a/services/web/package.json b/services/web/package.json index 63333dc0a6..31df244569 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -104,7 +104,7 @@ "v8-profiler": "^5.2.3", "valid-url": "^1.0.9", "xml2js": "0.2.0", - "yauzl": "^2.8.0" + "yauzl": "^2.10.0" }, "devDependencies": { "autoprefixer": "^6.6.1", diff --git a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee index 969c1c638e..0b633fecc7 100644 --- a/services/web/test/acceptance/coffee/ProjectStructureTests.coffee +++ b/services/web/test/acceptance/coffee/ProjectStructureTests.coffee @@ -204,6 +204,54 @@ describe "ProjectStructureChanges", -> expect(project.name).to.match @test_project_match done() + describe "uploading a project with a shared top-level folder", -> + before (done) -> + MockDocUpdaterApi.clearProjectStructureUpdates() + + zip_file = fs.createReadStream(Path.resolve(__dirname + '/../files/test_project_with_shared_top_level_folder.zip')) + + @owner.request.post { + uri: "project/new/upload", + formData: + qqfile: zip_file + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to upload project #{res.statusCode}") + @uploaded_project_id = JSON.parse(body).project_id + done() + + it "should not create the top-level folder", (done) -> + ProjectGetter.getProject @uploaded_project_id, (error, project) -> + expect(error).not.to.exist + expect(project.rootFolder[0].folders.length).to.equal 0 + expect(project.rootFolder[0].docs.length).to.equal 2 + done() + + describe "uploading a project with backslashes in the path names", -> + before (done) -> + MockDocUpdaterApi.clearProjectStructureUpdates() + + zip_file = fs.createReadStream(Path.resolve(__dirname + '/../files/test_project_with_backslash_in_filename.zip')) + + @owner.request.post { + uri: "project/new/upload", + formData: + qqfile: zip_file + }, (error, res, body) => + throw error if error? + if res.statusCode < 200 || res.statusCode >= 300 + throw new Error("failed to upload project #{res.statusCode}") + @uploaded_project_id = JSON.parse(body).project_id + done() + + it "should set the project name from the zip contents", (done) -> + ProjectGetter.getProject @uploaded_project_id, (error, project) -> + expect(error).not.to.exist + expect(project.rootFolder[0].folders[0].name).to.equal('styles') + expect(project.rootFolder[0].folders[0].docs[0].name).to.equal('ao.sty') + done() + describe "uploading a file", -> beforeEach (done) -> MockDocUpdaterApi.clearProjectStructureUpdates() diff --git a/services/web/test/acceptance/files/test_project_with_backslash_in_filename.zip b/services/web/test/acceptance/files/test_project_with_backslash_in_filename.zip new file mode 100644 index 0000000000000000000000000000000000000000..0774dfc3cbc4a00984c34b2b2b167541d781f335 GIT binary patch literal 445 zcmWIWW@Zs#U|`^2Xl_&Sd;2ISa4(Se8Hjm-xVWS;C$%^xF<%eJ3hlMeK5QUzq`bas zdx~*k%I>n`+nl+UZdq8uaYZ(6?evIF9m&6(A`9Qk9X%qFnk=^~r+lD~KsY?6M{zhimA-Tf-j3;OJzcG-%pUbRAE&W%ki&od`AS=9LXeG$92 z#hgp^OH>x~Hu;Pt_jO)dyxq0@@YWEEpdHUIrAKkN8jD}p?Ku19@ef~r2|0&8^_{!9 z-RcmN#VqANp8nsbn12$w*Rkv5V9Ablwe_yVMxo**AEThWMCH2GLGQ_;?fFk21b?_%nS@HDL{1r zICb+ubrkK()0|Ccu7H_So}ZhPSOPbPA7};$W0=FpB*%=)KN3)% z2{62M1Ti7LV} @zipfile.emit "entry", {fileName: "foo/./testfile.txt"} @zipfile.emit "end" - it "should not write try to read the file entry", -> + it "should not try to read the file entry", -> @zipfile.openReadStream.called.should.equal false it "should log out a warning", -> @logger.warn.called.should.equal true + describe "with backslashes in the path", -> + beforeEach (done) -> + @readStream = new events.EventEmitter + @readStream.pipe = sinon.stub() + @writeStream = new events.EventEmitter + @fs.createWriteStream = sinon.stub().returns @writeStream + @zipfile.openReadStream = sinon.stub().callsArgWith(1, null, @readStream) + @fse.ensureDir = sinon.stub().callsArg(1) + @ArchiveManager.extractZipArchive @source, @destination, (error) => + @callback(error) + done() + @zipfile.emit "entry", {fileName: 'wombat\\foo.tex'} + @zipfile.emit "entry", {fileName: 'potato\\bar.tex'} + @zipfile.emit "end" + + it "should read the file entry with its original path", -> + @zipfile.openReadStream.should.be.calledWith({fileName: 'wombat\\foo.tex'}) + @zipfile.openReadStream.should.be.calledWith({fileName: 'potato\\bar.tex'}) + + it "should treat the backslashes as a directory separator when creating the directory", -> + @fse.ensureDir.should.be.calledWith("#{@destination}/wombat"); + @fse.ensureDir.should.be.calledWith("#{@destination}/potato"); + + it "should treat the backslashes as a directory separator when creating the file", -> + @fs.createWriteStream.should.be.calledWith("#{@destination}/wombat/foo.tex"); + @fs.createWriteStream.should.be.calledWith("#{@destination}/potato/bar.tex"); + describe "with a directory entry", -> beforeEach (done) -> @zipfile.openReadStream = sinon.stub() @@ -126,7 +153,7 @@ describe "ArchiveManager", -> @zipfile.emit "entry", {fileName: "testdir/"} @zipfile.emit "end" - it "should not write try to read the entry", -> + it "should not try to read the entry", -> @zipfile.openReadStream.called.should.equal false it "should not log out a warning", -> @@ -295,4 +322,3 @@ describe "ArchiveManager", -> @callback .calledWith(null, @directory + "/folder") .should.equal true -