Merge pull request #929 from GitbookIO/feature/improve_resolve

Improve paths resolving for conrefs

15 files changed, 337 insertions, 65 deletions

diff --git a/lib/book.js b/lib/book.js
index b306c51..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");
@@ -630,21 +631,26 @@ Book.prototype.findFile = function(filename) {
 // Check if a file exists in the book
 Book.prototype.fileExists = function(filename) {
     return fs.exists(
-        path.join(this.root, filename)
+        this.resolve(filename)
     );
 };
 
+// Check if a file path is inside the book
+Book.prototype.fileIsInBook = function(filename) {
+    return pathUtil.isInRoot(this.root, filename);
+};
+
 // Read a file
 Book.prototype.readFile = function(filename) {
     return fs.readFile(
-        path.join(this.root, filename),
+        this.resolve(filename),
         { encoding: "utf8" }
     );
 };
 
 // Return stat for a file
 Book.prototype.statFile = function(filename) {
-    return fs.stat(path.join(this.root, filename));
+    return fs.stat(this.resolve(filename));
 };
 
 // List all files in the book
@@ -702,9 +708,20 @@ Book.prototype.isEntryPoint = function(fp) {
     return fp == this.readmeFile;
 };
 
-// Resolve a path in book
-Book.prototype.resolve = function(p) {
-    return path.resolve(this.root, p);
+// Alias to book.config.get
+Book.prototype.getConfig = function(key, def) {
+    return this.config.get(key, def);
+};
+
+// Resolve a path in the book source
+// Enforce that the output path in the root folder
+Book.prototype.resolve = function() {
+    return pathUtil.resolveInRoot.apply(null, [this.root].concat(_.toArray(arguments)));
+};
+
+// 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 4b2035f..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,40 +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 = that.book.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) {
-        return path.resolve(path.dirname(from), to);
-    }
-});
-
 
 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/navigation.js b/lib/utils/navigation.js
