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 committed Jan 8, 2018
1 parent 816178a commit 0a5ead3
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 34 deletions.
29 changes: 29 additions & 0 deletions api/tests/test-router.js
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
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
9 changes: 6 additions & 3 deletions router/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ 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]
// The route might contain invalid escapes, and thus
// `decodeURIComponent` could throw (consider `/abc%def`). We need to
// handle that case accordingly.
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,8 +46,7 @@ module.exports = function($window) {

var router = {prefix: "#!"}
router.getPath = function() {
var type = router.prefix.charAt(0)
switch (type) {
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")
Expand Down
70 changes: 48 additions & 22 deletions router/tests/test-defineRoutes.js
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
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
30 changes: 21 additions & 9 deletions router/tests/test-setPath.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ o.spec("Router.setPath", function() {
router.setPath("/other/x/y/z?c=d#e=f")

o(router.getPath()).equals("/other/x/y/z?c=d#e=f")

done()
})
})
Expand All @@ -67,7 +67,19 @@ o.spec("Router.setPath", function() {
router.setPath("/%C3%B6?%C3%B6=%C3%B6#%C3%B6=%C3%B6")

o(router.getPath()).equals("/ö?ö=ö#ö=ö")


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

callAsync(function() {
router.setPath("/abc%def")

o(router.getPath()).equals("/abc%def")

done()
})
})
Expand All @@ -79,7 +91,7 @@ o.spec("Router.setPath", function() {
router.setPath("/ö?ö=ö#ö=ö")

o(router.getPath()).equals("/ö?ö=ö#ö=ö")

done()
})
})
Expand All @@ -96,7 +108,7 @@ o.spec("Router.setPath", function() {
router.setPath("/other/x/y/z?c=d#e=f")

o(router.getPath()).equals("/other/x/y/z?c=d#e=f")

done()
})
})
Expand All @@ -109,7 +121,7 @@ o.spec("Router.setPath", function() {
$window.onpopstate()

o(router.getPath()).equals("/other/x/y/z?c=d#e=f")

done()
})
})
Expand All @@ -121,7 +133,7 @@ o.spec("Router.setPath", function() {
router.setPath("/other/:a/:b", {a: "x", b: "y/z", c: "d", e: "f"})

o(router.getPath()).equals("/other/x/y/z?c=d&e=f")

done()
})
})
Expand All @@ -134,7 +146,7 @@ o.spec("Router.setPath", function() {
$window.history.back()

o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + "/")

done()
})
})
Expand All @@ -149,7 +161,7 @@ o.spec("Router.setPath", function() {
var slash = prefix[0] === "/" ? "" : "/"

o($window.location.href).equals(env.protocol + "//" + (env.hostname === "/" ? "" : env.hostname) + slash + (prefix ? prefix + "/" : "") + "test")

done()
})
})
Expand All @@ -161,7 +173,7 @@ o.spec("Router.setPath", function() {
router.setPath("/other", null, {state: {a: 1}})

o($window.history.state).deepEquals({a: 1})

done()
})
})
Expand Down

0 comments on commit 0a5ead3

Please sign in to comment.