Skip to content

Commit

Permalink
Correctly handle invalid escapes in routes
Browse files Browse the repository at this point in the history
  • Loading branch information
@dead-claudia
dead-claudia committed Dec 31, 2017
1 parent 816178a commit 0fe0fd1
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 37 deletions.
29 changes: 29 additions & 0 deletions api/tests/test-router.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,35 @@ o.spec("route", function() {
o(root.firstChild.nodeName).equals("DIV")
})

o("renders default route when an invalid escape is in the route", function(done) {
$window.location.href = prefix + "/abc%def"
route(root, "/", {
"/" : {
view: function() {
return m("div")
}
},
"/abcdef" : {
view: function() {
return m("span")
}
},
"/abc%def" : {
view: function() {
return m("p")
}
}
})

callAsync(function() {
throttleMock.fire()

o(root.firstChild.nodeName).equals("P")

done()
})
})

o("routed mount points only redraw asynchronously (POJO component)", function() {
var view = o.spy()

Expand Down
1 change: 1 addition & 0 deletions docs/change-log.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

#### Bug fixes

- API: Invalid escapes in routes are now safely handled.
- API: `m.route.set()` causes all mount points to be redrawn ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592))
- API: Using style objects in hyperscript calls will now properly diff style properties from one render to another as opposed to re-writing all element style properties every render.
- core: `addEventListener` and `removeEventListener` are always used to manage event subscriptions, preventing external interference.
Expand Down
23 changes: 17 additions & 6 deletions router/router.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ module.exports = function($window) {
var callAsync = typeof setImmediate === "function" ? setImmediate : setTimeout

function normalize(fragment) {
var data = $window.location[fragment].replace(/(?:%[a-f89][a-f0-9])+/gim, decodeURIComponent)
var data = $window.location[fragment]
try { data = data.replace(/(?:%[a-f89][a-f0-9])+/gim, decodeURIComponent) } catch (e) { /* ignore */ }
if (fragment === "pathname" && data[0] !== "/") data = "/" + data
return data
}
Expand Down Expand Up @@ -42,11 +43,18 @@ module.exports = function($window) {

var router = {prefix: "#!"}
router.getPath = function() {
var type = router.prefix.charAt(0)
switch (type) {
case "#": return normalize("hash").slice(router.prefix.length)
case "?": return normalize("search").slice(router.prefix.length) + normalize("hash")
default: return normalize("pathname").slice(router.prefix.length) + normalize("search") + normalize("hash")
// The route might contain invalid escapes, and thus
// `decodeURIComponent` in `normalize` could throw (consider
// `/abc%def`). We need to handle that case accordingly.
try {
switch (router.prefix.charAt(0)) {
case "#": return normalize("hash").slice(router.prefix.length)
case "?": return normalize("search").slice(router.prefix.length) + normalize("hash")
default: return normalize("pathname").slice(router.prefix.length) + normalize("search") + normalize("hash")
}
} catch (e) {
if (e instanceof URIError) return null
throw e
}
}
router.setPath = function(path, data, options) {
Expand Down Expand Up @@ -78,6 +86,9 @@ module.exports = function($window) {
router.defineRoutes = function(routes, resolve, reject) {
function resolveRoute() {
var path = router.getPath()
// Easiest and most sensible way to handle invalid URLs: just
// trigger the default handler.
if (path == null) { reject(path, {}); return }
var params = {}
var pathname = parsePath(path, params, params)

Expand Down
70 changes: 48 additions & 22 deletions router/tests/test-defineRoutes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ o.spec("Router.defineRoutes", function() {
o("calls onRouteChange on init", function(done) {
$window.location.href = prefix + "/a"
router.defineRoutes({"/a": {data: 1}}, onRouteChange, onFail)

callAsync(function() {
o(onRouteChange.callCount).equals(1)

done()
})
})

o("resolves to route", function(done) {
$window.location.href = prefix + "/test"
router.defineRoutes({"/test": {data: 1}}, onRouteChange, onFail)
Expand All @@ -38,7 +38,7 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {}, "/test", "/test"])
o(onFail.callCount).equals(0)

done()
})
})
Expand All @@ -51,7 +51,33 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 2}, {"ö": "ö"}, "/ö?ö=ö#ö=ö", "/ö"])
o(onFail.callCount).equals(0)


done()
})
})

o("resolves to route w/ matching invalid escape", function(done) {
$window.location.href = prefix + "/abc%def"
router.defineRoutes({"/abcdef": {data: 2}, "/abc%def": {data: 3}}, onRouteChange, onFail)

callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 3}, {}, "/abc%def", "/abc%def"])
o(onFail.callCount).equals(0)

