Skip to content
Browse files

Merge branch 'router-decode-path'

  • Loading branch information...
2 parents 04aa96d + 9ddf37c commit bd2cb9b1e405d3ec49910f39ebe7aa34c35c4676 @ericf ericf committed Nov 9, 2012
Showing with 75 additions and 18 deletions.
  1. +8 −0 src/app/HISTORY.md
  2. +20 −10 src/app/js/router.js
  3. +47 −8 src/app/tests/unit/assets/router-test.js
View
8 src/app/HISTORY.md
@@ -1,6 +1,14 @@
App Framework Change History
============================
+3.8.0
+-----
+
+### Router
+
+* Decode URL-encoded path matches for Router's `req.params`. [Ticket #2532941]
+
+
3.7.3
-----
View
30 src/app/js/router.js
@@ -9,7 +9,6 @@ Provides URL-based routing using HTML5 `pushState()` or the location hash.
var HistoryHash = Y.HistoryHash,
QS = Y.QueryString,
YArray = Y.Array,
- YLang = Y.Lang,
win = Y.config.win,
@@ -197,7 +196,9 @@ Y.Router = Y.extend(Router, Y.Base, {
instances.splice(instanceIndex, 1);
}
- this._historyEvents && this._historyEvents.detach();
+ if (this._historyEvents) {
+ this._historyEvents.detach();
+ }
},
// -- Public Methods -------------------------------------------------------
@@ -619,6 +620,7 @@ Y.Router = Y.extend(Router, Y.Base, {
**/
_dispatch: function (path, url, src) {
var self = this,
+ decode = self._decode,
routes = self.match(path),
callbacks = [],
matches, req, res;
@@ -657,9 +659,13 @@ Y.Router = Y.extend(Router, Y.Base, {
callback.call(self, req, res, req.next);
} else if ((route = routes.shift())) {
- // Make a copy of this route's `callbacks` and find its matches.
+ // Make a copy of this route's `callbacks` so the original array
+ // is preserved.
callbacks = route.callbacks.concat();
- matches = route.regex.exec(path);
+
+ // Decode each of the path matches so that the any URL-encoded
+ // path segments are decoded in the `req.params` object.
+ matches = YArray.map(route.regex.exec(path) || [], decode);
// Use named keys for parameter names if the route path contains
// named keys. Otherwise, use numerical match indices.
@@ -669,7 +675,7 @@ Y.Router = Y.extend(Router, Y.Base, {
req.params = matches.concat();
}
- // Allow access tot he num of remaining routes for this request.
+ // Allow access to the num of remaining routes for this request.
req.pendingRoutes = routes.length;
// Execute this route's `callbacks`.
@@ -1150,7 +1156,9 @@ Y.Router = Y.extend(Router, Y.Base, {
}
// Joins the `url` with the `root`.
- urlIsString && (url = this._joinURL(url));
+ if (urlIsString) {
+ url = this._joinURL(url);
+ }
// Force _ready to true to ensure that the history change is handled
// even if _save is called before the `ready` event fires.
@@ -1234,11 +1242,13 @@ Y.Router = Y.extend(Router, Y.Base, {
hash = hash.replace(hashPrefix, '');
}
- hash && (hashPath = this._getHashPath(hash));
-
// If the hash looks like a URL path, assume it is, and upgrade it!
- if (hashPath) {
- return this._resolveURL(hashPath);
+ if (hash) {
+ hashPath = this._getHashPath(hash);
+
+ if (hashPath) {
+ return this._resolveURL(hashPath);
+ }
}
return url;
View
55 src/app/tests/unit/assets/router-test.js
@@ -12,10 +12,12 @@ var ArrayAssert = Y.ArrayAssert,
routerSuite;
function resetURL() {
+ if (!win) { return; }
+
if (html5) {
- win && win.history.replaceState(null, null, originalURL);
+ win.history.replaceState(null, null, originalURL);
} else {
- win && (win.location.hash = '');
+ win.location.hash = '';
}
}
@@ -40,7 +42,10 @@ routerSuite.add(new Y.Test.Case({
name: 'Lifecycle',
tearDown: function () {
- this.router && this.router.destroy();
+ if (this.router) {
+ this.router.destroy();
+ }
+
delete this.router;
},
@@ -96,7 +101,10 @@ routerSuite.add(new Y.Test.Case({
name: 'Attributes',
tearDown: function () {
- this.router && this.router.destroy();
+ if (this.router) {
+ this.router.destroy();
+ }
+
delete this.router;
},
@@ -167,7 +175,10 @@ routerSuite.add(new Y.Test.Case({
name: 'Events',
tearDown: function () {
- this.router && this.router.destroy();
+ if (this.router) {
+ this.router.destroy();
+ }
+
delete this.router;
},
@@ -218,7 +229,10 @@ routerSuite.add(new Y.Test.Case({
},
tearDown: function () {
- this.router && this.router.destroy();
+ if (this.router) {
+ this.router.destroy();
+ }
+
delete this.router;
Y.config.errorFn = this.errorFn;
@@ -732,8 +746,13 @@ routerSuite.add(new Y.Test.Case({
name: 'Routes',
tearDown: function () {
- this.router && this.router.destroy();
- this.router2 && this.router2.destroy();
+ if (this.router) {
+ this.router.destroy();
+ }
+
+ if (this.router2) {
+ this.router2.destroy();
+ }
delete this.router;
delete this.router2;
@@ -933,6 +952,26 @@ routerSuite.add(new Y.Test.Case({
Assert.areSame(3, calls);
},
+ 'route parameters should be decoded': function () {
+ var calls = 0,
+ router = this.router = new Y.Router(),
+ pathSegment = 'path with spaces';
+
+ router.route('/:foo', function (req) {
+ calls += 1;
+
+ ArrayAssert.itemsAreSame(['foo'], Y.Object.keys(req.params));
+ ArrayAssert.itemsAreSame([pathSegment], Y.Object.values(req.params));
+ });
+
+ Assert.areSame('path%20with%20spaces', encodeURIComponent(pathSegment));
+
+ router._dispatch('/' + pathSegment, {});
+ router._dispatch('/' + encodeURIComponent(pathSegment), {});
+
+ Assert.areSame(2, calls);
+ },
+
'request object should contain a `pendingRoutes` property': function () {
var calls = 0,
router = this.router = new Y.Router();

0 comments on commit bd2cb9b

Please sign in to comment.
Something went wrong with that request. Please try again.