Enforce a router's root as its mount point

src/app/HISTORY.md

@@ -4,7 +4,30 @@ App Framework Change History
 @VERSION@
 ------
 
-* No changes.
+### Router
+
+* __[!]__ A router's `root` path is now enforced as its mount point. Routers
+  with a specified `root` will only consider paths as matching its route handles
+  if that path is semantically within the router's `root` path. For example:
+
+    ```js
+    router.set('root', '/app/');
+
+    router.hasRoute('/app/');    // => true
+    router.hasRoute('/app/foo'); // => true
+    router.hasRoute('/bar/');    // => false
+    ```
+
+  This fixed some issues with paths being erroneously considered matching even
+  when they were not semantically within the router's `root` path. ([#1083][])
+
+* __[!]__ The `getPath()` method now returns the _full_ current path, whereas
+  before it returned the current path relative to the router's `root`. This also
+  affects `req.path` which is now the full path as well.
+
+
+[#1083]: https://github.com/yui/yui3/issues/1083
+
 
 3.12.0
 ------

src/app/docs/router/index.mustache

@@ -269,6 +269,23 @@ This is where the `root` config attribute comes in. If you set `root` to `'/mysi
   </tbody>
 </table>
 
+<p>
+The `root` path also acts a mount point for the router and will only consider paths that fall under the `root` (when set). The following example demonstrates these semantics:
+</p>
+
+```js
+router.set('root', '/mysite/');
+
+router.route('/',    function () { alert('Hello!'); });
+router.route('/pie', function () { alert('Mmm. Pie.'); });
+
+Y.log(router.hasRoute('/'));           // => false
+Y.log(router.hasRoute('/pie'));        // => false
+
+Y.log(router.hasRoute('/mysite/'));    // => true
+Y.log(router.hasRoute('/mysite/pie')); // => true
+```
+
 <h3>Extending `Y.Router`</h3>
 
 <p>

src/app/js/router.js

@@ -247,7 +247,7 @@ Y.Router = Y.extend(Router, Y.Base, {
     },
 
     /**
-    Gets the current route path, relative to the `root` (if any).
+    Gets the current route path.
 
     @method getPath
     @return {String} Current route path.
@@ -280,14 +280,18 @@ Y.Router = Y.extend(Router, Y.Base, {
             url = this._upgradeURL(url);
         }
 
-        path = this.removeQuery(this.removeRoot(url));
+        // Get just the path portion of the specified `url`.
+        path = this.removeQuery(url.replace(this._regexUrlOrigin, ''));
 
         return !!this.match(path).length;
     },
 
     /**
     Returns an array of route objects that match the specified URL path.
 
+    If this router has a `root`, then the specified `path` _must_ be
+    semantically within the `root` path to match any routes.
+
     This method is called internally to determine which routes match the current
     path whenever the URL changes. You may override it if you want to customize
     the route matching logic, although this usually shouldn't be necessary.
@@ -312,10 +316,26 @@ Y.Router = Y.extend(Router, Y.Base, {
         // => [{callback: ..., keys: [], path: '/foo', regex: ...}]
 
     @method match
-    @param {String} path URL path to match.
+    @param {String} path URL path to match. This should be an absolute path that
+        starts with a slash: "/".
     @return {Object[]} Array of route objects that match the specified path.
     **/
     match: function (path) {
+        var root = this.get('root');
+
+        if (root) {
+            // The `path` must be semantically within this router's `root` path
+            // or mount point, if it's not then no routes should be considered a
+            // match.
+            if (!this._pathHasRoot(root, path)) {
+                return [];
+            }
+
+            // Remove this router's `root` from the `path` before checking the
+            // routes for any matches.
+            path = this.removeRoot(path);
+        }
+
         return YArray.filter(this._routes, function (route) {
             return path.search(route.regex) > -1;
         });
@@ -392,13 +412,23 @@ Y.Router = Y.extend(Router, Y.Base, {
     @return {String} Rootless path.
     **/
     removeRoot: function (url) {
-        var root = this.get('root');
+        var root = this.get('root'),
+            path;
 
         // Strip out the non-path part of the URL, if any (e.g.
         // "http://foo.com"), so that we're left with just the path.
         url = url.replace(this._regexUrlOrigin, '');
 
-        if (root && url.indexOf(root) === 0) {
+        // Return the host-less URL if there's no `root` path to further remove.
+        if (!root) {
+            return url;
+        }
+
+        path = this.removeQuery(url);
+
+        // Remove the `root` from the `url` if it's the same or its path is
+        // semantically within the root path.
+        if (path === root || this._pathHasRoot(root, path)) {
             url = url.substring(root.length);
         }
 
@@ -864,7 +894,7 @@ Y.Router = Y.extend(Router, Y.Base, {
     },
 
     /**
-    Gets the current route path, relative to the `root` (if any).
+    Gets the current route path.
 
     @method _getPath
     @return {String} Current route path.
@@ -874,7 +904,7 @@ Y.Router = Y.extend(Router, Y.Base, {
         var path = (!this._html5 && this._getHashPath()) ||
                 Y.getLocation().pathname;
 
-        return this.removeQuery(this.removeRoot(path));
+        return this.removeQuery(path);
     },
 
     /**
@@ -1151,6 +1181,38 @@ Y.Router = Y.extend(Router, Y.Base, {
         return result;
     },
 
+    /**
+    Returns `true` when the specified `path` is semantically within the
+    specified `root` path.
+
+    If the `root` does not end with a trailing slash ("/"), one will be added
+    before the `path` is evaluated against the root path.
+
+    @example
+        this._pathHasRoot('/app',  '/app/foo'); // => true
+        this._pathHasRoot('/app/', '/app/foo'); // => true
+        this._pathHasRoot('/app/', '/app/');    // => true
+
+        this._pathHasRoot('/app',  '/foo/bar'); // => false
+        this._pathHasRoot('/app/', '/foo/bar'); // => false
+        this._pathHasRoot('/app/', '/app');     // => false
+        this._pathHasRoot('/app',  '/app');     // => false
+
+    @method _pathHasRoot
+    @param {String} root Root path used to evaluate whether the specificed
+        `path` is semantically within. A trailing slash ("/") will be added if
+        it does not already end with one.
+    @param {String} path Path to evaluate for containing the specified `root`.
+    @return {Boolean} Whether or not the `path` is semantically within the
+        `root` path.
+    @protected
+    @since @SINCE@
+    **/
+    _pathHasRoot: function (root, path) {
+        var rootPath = root.charAt(root.length - 1) === '/' ? root : root + '/';
+        return path.indexOf(rootPath) === 0;
+    },
+
     /**
     Queues up a `_save()` call to run after all previously-queued calls have
     finished.
@@ -1519,6 +1581,16 @@ Y.Router = Y.extend(Router, Y.Base, {
         evaluated relative to that root URL, so the `/` route would then execute
         when the user browses to `http://example.com/myapp/`.
 
+        @example
+            router.set('root', '/myapp');
+            router.route('/foo', function () { ... });
+
+            Y.log(router.hasRoute('/foo'));       // => false
+            Y.log(router.hasRoute('/myapp/foo')); // => true
+
+            // Updates the URL to: "/myapp/foo"
+            router.save('/foo');
+
         @attribute root
         @type String
         @default `''`

src/app/tests/unit/assets/router-test.js

@@ -387,6 +387,32 @@ routerSuite.add(new Y.Test.Case({
         Assert.areSame(two, routes[1].callbacks[0]);
     },
 
+    'match() should return an array of routes that match the given path under a `root`': function () {
+        var router = this.router = new Y.Router(),
+            routes;
+
+        function one() {}
+        function two() {}
+        function three() {}
+
+        router.set('root', '/app/');
+
+        router.route('/:foo', one);
+        router.route(/foo/, two);
+        router.route('/bar', three);
+
+        Assert.areSame(0, router.match('/').length);
+        Assert.areSame(0, router.match('/foo').length);
+        Assert.areSame(0, router.match('/foo/').length);
+        Assert.areSame(0, router.match('/bar').length);
+
+        routes = router.match('/app/foo');
+
+        Assert.areSame(2, routes.length);
+        Assert.areSame(one, routes[0].callbacks[0]);
+        Assert.areSame(two, routes[1].callbacks[0]);
+    },
+
     'hasRoute() should return `true` if one or more routes match the given path': function () {
         var router = this.router = new Y.Router(),
             router2 = new Y.Router();
@@ -417,6 +443,53 @@ routerSuite.add(new Y.Test.Case({
         router2.destroy();
     },
 
+    'hasRoute() should always return `false` for paths not under the `root`': function () {
+        var location = Y.getLocation(),
+            router1  = new Y.Router({root: '/app'}),
+            router2  = new Y.Router({root: '/app/'});
+
+        function noop () {}
+
+        router1.route('/', noop);
+        router1.route('/:foo', noop);
+        router1.route(/foo/, noop);
+        router1.route('/bar', noop);
+
+        router2.route('/', noop);
+        router2.route('/:foo', noop);
+        router2.route(/foo/, noop);
+        router2.route('/bar', noop);
+
+        Assert.isFalse(router1.hasRoute('/'));
+        Assert.isFalse(router1.hasRoute('/app'));
+        Assert.isFalse(router1.hasRoute('/foo/'));
+        Assert.isFalse(router1.hasRoute('/bar'));
+        Assert.isFalse(router1.hasRoute('/baz'));
+        Assert.isFalse(router1.hasRoute(router1._getOrigin() + '/bar'));
+
+        Assert.isFalse(router2.hasRoute('/'));
+        Assert.isFalse(router2.hasRoute('/app'));
+        Assert.isFalse(router2.hasRoute('/foo/'));
+        Assert.isFalse(router2.hasRoute('/bar'));
+        Assert.isFalse(router2.hasRoute('/baz'));
+        Assert.isFalse(router2.hasRoute(router2._getOrigin() + '/bar'));
+
+        Assert.isTrue(router1.hasRoute('/app/'));
+        Assert.isTrue(router1.hasRoute('/app/foo/'));
+        Assert.isTrue(router1.hasRoute('/app/bar'));
+        Assert.isTrue(router1.hasRoute('/app/baz'));
+        Assert.isTrue(router1.hasRoute(router1._getOrigin() + '/app/bar'));
+
+        Assert.isTrue(router2.hasRoute('/app/'));
+        Assert.isTrue(router2.hasRoute('/app/foo/'));
+        Assert.isTrue(router2.hasRoute('/app/bar'));
+        Assert.isTrue(router2.hasRoute('/app/baz'));
+        Assert.isTrue(router2.hasRoute(router2._getOrigin() + '/app/bar'));
+
+        router1.destroy();
+        router2.destroy();
+    },
+
     'hasRoute() should support full URLs': function () {
         var router = this.router = new Y.Router(),
             loc    = win && win.location,
@@ -482,7 +555,7 @@ routerSuite.add(new Y.Test.Case({
 
         router.route('/hashpath', function (req) {
             test.resume(function () {
-                Assert.areSame('/hashpath', req.path);
+                Assert.areSame('/foo/hashpath', req.path);
                 Assert.areSame(Y.getLocation().pathname, root + 'hashpath');
             });
         });
@@ -497,15 +570,28 @@ routerSuite.add(new Y.Test.Case({
         router.set('root', '/');
         Assert.areSame('/bar', router.removeRoot('/bar'));
         Assert.areSame('/bar', router.removeRoot('bar'));
+        Assert.areSame('/bar/', router.removeRoot('/bar/'));
+        Assert.areSame('/bar/', router.removeRoot('bar/'));
 
         router.set('root', '/foo');
+        Assert.areSame('/', router.removeRoot('/foo'));
+        Assert.areSame('/', router.removeRoot('/foo/'));
         Assert.areSame('/bar', router.removeRoot('/foo/bar'));
+        Assert.areSame('/bar/', router.removeRoot('/foo/bar/'));
+        Assert.areSame('/foobar', router.removeRoot('/foobar'));
+        Assert.areSame('/foobar/', router.removeRoot('/foobar/'));
 
         router.set('root', '/foo/');
+        Assert.areSame('/foo', router.removeRoot('/foo'));
+        Assert.areSame('/', router.removeRoot('/foo/'));
         Assert.areSame('/bar', router.removeRoot('/foo/bar'));
+        Assert.areSame('/bar/', router.removeRoot('/foo/bar/'));
+        Assert.areSame('/foobar', router.removeRoot('/foobar'));
+        Assert.areSame('/foobar/', router.removeRoot('/foobar/'));
 
         router.set('root', '/moo');
         Assert.areSame('/foo/bar', router.removeRoot('/foo/bar'));
+        Assert.areSame('/foo/moo', router.removeRoot('/foo/moo'));
     },
 
     'removeRoot() should strip the origin ("http://foo.com") portion of the URL, if any': function () {
@@ -653,7 +739,7 @@ routerSuite.add(new Y.Test.Case({
         router.set('root', pathRoot);
         router.route('/save', function (req) {
             test.resume(function () {
-                Assert.areSame('/save', req.path);
+                Assert.areSame(router._joinURL('/save'), req.path);
                 Assert.areSame('/save', Y.HistoryHash.getHash());
             });
         });
@@ -674,7 +760,7 @@ routerSuite.add(new Y.Test.Case({
         router.set('root', '/app');
         router.route('/save', function (req) {
             test.resume(function () {
-                Assert.areSame('/save', req.path);
+                Assert.areSame('/app/save', req.path);
                 Assert.areSame('/app/save', Y.HistoryHash.getHash());
             });
         });