From c07de3d7c5cf6a53c164e93465d2477f24a18dda Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Mon, 23 Mar 2015 19:25:27 -0700 Subject: [PATCH 1/3] Add test for long file paths This test reads and write a directory with a path that is above 300 characters long, and checks that it comes out the other side. Currently, this test fails on Windows due to https://www.npmjs.com/package/standard. A subsequent commit will fix this. --- examples/long-paths.js | 41 +++++++++++++++++++++++++++++++++++++++++ package.json | 3 ++- 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 examples/long-paths.js diff --git a/examples/long-paths.js b/examples/long-paths.js new file mode 100644 index 0000000..ee2738f --- /dev/null +++ b/examples/long-paths.js @@ -0,0 +1,41 @@ +// This test creates a file with a path that's over 300 characters +// long, which is longer than the Windows limit unless you use the +// '\\?\' prefix. +// https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx +// +// Then it passes that directory into and out of fstream, to see if +// that file comes out the other side. This tests +// https://github.com/npm/fstream/issues/30 + +var tap = require('tap') +var temp = require('temp').track() +var fs = require('fs') +var path = require('path') +var mkdirp = require('mkdirp') +var fstream = require('../fstream.js') + +tap.test('long file paths', function (t) { + var inputDir = temp.mkdirSync('fstream-test-input') + var outputDir = temp.mkdirSync('fstream-test-output') + + var longDir = inputDir + while (longDir.length < 300) { + longDir = path.join(longDir, 'subdirectory') + } + + var STAMP = 'stamp' + + mkdirp.sync(longDir) + var inputStampedFile = path.join(longDir, 'file') + fs.writeFileSync(inputStampedFile, STAMP) + + var onPipeComplete = function () { + var outputStampedFile = inputStampedFile.replace(inputDir, outputDir) + t.equal(fs.readFileSync(outputStampedFile, 'utf-8'), STAMP) + t.end() + } + + var reader = fstream.Reader(inputDir) + reader.on('end', onPipeComplete) + reader.pipe(fstream.Writer(outputDir)) +}) diff --git a/package.json b/package.json index 13ba5bb..ceac32b 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,8 @@ }, "devDependencies": { "tap": "", - "standard": "^2.3.2" + "standard": "^2.3.2", + "temp": "^0.8.1" }, "scripts": { "test": "standard && tap examples/*.js" From a7b34f7fc446a020856b22c84bd766403ffb7d25 Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Mon, 23 Mar 2015 19:43:41 -0700 Subject: [PATCH 2/3] Disable symlink test on Windows --- examples/symlink-write.js | 45 +++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/examples/symlink-write.js b/examples/symlink-write.js index f6f5109..cb864b3 100644 --- a/examples/symlink-write.js +++ b/examples/symlink-write.js @@ -1,25 +1,28 @@ var fstream = require('../fstream.js') var notOpen = false -fstream - .Writer({ - path: 'path/to/symlink', - linkpath: './file', - isSymbolicLink: true, - mode: '0755' // octal strings supported - }) - .on('close', function () { - notOpen = true - var fs = require('fs') - var s = fs.lstatSync('path/to/symlink') - var isSym = s.isSymbolicLink() - console.log((isSym ? '' : 'not ') + 'ok 1 should be symlink') - var t = fs.readlinkSync('path/to/symlink') - var isTarget = t === './file' - console.log((isTarget ? '' : 'not ') + 'ok 2 should link to ./file') - }) - .end() +// No symlinks on Windows +if (process.platform !== 'win32') { + fstream + .Writer({ + path: 'path/to/symlink', + linkpath: './file', + isSymbolicLink: true, + mode: '0755' // octal strings supported + }) + .on('close', function () { + notOpen = true + var fs = require('fs') + var s = fs.lstatSync('path/to/symlink') + var isSym = s.isSymbolicLink() + console.log((isSym ? '' : 'not ') + 'ok 1 should be symlink') + var t = fs.readlinkSync('path/to/symlink') + var isTarget = t === './file' + console.log((isTarget ? '' : 'not ') + 'ok 2 should link to ./file') + }) + .end() -process.on('exit', function () { - console.log((notOpen ? '' : 'not ') + 'ok 3 should be closed') -}) + process.on('exit', function () { + console.log((notOpen ? '' : 'not ') + 'ok 3 should be closed') + }) +} From d11b9ec4a13918447c8af7559c243c190744dd1c Mon Sep 17 00:00:00 2001 From: Avital Oliver Date: Mon, 23 Mar 2015 19:45:39 -0700 Subject: [PATCH 3/3] Fix treatment of long file paths in Windows 3e5d1714 (from November 2011) introduced this code to fix treatment of long file paths in Windows. Then, one week later, this commit fixed long file paths directly in Node's `fs` library: https://github.com/joyent/node/commit/1f16a7b6e541694067178a022b9f7a082a6ce7f1 So, the code currently in fstream actually /breaks/ long file paths in Windows. This fixes it, but keeps an additional mysterious line introduced by that original commit that I don't understand. Fixes #30. Thanks to @sdarnell for identifying the problem. --- lib/reader.js | 9 +-------- lib/writer.js | 5 +---- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/lib/reader.js b/lib/reader.js index c23fe45..4550210 100644 --- a/lib/reader.js +++ b/lib/reader.js @@ -97,15 +97,8 @@ function Reader (props, currentStat) { self._path = self.path = path.resolve(props.path) if (process.platform === 'win32') { + // XXX What is this code for? self.path = self._path = self.path.replace(/\?/g, '_') - if (self._path.length >= 260) { - // how DOES one create files on the moon? - // if the path has spaces in it, then UNC will fail. - self._swallowErrors = true - // if (self._path.indexOf(" ") === -1) { - self._path = '\\\\?\\' + self.path.replace(/\//g, '\\') - // } - } } self.basename = props.basename = path.basename(self.path) self.dirname = props.dirname = path.dirname(self.path) diff --git a/lib/writer.js b/lib/writer.js index 25a608d..1188b91 100644 --- a/lib/writer.js +++ b/lib/writer.js @@ -72,11 +72,8 @@ function Writer (props, current) { self._path = self.path = path.resolve(props.path) if (process.platform === 'win32') { + // XXX What is this code for? self.path = self._path = self.path.replace(/\?/g, '_') - if (self._path.length >= 260) { - self._swallowErrors = true - self._path = '\\\\?\\' + self.path.replace(/\//g, '\\') - } } self.basename = path.basename(props.path) self.dirname = path.dirname(props.path)