Improve conrefs to handle all absolute file paths correctly

Add test for it

5 files changed, 130 insertions, 71 deletions

diff --git a/lib/book.js b/lib/book.js
index 980e505..08dd6dc 100644
--- a/lib/book.js
+++ b/lib/book.js
@@ -10,6 +10,7 @@ var fs = require("./utils/fs");
 var parseNavigation = require("./utils/navigation");
 var parseProgress = require("./utils/progress");
 var pageUtil = require("./utils/page");
+var pathUtil = require("./utils/path");
 var batch = require("./utils/batch");
 var links = require("./utils/links");
 var i18n = require("./utils/i18n");
@@ -636,8 +637,7 @@ Book.prototype.fileExists = function(filename) {
 
 // Check if a file path is inside the book
 Book.prototype.fileIsInBook = function(filename) {
-    filename = path.normalize(filename);
-    return (filename.substr(0, this.root.length) === this.root);
+    return pathUtil.isInRoot(this.root, filename);
 };
 
 // Read a file
@@ -716,26 +716,12 @@ Book.prototype.getConfig = function(key, def) {
 // Resolve a path in the book source
 // Enforce that the output path in the root folder
 Book.prototype.resolve = function() {
-    var input = _.chain(arguments)
-        .toArray()
-        .reduce(function(current, p) {
-            // Handle path relative to book root ('/README.md')
-            if (p[0] == '/' || p[0] == '\\') return p.slice(1);
-
-            return path.join(current, p);
-        })
-        .value();
-
-
-    var result = path.resolve(this.root, input);
-
-    if (!this.fileIsInBook(result)) {
-        err = new Error("EACCESS: '" + result + "' not in '" + this.root + "'");
-        err.code = "EACCESS";
-        throw err;
-    }
+    return pathUtil.resolveInRoot.apply(null, [this.root].concat(_.toArray(arguments)));
+};
 
-    return result
+// Convert an abslute path into a relative path to this
+Book.prototype.relative = function(p) {
+    return path.relative(this.root, p);
 };
 
 // Normalize a path to .html and convert README -> index
diff --git a/lib/conrefs_loader.js b/lib/conrefs_loader.js
new file mode 100644
index 0000000..72dce8a
--- /dev/null
+++ b/lib/conrefs_loader.js
@@ -0,0 +1,64 @@
+var Q = require("q");
+var path = require("path");
+var nunjucks = require("nunjucks");
+
+var git = require("./utils/git");
+var fs = require("./utils/fs");
+var pathUtil = require("./utils/path");
+
+// The loader should handle relative and git url
+var BookLoader = nunjucks.Loader.extend({
+    async: true,
+
+    init: function(book) {
+        this.book = book;
+    },
+
+    getSource: function(fileurl, callback) {
+        var that = this;
+
+        git.resolveFile(fileurl)
+        .then(function(filepath) {
+            // Is local file
+            if (!filepath) filepath = path.resolve(fileurl);
+            else that.book.log.debug.ln("resolve from git", fileurl, "to", filepath);
+
+            //  Read file from absolute path
+            return fs.readFile(filepath)
+            .then(function(source) {
+                return {
+                    src: source.toString(),
+                    path: filepath
+                }
+            });
+        })
+        .nodeify(callback);
+    },
+
+    resolve: function(from, to) {
+        // If origin is in the book, we enforce result file to be in the book
+        if (this.book.fileIsInBook(from)) {
+            return this.book.resolve(
+                this.book.relative(path.dirname(from)),
+                to
+            );
+        }
+
+        // If origin is in a git repository, we resolve file in the git repository
+        var gitRoot = git.resolveRoot(from);
+        if (gitRoot) {
+            return pathUtil.resolveInRoot(gitRoot, to);
+        }
+
+        // If origin is not in the book (include from a git content ref)
+        return path.resolve(path.dirname(from), to);
+    },
+
+    // Handle all files as relative, so that nunjucks pass responsability to "resolve"
+    // Only git urls are considered as absolute
+    isRelative: function(filename) {
+        return !git.checkUrl(filename);
+    }
+});
+
+module.exports = BookLoader;
diff --git a/lib/template.js b/lib/template.js
index 65f24c5..9f01d3c 100644
--- a/lib/template.js
+++ b/lib/template.js
@@ -4,11 +4,10 @@ var path = require("path");
 var nunjucks = require("nunjucks");
 var escapeStringRegexp = require("escape-string-regexp");
 
-var git = require("./utils/git");
-var fs = require("./utils/fs");
 var batch = require("./utils/batch");
 var pkg = require("../package.json");
 var defaultBlocks = require("./blocks");
+var BookLoader = require("./conrefs_loader")
 
 // Normalize result from a block
 function normBlockResult(blk) {
@@ -16,52 +15,6 @@ function normBlockResult(blk) {
     return blk;
 }
 
-// The loader should handle relative and git url
-var BookLoader = nunjucks.Loader.extend({
-    async: true,
-
-    init: function(book) {
-        this.book = book;
-    },
-
-    getSource: function(fileurl, callback) {
-        var that = this;
-
-        git.resolveFile(fileurl)
-        .then(function(filepath) {
-            // Is local file
-            if (!filepath) filepath = path.resolve(fileurl);
-            else that.book.log.debug.ln("resolve from git", fileurl, "to", filepath);
-
-            //  Read file from absolute path
-            return fs.readFile(filepath)
-            .then(function(source) {
-                return {
-                    src: source.toString(),
-                    path: filepath
-                }
-            });
-        })
-        .nodeify(callback);
-    },
-
-    resolve: function(from, to) {
-        // If origin is in the book, we enforce result file to be in the book
-        if (this.book.fileIsInBook(from)) {
-            return this.book.resolve(path.dirname(from), to);
-        }
-
-        // If origin is not in the book (include from a git content ref)
-        return path.resolve(path.dirname(from), to);
-    },
-
-    // Handle all files as relative, so that nunjucks pass responsability to "resolve"
-    // Only git urls are considered as absolute
-    isRelative: function(filename) {
-        return !git.checkUrl(filename);
-    }
-});
-
 
 var TemplateEngine = function(book) {
     this.book = book;
diff --git a/lib/utils/git.js b/lib/utils/git.js
index 5f17395..6eb9681 100644
--- a/lib/utils/git.js
+++ b/lib/utils/git.js
@@ -6,6 +6,7 @@ var path = require("path");
 var crc = require("crc");
 var exec = Q.denodeify(require("child_process").exec);
 var URI = require("URIjs");
+var pathUtil = require("./path");
 
 var fs = require("./fs");
 
@@ -89,7 +90,6 @@ function cloneGitRepo(host, ref) {
     });
 }
 
-
 // Get file from a git repo
 function resolveFileFromGit(giturl) {
     if (_.isString(giturl)) giturl = parseGitUrl(giturl);
@@ -104,9 +104,26 @@ function resolveFileFromGit(giturl) {
     });
 };
 
+// Return root of git repo from a filepath
+function resolveGitRoot(filepath) {
+    var relativeToGit, repoId
+
+    // No git repo cloned, or file is not in a git repository
+    if (!GIT_TMP || !pathUtil.isInRoot(GIT_TMP, filepath)) return null;
+
+    // Extract first directory (is the repo id)
+    relativeToGit = path.relative(GIT_TMP, filepath);
+    repoId = _.first(relativeToGit.split(path.sep));
+    if (!repoId) return;
+
+    // Return an absolute file
+    return path.resolve(GIT_TMP, repoId);
+};
+
 
 module.exports = {
     checkUrl: checkGitUrl,
     parseUrl: parseGitUrl,
-    resolveFile: resolveFileFromGit
+    resolveFile: resolveFileFromGit,
+    resolveRoot: resolveGitRoot
 };
diff --git a/lib/utils/path.js b/lib/utils/path.js
new file mode 100644
index 0000000..d5b98f7
--- /dev/null
+++ b/lib/utils/path.js
@@ -0,0 +1,39 @@
+var _ = require("lodash");
+var path = require('path');
+
+// Return true if file path is inside a folder
+function isInRoot(root, filename) {
+    filename = path.normalize(filename);
+    return (filename.substr(0, root.length) === root);
+}
+
+// Resolve paths in a specific folder
+// Throw error if file is outside this folder
+function resolveInRoot(root) {
+    var input = _.chain(arguments)
+        .toArray()
+        .slice(1)
+        .reduce(function(current, p, i) {
+            // Handle path relative to book root ('/README.md')
+            if (p[0] == '/' || p[0] == '\\') return p.slice(1);
+
+            return current? path.join(current, p) : path.normalize(p);
+        }, '')
+        .value();
+
+    var result = path.resolve(root, input);
+
+    if (!isInRoot(root, result)) {
+        err = new Error("EACCESS: '" + result + "' not in '" + root + "'");
+        err.code = "EACCESS";
+        throw err;
+    }
+
+    return result
+};
+
+
+module.exports = {
+    isInRoot: isInRoot,
+    resolveInRoot: resolveInRoot
+};