index af9330d..d825c2c 100644
--- a/lib/utils/navigation.js
+++ b/lib/utils/navigation.js
@@ -27,7 +27,7 @@ function navigation(summary, files) {
     files = _.isArray(files) ? files : (_.isString(files) ? [files] : null);
 
     // List of all navNodes
-    // Flatten chapters, then add in default README node if needed etc ...
+    // Flatten chapters
     var navNodes = flattenChapters(summary.chapters);
 
     // Mapping of prev/next for a give path
@@ -39,8 +39,7 @@ function navigation(summary, files) {
         if(!current.exists) return null;
 
         // Find prev
-        prev = _.chain(navNodes)
-            .slice(0, i)
+        prev = _.chain(navNodes.slice(0, i))
             .reverse()
             .find(function(node) {
                 return node.exists && !node.external;
@@ -48,14 +47,12 @@ function navigation(summary, files) {
             .value();
 
         // Find next
-        next = _.chain(navNodes)
-            .slice(i+1)
+        next = _.chain(navNodes.slice(i+1))
             .find(function(node) {
                 return node.exists && !node.external;
             })
             .value();
 
-
         return [current.path, {
             index: i,
             title: current.title,
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
+};
diff --git a/package.json b/package.json
index 4baeec2..13d3259 100644
--- a/package.json
+++ b/package.json
@@ -7,13 +7,13 @@
     "dependencies": {
         "q": "1.0.1",
         "lunr": "0.5.7",
-        "lodash": "3.5.0",
+        "lodash": "3.10.1",
         "graceful-fs": "3.0.5",
         "resolve": "0.6.3",
         "fs-extra": "0.16.5",
         "fstream-ignore": "1.0.2",
         "gitbook-parsers": "0.8.2",
-        "gitbook-plugin-highlight": "1.0.0",
+        "gitbook-plugin-highlight": "1.0.1",
         "nunjucks": "mozilla/nunjucks#dc89bf91611a2101731c2c06afcf5c32160b4dc9",
         "nunjucks-autoescape": "1.0.0",
         "nunjucks-filter": "1.0.0",
@@ -36,8 +36,8 @@
         "escape-string-regexp": "1.0.3"
     },
     "devDependencies": {
-        "mocha": "2.2.1",
-        "should": "5.2.0",
+        "mocha": "2.3.2",
+        "should": "7.1.0",
         "grunt": "~0.4.2",
         "grunt-cli": "0.1.11",
         "grunt-contrib-copy": "0.5.0",
diff --git a/test/books/conrefs/README.md b/test/books/conrefs/README.md
index 7f56eed..804a77a 100644
--- a/test/books/conrefs/README.md
+++ b/test/books/conrefs/README.md
@@ -3,8 +3,10 @@
 ### Relative
 
 <p id="t1">{% include "./hello.md" %}</p>
+<p id="t2">{% include "/hello.md" %}</p>
 
 ### Git
 
-<p id="t2">{% include "git+https://gist.github.com/69ea4542e4c8967d2fa7.git/test.md" %}</p>
-<p id="t3">{% include "git+https://gist.github.com/69ea4542e4c8967d2fa7.git/test2.md" %}</p>
+<p id="t3">{% include "git+https://gist.github.com/69ea4542e4c8967d2fa7.git/test.md" %}</p>
+<p id="t4">{% include "git+https://gist.github.com/69ea4542e4c8967d2fa7.git/test2.md" %}</p>
+<p id="t5">{% include "git+https://gist.github.com/69ea4542e4c8967d2fa7.git/test3.md" %}</p>
diff --git a/test/configuration.js b/test/configuration.js
index eedec49..2cff26e 100644
--- a/test/configuration.js
+++ b/test/configuration.js
@@ -29,4 +29,12 @@ describe('Configuration', function () {
                 book.options.title.should.be.equal("js-config");
             });
     });
+
+    it('should provide configuration on book.config.get', function() {
+        return books.parse("basic")
+            .then(function(book) {
+                book.config.get('description').should.be.equal("Default description for the book.");
+                book.getConfig('description').should.be.equal("Default description for the book.");
+            });
+    });
 });
diff --git a/test/conrefs.js b/test/conrefs.js
index 8d6a181..7e044f5 100644
--- a/test/conrefs.js
+++ b/test/conrefs.js
@@ -26,15 +26,40 @@ describe('ConRefs', function () {
         });
     });
 
-    it('should handle git references', function() {
+    it('should handle local references with absolute paths', function() {
         readme.should.be.html({
             ".page-inner p#t2": {
                 count: 1,
-                text: "Hello from git",
+                text: "Hello World",
                 trim: true
-            },
+            }
+        });
+    });
+
+    it('should correctly include file from git reference', function() {
+        readme.should.be.html({
             ".page-inner p#t3": {
                 count: 1,
+                text: "Hello from git",
+                trim: true
+            }
+        });
+    });
+
+    it('should correctly handle deep include in git reference', function() {
+        readme.should.be.html({
+            ".page-inner p#t4": {
+                count: 1,
+                text: "First Hello. Hello from git",
+                trim: true
+            }
+        });
+    });
+
+    it('should correctly handle absolute include in git reference', function() {
+        readme.should.be.html({
+            ".page-inner p#t5": {
+                count: 1,
                 text: "First Hello. Hello from git",
                 trim: true
             }
diff --git a/test/ebook.js b/test/ebook.js
index 9b353d2..033fd04 100644
--- a/test/ebook.js
+++ b/test/ebook.js
@@ -37,12 +37,23 @@ describe('eBook generator', function () {
                 path.join(book.options.output, "index.html"),
                 { encoding: "utf-8" }
             );
+
+            // There are 2 styles (one from plugin-highlight and the new style)
             PAGE.should.be.html({
                 "link": {
-                    count: 1,
-                    attributes: {
-                        href: "./styles/print.css"
-                    }
+                    count: 2
+                }
+            });
+
+            PAGE.should.be.html({
+                "link[href='./styles/print.css']": {
+                    count: 1
+                }
+            });
+
+            PAGE.should.be.html({
+                "link[href='gitbook/plugins/gitbook-plugin-highlight/ebook.css']": {
+                    count: 1
                 }
             });
         });
diff --git a/test/json.js b/test/json.js
index 0e50237..758cfd7 100644
--- a/test/json.js
+++ b/test/json.js
@@ -35,7 +35,7 @@ describe('JSON generator', function () {
             it('should contains valid section', function() {
                 page.should.have.property("sections").with.lengthOf(1);
                 page.sections[0].should.have.property("content").which.is.a.String;
-                page.sections[0].should.have.property("type").which.is.a.String.which.equal("normal");
+                page.sections[0].should.have.property("type", "normal");
             });
 
             it('should contains valid progress', function() {
diff --git a/test/navigation.js b/test/navigation.js
new file mode 100644
index 0000000..ddcc86e
--- /dev/null
+++ b/test/navigation.js
@@ -0,0 +1,61 @@
+var should = require('should');
+
+describe('Navigation', function () {
+    var book;
+
+    before(function() {
+        return books.parse("summary")
+            .then(function(_book) {
+                book = _book;
+            });
+    });
+
+    it('should correctly parse navigation as a map', function() {
+        book.should.have.property("navigation");
+        book.navigation.should.have.property("README.md");
+        book.navigation.should.have.property("README.md");
+    });
+
+    it('should correctly include filenames', function() {
+        book.navigation.should.have.property("README.md");
+        book.navigation.should.have.property("PAGE1.md");
+        book.navigation.should.have.property("folder/PAGE2.md");
+        book.navigation.should.not.have.property("NOTFOUND.md");
+    });
+
+    it('should correctly detect next/prev for README', function() {
+        var README = book.navigation['README.md'];
+
+        README.index.should.equal(0);
+        README.should.have.property('next');
+        should(README.prev).not.be.ok();
+
+        README.next.should.have.property('path');
+        README.next.path.should.equal('PAGE1.md');
+    });
+
+    it('should correctly detect next/prev a page', function() {
+        var PAGE = book.navigation['PAGE1.md'];
+
+        PAGE.index.should.equal(1);
+        PAGE.should.have.property('next');
+        PAGE.should.have.property('prev');
+
+        PAGE.prev.should.have.property('path');
+        PAGE.prev.path.should.equal('README.md');
+
+        PAGE.next.should.have.property('path');
+        PAGE.next.path.should.equal('folder/PAGE2.md');
+    });
+
+    it('should correctly detect next/prev for last page', function() {
+        var PAGE = book.navigation['folder/PAGE2.md'];
+
+        PAGE.index.should.equal(2);
+        PAGE.should.have.property('prev');
+        should(PAGE.next).not.be.ok();
+
+        PAGE.prev.should.have.property('path');
+        PAGE.prev.path.should.equal('PAGE1.md');
+    });
+});
diff --git a/test/plugins.js b/test/plugins.js
index cc9c8e6..6d5b9de 100644
--- a/test/plugins.js
+++ b/test/plugins.js
@@ -90,7 +90,12 @@ describe('Plugins', function () {
 
             it('should extend books plugins', function() {
                 var resources = book.plugins.resources("ebook");
-                resources["css"].should.have.lengthOf(1);
+
+                // There is resources from highlight plugin and this plugin
+                resources["css"].should.have.lengthOf(2);
+                should.exist(_.find(resources["css"], {
+                    path: './resources/test'
+                }));
             });
         });
     });
diff --git a/test/resolve.js b/test/resolve.js
new file mode 100644
index 0000000..b31a10d
--- /dev/null
+++ b/test/resolve.js
@@ -0,0 +1,61 @@
+var fs = require('fs');
+var path = require('path');
+
+describe('Resolve Files', function () {
+    var book;
+
+    before(function() {
+        return books.parse("basic")
+            .then(function(_book) {
+                book = _book;
+            });
+    });
+
+    describe('book.fileIsInBook', function() {
+        it('should return true for correct paths', function() {
+            book.fileIsInBook(path.join(book.root, 'README.md')).should.equal(true);
+            book.fileIsInBook(path.join(book.root, 'styles/website.css')).should.equal(true);
+        });
+
+        it('should return true for root folder', function() {
+            book.fileIsInBook(path.join(book.root, './')).should.equal(true);
+            book.fileIsInBook(book.root).should.equal(true);
+        });
+
+        it('should return false for files out of scope', function() {
+            book.fileIsInBook(path.join(book.root, '../')).should.equal(false);
+            book.fileIsInBook('README.md').should.equal(false);
+            book.fileIsInBook(path.resolve(book.root, '../README.md')).should.equal(false);
+        });
+
+        it('should correctly handle windows paths', function() {
+            book.fileIsInBook(path.join(book.root, '\\styles\\website.css')).should.equal(true);
+        });
+    });
+
+    describe('book.resolve', function() {
+        it('should resolve a file to its absolute path', function() {
+            book.resolve('README.md').should.equal(path.resolve(book.root, 'README.md'));
+            book.resolve('website/README.md').should.equal(path.resolve(book.root, 'website/README.md'));
+        });
+
+        it('should correctly handle windows paths', function() {
+            book.resolve('styles\\website.css').should.equal(path.resolve(book.root, 'styles\\website.css'));
+        });
+
+        it('should correctly resolve all arguments', function() {
+            book.resolve('test', 'hello', '..', 'README.md').should.equal(path.resolve(book.root, 'test/README.md'));
+        });
+
+        it('should correctly resolve to root folder', function() {
+            book.resolve('test', '/README.md').should.equal(path.resolve(book.root, 'README.md'));
+            book.resolve('test', '\\README.md').should.equal(path.resolve(book.root, 'README.md'));
+        });
+
+        it('should throw an error for file out of book', function() {
+            (function() {
+                return book.resolve('../README.md');
+            }).should.throw();
+        });
+    });
+});