From 95408e201c7e8958454e6ffde055289f603bb9eb Mon Sep 17 00:00:00 2001 From: Ian Wehrman Date: Mon, 18 Nov 2013 16:28:52 -0800 Subject: [PATCH 1/3] Add support for normalizing UNC paths in the FileSystem, controlled by the FileSystemImpl.normalizeUNCPaths flag --- src/filesystem/FileSystem.js | 25 ++++++--- .../impls/appshell/AppshellFileSystem.js | 3 ++ test/spec/FileSystem-test.js | 52 +++++++++++++++++++ test/spec/MockFileSystemImpl.js | 7 +++ 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/src/filesystem/FileSystem.js b/src/filesystem/FileSystem.js index 1df56986221..e5fe7733dbc 100644 --- a/src/filesystem/FileSystem.js +++ b/src/filesystem/FileSystem.js @@ -388,6 +388,9 @@ define(function (require, exports, module) { return (fullPath[0] === "/" || fullPath[1] === ":"); }; + // Matches continguous groups of forward slashes + var _duplicatedSlashRE = /\/{2,}/g; + /** * Returns a canonical version of the path: no duplicated "/"es, no ".."s, * and directories guaranteed to end in a trailing "/" @@ -395,14 +398,17 @@ define(function (require, exports, module) { * @param {boolean=} isDirectory * @return {!string} */ - function _normalizePath(path, isDirectory) { + FileSystem.prototype._normalizePath = function (path, isDirectory) { if (!FileSystem.isAbsolutePath(path)) { throw new Error("Paths must be absolute: '" + path + "'"); // expect only absolute paths } - + + var normalizeUNCPaths = this._impl && this._impl.normalizeUNCPaths, + isUNCPath = normalizeUNCPaths && path.search(_duplicatedSlashRE) === 0; + // Remove duplicated "/"es - path = path.replace(/\/{2,}/g, "/"); + path = path.replace(_duplicatedSlashRE, "/"); // Remove ".." segments if (path.indexOf("..") !== -1) { @@ -427,8 +433,13 @@ define(function (require, exports, module) { } } + if (isUNCPath) { + // Restore the leading double slash that was removed previously + path = "/" + path; + } + return path; - } + }; /** * Return a File object for the specified path. @@ -438,7 +449,7 @@ define(function (require, exports, module) { * @return {File} The File object. This file may not yet exist on disk. */ FileSystem.prototype.getFileForPath = function (path) { - path = _normalizePath(path, false); + path = this._normalizePath(path, false); var file = this._index.getEntry(path); if (!file) { @@ -457,7 +468,7 @@ define(function (require, exports, module) { * @return {Directory} The Directory object. This directory may not yet exist on disk. */ FileSystem.prototype.getDirectoryForPath = function (path) { - path = _normalizePath(path, true); + path = this._normalizePath(path, true); var directory = this._index.getEntry(path); if (!directory) { @@ -579,7 +590,7 @@ define(function (require, exports, module) { return; } - path = _normalizePath(path, false); + path = this._normalizePath(path, false); var entry = this._index.getEntry(path); if (entry) { diff --git a/src/filesystem/impls/appshell/AppshellFileSystem.js b/src/filesystem/impls/appshell/AppshellFileSystem.js index a331dd57fff..04640726142 100644 --- a/src/filesystem/impls/appshell/AppshellFileSystem.js +++ b/src/filesystem/impls/appshell/AppshellFileSystem.js @@ -419,4 +419,7 @@ define(function (require, exports, module) { exports.watchPath = watchPath; exports.unwatchPath = unwatchPath; exports.unwatchAll = unwatchAll; + + // Only perform UNC path normalization on Windows + exports.normalizeUNCPaths = appshell.platform === "win"; }); diff --git a/test/spec/FileSystem-test.js b/test/spec/FileSystem-test.js index b1fdf2b6bba..25176a86e7a 100644 --- a/test/spec/FileSystem-test.js +++ b/test/spec/FileSystem-test.js @@ -141,7 +141,58 @@ define(function (require, exports, module) { }); it("should eliminate duplicated (contiguous) slashes", function () { + MockFileSystemImpl.normalizeUNCPaths = false; testPrefixes(["", "c:"], function () { + expectNormDir("/", "/"); + expectNormDir("//", "/"); + expectNormDir("///", "/"); + expectNormDir("//foo", "/foo/"); + expectNormDir("/foo//", "/foo/"); + expectNormDir("//foo//", "/foo/"); + expectNormDir("///foo///", "/foo/"); + expectNormDir("/foo//bar", "/foo/bar/"); + expectNormDir("/foo///bar", "/foo/bar/"); + + expectNormFile("/foo", "/foo"); + expectNormFile("//foo", "/foo"); + expectNormFile("///foo", "/foo"); + expectNormFile("/foo//bar", "/foo/bar"); + expectNormFile("/foo///bar", "/foo/bar"); + expectNormFile("//foo///bar", "/foo/bar"); + expectNormFile("///foo///bar", "/foo/bar"); + expectNormFile("///foo//bar", "/foo/bar"); + expectNormFile("///foo/bar", "/foo/bar"); + }); + }); + + it("should normalize continguous-slash prefixes for UNC paths", function () { + // UNC paths should have leading slashes reduced to a single leading pair + MockFileSystemImpl.normalizeUNCPaths = true; + testPrefixes([""], function () { + expectNormDir("/", "/"); + expectNormDir("//", "//"); + expectNormDir("///", "//"); + expectNormDir("//foo", "//foo/"); + expectNormDir("/foo//", "/foo/"); + expectNormDir("//foo//", "//foo/"); + expectNormDir("///foo///", "//foo/"); + expectNormDir("/foo//bar", "/foo/bar/"); + expectNormDir("/foo///bar", "/foo/bar/"); + + expectNormFile("/foo", "/foo"); + expectNormFile("//foo", "//foo"); + expectNormFile("///foo", "//foo"); + expectNormFile("/foo//bar", "/foo/bar"); + expectNormFile("/foo///bar", "/foo/bar"); + expectNormFile("//foo///bar", "//foo/bar"); + expectNormFile("///foo///bar", "//foo/bar"); + expectNormFile("///foo//bar", "//foo/bar"); + expectNormFile("///foo/bar", "//foo/bar"); + }); + + // UNC paths do not begin with a letter, so normalization is unchanged + testPrefixes(["c:"], function () { + expectNormDir("/", "/"); expectNormDir("//", "/"); expectNormDir("///", "/"); expectNormDir("//foo", "/foo/"); @@ -151,6 +202,7 @@ define(function (require, exports, module) { expectNormDir("/foo//bar", "/foo/bar/"); expectNormDir("/foo///bar", "/foo/bar/"); + expectNormFile("/foo", "/foo"); expectNormFile("//foo", "/foo"); expectNormFile("///foo", "/foo"); expectNormFile("/foo//bar", "/foo/bar"); diff --git a/test/spec/MockFileSystemImpl.js b/test/spec/MockFileSystemImpl.js index 73a0501eee5..a92f76b14ab 100644 --- a/test/spec/MockFileSystemImpl.js +++ b/test/spec/MockFileSystemImpl.js @@ -65,6 +65,9 @@ define(function (require, exports, module) { } }; + // Indicates whether, by default, the FS should perform UNC Path normalization + var _normalizeUNCPathsDefault = false; + // "Live" data for this instance of the file system. Use reset() to // initialize with _initialData var _data; @@ -339,11 +342,15 @@ define(function (require, exports, module) { exports.unwatchPath = unwatchPath; exports.unwatchAll = unwatchAll; + exports.normalizeUNCPaths = _normalizeUNCPathsDefault; + // Test methods exports.reset = function () { _data = {}; $.extend(_data, _initialData); _hooks = {}; + + exports.normalizeUNCPaths = _normalizeUNCPathsDefault; }; /** From 44396aaa3c4c3661339a85b2422a0090807d7989 Mon Sep 17 00:00:00 2001 From: Ian Wehrman Date: Mon, 18 Nov 2013 17:19:08 -0800 Subject: [PATCH 2/3] Rename the duplicated-slash regular expression and add JSDoc --- src/filesystem/FileSystem.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/filesystem/FileSystem.js b/src/filesystem/FileSystem.js index e5fe7733dbc..747791ec1b0 100644 --- a/src/filesystem/FileSystem.js +++ b/src/filesystem/FileSystem.js @@ -388,8 +388,11 @@ define(function (require, exports, module) { return (fullPath[0] === "/" || fullPath[1] === ":"); }; - // Matches continguous groups of forward slashes - var _duplicatedSlashRE = /\/{2,}/g; + /* + * Matches continguous groups of forward slashes + * @const + */ + var _DUPLICATED_SLASH_RE = /\/{2,}/g; /** * Returns a canonical version of the path: no duplicated "/"es, no ".."s, @@ -405,10 +408,10 @@ define(function (require, exports, module) { } var normalizeUNCPaths = this._impl && this._impl.normalizeUNCPaths, - isUNCPath = normalizeUNCPaths && path.search(_duplicatedSlashRE) === 0; + isUNCPath = normalizeUNCPaths && path.search(_DUPLICATED_SLASH_RE) === 0; // Remove duplicated "/"es - path = path.replace(_duplicatedSlashRE, "/"); + path = path.replace(_DUPLICATED_SLASH_RE, "/"); // Remove ".." segments if (path.indexOf("..") !== -1) { From a49cc5f327286ed6a2886f39e7837fe73bc477de Mon Sep 17 00:00:00 2001 From: Ian Wehrman Date: Tue, 19 Nov 2013 11:32:53 -0800 Subject: [PATCH 3/3] Assume the FileSystem is initialized within `_normalizePath`; initialize the singleton instance at module load --- src/brackets.js | 3 --- src/filesystem/FileSystem.js | 6 ++++-- test/SpecRunner.js | 14 ++++++-------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/brackets.js b/src/brackets.js index f19a108b983..3d3cd9569ce 100644 --- a/src/brackets.js +++ b/src/brackets.js @@ -107,9 +107,6 @@ define(function (require, exports, module) { require("file/NativeFileError"); PerfUtils.addMeasurement("brackets module dependencies resolved"); - - // Initialize the file system - FileSystem.init(require("fileSystemImpl")); // Local variables var params = new UrlParams(); diff --git a/src/filesystem/FileSystem.js b/src/filesystem/FileSystem.js index 747791ec1b0..b3b2b24de40 100644 --- a/src/filesystem/FileSystem.js +++ b/src/filesystem/FileSystem.js @@ -407,8 +407,7 @@ define(function (require, exports, module) { throw new Error("Paths must be absolute: '" + path + "'"); // expect only absolute paths } - var normalizeUNCPaths = this._impl && this._impl.normalizeUNCPaths, - isUNCPath = normalizeUNCPaths && path.search(_DUPLICATED_SLASH_RE) === 0; + var isUNCPath = this._impl.normalizeUNCPaths && path.search(_DUPLICATED_SLASH_RE) === 0; // Remove duplicated "/"es path = path.replace(_DUPLICATED_SLASH_RE, "/"); @@ -814,4 +813,7 @@ define(function (require, exports, module) { // Create the singleton instance _instance = new FileSystem(); + + // Initialize the singleton instance + _instance.init(require("fileSystemImpl")); }); diff --git a/test/SpecRunner.js b/test/SpecRunner.js index 785ad1d7e82..d6cffcd9588 100644 --- a/test/SpecRunner.js +++ b/test/SpecRunner.js @@ -28,11 +28,12 @@ require.config({ baseUrl: "../src", paths: { - "test" : "../test", - "perf" : "../test/perf", - "spec" : "../test/spec", - "text" : "thirdparty/text/text", - "i18n" : "thirdparty/i18n/i18n" + "test" : "../test", + "perf" : "../test/perf", + "spec" : "../test/spec", + "text" : "thirdparty/text/text", + "i18n" : "thirdparty/i18n/i18n", + "fileSystemImpl" : "filesystem/impls/appshell/AppshellFileSystem" } }); @@ -92,9 +93,6 @@ define(function (require, exports, module) { * for the installer in this run of Brackets. */ var NODE_CONNECTION_TIMEOUT = 30000; // 30 seconds - TODO: share with StaticServer? - - // Initialize the file system - FileSystem.init(require("filesystem/impls/appshell/AppshellFileSystem")); // parse URL parameters params.parse();