done()
})
})

o("resolves to route w/ non-matching invalid escape", function(done) {
$window.location.href = prefix + "/abc%def"
router.defineRoutes({"/abcdef": {data: 2}, "/nope": {data: 3}}, onRouteChange, onFail)

callAsync(function() {
o(onRouteChange.callCount).equals(0)
o(onFail.callCount).equals(1)
o(onFail.args).deepEquals(["/abc%def", {}])

done()
})
})
Expand All @@ -64,7 +90,7 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 2}, {"ö": "ö"}, "/ö?ö=ö#ö=ö", "/ö"])
o(onFail.callCount).equals(0)

done()
})
})
Expand All @@ -81,7 +107,7 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {}, "/test", "/test"])
o(onFail.callCount).equals(0)

done()
})
})
Expand All @@ -94,7 +120,7 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {a: "x"}, "/test/x", "/test/:a"])
o(onFail.callCount).equals(0)

done()
})
})
Expand All @@ -107,7 +133,7 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {a: "x", b: "y"}, "/test/x/y", "/test/:a/:b"])
o(onFail.callCount).equals(0)

done()
})
})
Expand All @@ -120,7 +146,7 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {a: "x/y"}, "/test/x/y", "/test/:a..."])
o(onFail.callCount).equals(0)

done()
})
})
Expand All @@ -133,7 +159,7 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {a: "b", c: "d"}, "/test?a=b&c=d", "/test"])
o(onFail.callCount).equals(0)

done()
})
})
Expand All @@ -146,7 +172,7 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {a: "b", c: "d"}, "/test#a=b&c=d", "/test"])
o(onFail.callCount).equals(0)

done()
})
})
Expand All @@ -159,7 +185,7 @@ o.spec("Router.defineRoutes", function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {a: "b", c: "d"}, "/test?a=b#c=d", "/test"])
o(onFail.callCount).equals(0)

done()
})
})
Expand All @@ -171,7 +197,7 @@ o.spec("Router.defineRoutes", function() {
callAsync(function() {
o(onFail.callCount).equals(1)
o(onFail.args).deepEquals(["/test", {}])

done()
})
})
Expand All @@ -183,7 +209,7 @@ o.spec("Router.defineRoutes", function() {
callAsync(function() {
o(onFail.callCount).equals(1)
o(onFail.args).deepEquals(["/test?a=b#c=d", {a: "b", c: "d"}])

done()
})
})
Expand All @@ -195,7 +221,7 @@ o.spec("Router.defineRoutes", function() {
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {}, "/z/y/x", "/z/y/x"])

done()
})
})
Expand All @@ -207,7 +233,7 @@ o.spec("Router.defineRoutes", function() {
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 2}, {a: "z/y/x"}, "/z/y/x", "/:a..."])

done()
})
})
Expand All @@ -223,7 +249,7 @@ o.spec("Router.defineRoutes", function() {
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {}, "/z/y/x", "/z/y/x"])

done()
})
})
Expand All @@ -239,7 +265,7 @@ o.spec("Router.defineRoutes", function() {
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 2}, {a: "z/y/x"}, "/z/y/x", "/:a..."])

done()
})
})
Expand All @@ -254,7 +280,7 @@ o.spec("Router.defineRoutes", function() {
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 1}, {}, "/z/y/x", "/z/y/x"])

done()
})
})
Expand All @@ -269,7 +295,7 @@ o.spec("Router.defineRoutes", function() {
callAsync(function() {
o(onRouteChange.callCount).equals(1)
o(onRouteChange.args).deepEquals([{data: 2}, {a: "z/y/x"}, "/z/y/x", "/:a..."])

done()
})
})
Expand All @@ -280,7 +306,7 @@ o.spec("Router.defineRoutes", function() {

callAsync(function() {
o(onRouteChange.callCount).equals(1)

done()
})
})
Expand Down
6 changes: 6 additions & 0 deletions router/tests/test-getPath.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ o.spec("Router.getPath", function() {

o(router.getPath()).equals("/ö?ö=ö#ö=ö")
})
o("gets route w/ invalid escape", function() {
$window.location.href = prefix + "/abc%def"
router.defineRoutes({"/test": {data: 1}, "/abcdef": {data: 2}}, onRouteChange, onFail)

o(router.getPath()).equals("/abc%def")
})
})
})
})
Expand Down
Loading

0 comments on commit 0fe0fd1

Please sign in to comment.