[Pkg-mozext-commits] [requestpolicy] 23/50: [fix] Ruleset: semantics of "endpoint specs"
David Prévot
taffit at moszumanska.debian.org
Mon Jun 27 12:50:31 UTC 2016
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch master
in repository requestpolicy.
commit 0e9f13f39beb58bad51c951929d15449e3862ce9
Author: Martin Kimmerle <dev at 256k.de>
Date: Sat Apr 23 14:49:24 2016 +0200
[fix] Ruleset: semantics of "endpoint specs"
This commit introduces a xpcshell test which defines the
semantics of the "rule specifications". It uncovered some
issues, which have been fixed. The following changes have been
made to the code:
* [improvement] "Any scheme" can now be specified by "*".
* [change] If the endpoint spec does not specify a host
. (host === undefined), then the port, if present, may be
. any port. (default-port-check only enabled if host is
. specified)
* [fix] Specifying the default port of a protocol explicitly
. is now possible. For example: {s: "http", port: "80"}
* [fix] If the endpoint spec defines a port, the rule should
. _not_ match URIs without a port.
---
src/content/lib/ruleset.jsm | 118 ++++++++++++++++----------------------
src/content/lib/script-loader.jsm | 2 +-
src/content/lib/utils/domains.jsm | 26 +++++++++
tests/xpcshell/test_rule_match.js | 113 ++++++++++++++++++++++++++++++++++++
tests/xpcshell/xpcshell.ini | 1 +
5 files changed, 189 insertions(+), 71 deletions(-)
diff --git a/src/content/lib/ruleset.jsm b/src/content/lib/ruleset.jsm
index 02a3492..07201b5 100644
--- a/src/content/lib/ruleset.jsm
+++ b/src/content/lib/ruleset.jsm
@@ -639,29 +639,52 @@ Rule.prototype = {
this.destinations = new Ruleset();
},
- isMatch: function(uriObj) {
- if (this.scheme && this.scheme !== uriObj.scheme) {
+ /**
+ * @param {nsIURI} uriObj
+ * @param {boolean} endpointSpecHasHost Whether the endpoint spec
+ * corresponding to this "Rule" instance has a host.
+ * @return {boolean}
+ */
+ isMatch: function(uriObj, endpointSpecHasHost) {
+ if (this.scheme && this.scheme !== "*" && this.scheme !== uriObj.scheme) {
//dprint("isMatch: wrong scheme (uri: '" + uriObj.scheme + "', rule: '" +
// this.scheme + "')");
return false;
}
// Check the port only in case the URI has a host at all.
- if (DomainUtil.uriObjHasHost(uriObj)) {
+ if (DomainUtil.uriObjHasPort(uriObj)) {
if (this.port) {
// If the rule's port is "*" it means any port. We use this convention
// because we assume an empty port in a rule means default ports rather
// than any port.
- if (parseInt(this.port, 10) !== uriObj.port && this.port !== "*") {
- //dprint("isMatch: wrong port (not the port specified by the rule)");
- return false;
+ if (this.port !== "*") {
+ let rulePort = parseInt(this.port, 10);
+ if (
+ rulePort === uriObj.port ||
+ uriObj.port === -1 &&
+ rulePort === DomainUtil.
+ getDefaultPortForScheme(uriObj.scheme)
+ ) {
+ // Port Match is OK, so continue
+ } else {
+ //dprint("isMatch: wrong port (not the port specified by the rule)");
+ return false;
+ }
}
} else {
- if (!DomainUtil.hasStandardPort(uriObj)) {
- //dprint("isMatch: wrong port (not the default port and the rule assumes default)");
- return false;
+ if (!endpointSpecHasHost) {
+ // Both host and port are undefined, so skip the default-port-check.
+ } else {
+ if (!DomainUtil.hasStandardPort(uriObj)) {
+ //dprint("isMatch: wrong port (not the default port and the rule assumes default)");
+ return false;
+ }
}
}
+ } else if (this.port) {
+ // The rule specifies a port, but the URI has no port.
+ return false;
}
if (this.path) {
@@ -925,7 +948,7 @@ Ruleset.prototype = {
// origin is non-host-specific but the destination doesn't
// have to be.
- yield this;
+ yield [this, false];
}
if (!host) {
@@ -935,7 +958,7 @@ Ruleset.prototype = {
if (DomainUtil.isIPAddress(host)) {
var addrEntry = this._ipAddr[host];
if (addrEntry) {
- yield addrEntry;
+ yield [addrEntry, true];
}
} else {
var parts = host.split(".");
@@ -947,12 +970,12 @@ Ruleset.prototype = {
break;
}
curLevel = nextLevel;
- yield nextLevel;
+ yield [nextLevel, true];
// Check for *.domain rules at each level.
nextLevel = curLevel.getLowerLevel("*");
if (nextLevel) {
- yield nextLevel;
+ yield [nextLevel, true];
}
}
}
@@ -983,11 +1006,11 @@ Ruleset.prototype = {
//dprint("Checking origin rules and origin-to-destination rules.");
// First, check for rules for each part of the origin host.
- originouterloop: for (let entry in this.getHostMatches(originHost)) {
+ for (let [entry, originSpecHasHost] in this.getHostMatches(originHost)) {
//dprint(entry);
for (let rule in entry.rules) {
//dprint("Checking rule: " + rule);
- let ruleMatchedOrigin = rule.isMatch(origin);
+ let ruleMatchedOrigin = rule.isMatch(origin, originSpecHasHost);
if (rule.allowOrigin && ruleMatchedOrigin) {
//dprint("ALLOW origin by rule " + entry + " " + rule);
@@ -997,31 +1020,18 @@ Ruleset.prototype = {
//dprint("DENY origin by rule " + entry + " " + rule);
matchedDenyRules.push(["origin", entry, rule]);
}
- // switch(rule.originRuleAction) {
- // case C.RULE_ACTION_ALLOW:
- // if (ruleMatchedOrigin) {
- // dprint("ALLOW origin by rule " + entry + " " + rule);
- // matchedAllowRules.push(["origin", entry, rule]);
- // }
- // break;
- // case C.RULE_ACTION_DENY:
- // if (ruleMatchedOrigin) {
- // dprint("DENY origin by rule " + entry + " " + rule);
- // matchedDenyRules.push(["origin", entry, rule]);
- // //break originouterloop;
- // break;
- // }
- // break;
- // }
+
// Check if there are origin-to-destination rules from the origin host
// entry we're currently looking at.
if (ruleMatchedOrigin && rule.destinations) {
//dprint("There are origin-to-destination rules using this origin rule.");
- for (let destEntry in rule.destinations.getHostMatches(destHost)) {
+ for (let [destEntry, destSpecHasHost]
+ in rule.destinations.getHostMatches(destHost)) {
//dprint(destEntry);
for (let destRule in destEntry.rules) {
//dprint("Checking rule: " + rule);
- if (destRule.allowDestination && destRule.isMatch(dest)) {
+ if (destRule.allowDestination &&
+ destRule.isMatch(dest, destSpecHasHost)) {
//dprint("ALLOW origin-to-dest by rule origin " + entry + " " + rule + " to dest " + destEntry + " " + destRule);
matchedAllowRules.push([
"origin-to-dest",
@@ -1031,7 +1041,8 @@ Ruleset.prototype = {
destRule
]);
}
- if (destRule.denyDestination && destRule.isMatch(dest)) {
+ if (destRule.denyDestination &&
+ destRule.isMatch(dest, destSpecHasHost)) {
//dprint("DENY origin-to-dest by rule origin " + entry + " " + rule + " to dest " + destEntry + " " + destRule);
matchedDenyRules.push([
"origin-to-dest",
@@ -1041,23 +1052,6 @@ Ruleset.prototype = {
destRule
]);
}
-
- // switch(destRule.destinationRuleAction) {
- // case C.RULE_ACTION_ALLOW:
- // if (destRule.isMatch(dest)) {
- // dprint("ALLOW origin-to-dest by rule origin " + entry + " " + rule + " to dest " + destEntry + " " + destRule);
- // matchedAllowRules.push(["origin-to-dest", entry, rule, destEntry, destRule]);
- // }
- // break;
- // case C.RULE_ACTION_DENY:
- // if (destRule.isMatch(dest)) {
- // dprint("DENY origin-to-dest by rule origin " + entry + " " + rule + " to dest " + destEntry + " " + destRule);
- // matchedDenyRules.push(["origin-to-dest", entry, rule, destEntry, destRule]);
- // //break originouterloop;
- // break;
- // }
- // break;
- // }
}
}
//dprint("Done checking origin-to-destination rules using this origin rule.");
@@ -1067,34 +1061,18 @@ Ruleset.prototype = {
//dprint("Checking dest rules.");
// Last, check for rules for each part of the destination host.
- destouterloop: for (let entry in this.getHostMatches(destHost)) {
+ for (let [entry, destSpecHasHost] in this.getHostMatches(destHost)) {
//dprint(entry);
for (let rule in entry.rules) {
//dprint("Checking rule: " + rule);
- if (rule.allowDestination && rule.isMatch(dest)) {
+ if (rule.allowDestination && rule.isMatch(dest, destSpecHasHost)) {
//dprint("ALLOW dest by rule " + entry + " " + rule);
matchedAllowRules.push(["dest", entry, rule]);
}
- if (rule.denyDestination && rule.isMatch(dest)) {
+ if (rule.denyDestination && rule.isMatch(dest, destSpecHasHost)) {
//dprint("DENY dest by rule " + entry + " " + rule);
matchedDenyRules.push(["dest", entry, rule]);
}
- // switch(rule.destinationRuleAction) {
- // case C.RULE_ACTION_ALLOW:
- // if (rule.isMatch(dest)) {
- // dprint("ALLOW dest by rule " + entry + " " + rule);
- // matchedAllowRules.push(["dest", entry, rule]);
- // }
- // break;
- // case C.RULE_ACTION_DENY:
- // if (rule.isMatch(dest)) {
- // dprint("DENY dest by rule " + entry + " " + rule);
- // matchedDenyRules.push(["dest", entry, rule]);
- // //break destouterloop;
- // break;
- // }
- // break;
- // }
}
}
diff --git a/src/content/lib/script-loader.jsm b/src/content/lib/script-loader.jsm
index d83d3e4..80ac91f 100644
--- a/src/content/lib/script-loader.jsm
+++ b/src/content/lib/script-loader.jsm
@@ -63,7 +63,7 @@ function logSevereError(aMessage, aError) {
let console = {
debug: Services.console.logStringMessage,
error: Services.console.logStringMessage
-}
+};
//==============================================================================
// ScriptLoader
diff --git a/src/content/lib/utils/domains.jsm b/src/content/lib/utils/domains.jsm
index a0a2a18..682057b 100644
--- a/src/content/lib/utils/domains.jsm
+++ b/src/content/lib/utils/domains.jsm
@@ -141,6 +141,19 @@ DomainUtil.uriObjHasHost = function(aUriObj) {
};
/**
+ * @param {nsIURI} aUriObj
+ * @return {boolean}
+ */
+DomainUtil.uriObjHasPort = function(aUriObj) {
+ try {
+ aUriObj.port; // jshint ignore:line
+ return true;
+ } catch (e) {
+ return false;
+ }
+};
+
+/**
* Returns an nsIURI object from a uri string. Note that nsIURI objects will
* automatically convert ACE formatting to UTF8 for IDNs in the various
* attributes of the object that are available.
@@ -353,3 +366,16 @@ DomainUtil.hasStandardPort = function(uri) {
uri.port === 80 && uri.scheme === "http" ||
uri.port === 443 && uri.scheme === "https";
};
+
+DomainUtil.getDefaultPortForScheme = function(scheme) {
+ switch (scheme) {
+ case "http":
+ return 80;
+ case "https":
+ return 443;
+ default:
+ Logger.warning(Logger.TYPE_INTERNAL,
+ "Unknown default port for scheme " + scheme + ".");
+ return null;
+ }
+};
diff --git a/tests/xpcshell/test_rule_match.js b/tests/xpcshell/test_rule_match.js
new file mode 100644
index 0000000..b4510b0
--- /dev/null
+++ b/tests/xpcshell/test_rule_match.js
@@ -0,0 +1,113 @@
+/* exported run_test */
+Components.utils.import("resource://gre/modules/Services.jsm");
+Components.utils.import("chrome://rpcontinued/content/lib/ruleset.jsm");
+Components.utils.import("chrome://rpcontinued/content/lib/utils/constants.jsm");
+
+function run_test() {
+ run_next_test();
+}
+
+add_test(function() {
+ function testRuleSpec(ruleSpec, shouldMatch, originUriSpec, destUriSpec) {
+ // SETUP
+ // create a Ruleset
+ let rawRuleset = new RawRuleset();
+ let ruleset = new Ruleset();
+ // Add the rule to the ruleset
+ let ruleAction = C.RULE_ACTION_ALLOW; // Doesn't matter
+ rawRuleset.addRule(ruleAction, ruleSpec, ruleset);
+ // create nsIURI
+ let originUri = Services.io.newURI(originUriSpec, null, null);
+ let destUri = Services.io.newURI(destUriSpec, null, null);
+
+ // EXERCISE
+ let [matchedAllowRules, ] = ruleset.check(originUri, destUri);
+
+ // VERIFY
+ strictEqual(shouldMatch ? 1 : 0, matchedAllowRules.length);
+ }
+
+ function test(endpointSpec, shouldMatch, uriSpec) {
+ testRuleSpec({o: endpointSpec}, shouldMatch, uriSpec, "dummy://destination/uri");
+ testRuleSpec({d: endpointSpec}, shouldMatch, "dummy://origin/uri", uriSpec);
+ }
+
+ // anything
+ test({ }, true, "http://localhost/");
+ test({ }, true, "http://localhost:8080/");
+ test({ }, true, "about:blank");
+ test({ }, true, "file:///etc/hosts");
+
+ // Specific port
+ test({ port: 81 }, true, "http://localhost:81");
+ test({ port: 81 }, false, "http://localhost");
+ test({ port: 80 }, true, "http://localhost");
+ test({ port: 81 }, false, "about:blank");
+ test({ port: 81 }, false, "file:///etc/hosts");
+
+ // Default port
+ test({ port: -1 }, true, "http://localhost");
+ test({ port: -1 }, false, "http://localhost:81");
+ test({ port: -1 }, true, "https://localhost");
+ test({ port: -1 }, false, "https://localhost:82");
+ test({ port: -1 }, false, "about:blank");
+ test({ port: -1 }, true, "file:///etc/hosts");
+
+ // Any port
+ test({ port: "*"}, true, "http://localhost");
+ test({ port: "*"}, true, "http://localhost:81");
+ test({ port: "*"}, true, "https://localhost");
+ test({ port: "*"}, true, "https://localhost:82");
+ test({ port: "*"}, false, "about:blank");
+ test({ port: "*"}, true, "file:///etc/hosts");
+
+ // Host + Default port
+ test({ h: "www.example.com" }, true, "http://www.example.com/");
+ test({ h: "www.example.com" }, false, "http://www.example.com:81/");
+
+ // Host + Specific port
+ test({ h: "www.example.com", port: 81 }, false, "http://www.example.com/");
+ test({ h: "www.example.com", port: 81 }, true, "http://www.example.com:81/");
+
+ // Host + Any port
+ test({ h: "www.example.com", port: "*"}, true, "http://www.example.com/");
+ test({ h: "www.example.com", port: "*"}, true, "http://www.example.com:81/");
+
+ // FIXME
+ // // Host non-existent
+ // test({ h: null }, false, "http://localhost/");
+ // test({ h: null }, false, "http://localhost:8080/");
+ // test({ h: null }, true, "about:blank");
+ // test({ h: null }, false, "file:///etc/hosts");
+
+ // FIXME
+ // // Host empty
+ // test({ h: "" }, false, "http://localhost/");
+ // test({ h: "" }, false, "http://localhost:8080/");
+ // test({ h: "" }, false, "about:blank");
+ // test({ h: "" }, true, "file:///etc/hosts");
+
+ // FIXME; see PR #555
+ // // Any Host + Default port
+ // test({ h: "*" }, true, "http://www.example.com/");
+ // test({ h: "*" }, true, "http://localhost/");
+ // test({ h: "*" }, false, "http://www.example.com:81/");
+ // test({ h: "*" }, false, "http://localhost:81/");
+
+ // Specific Scheme
+ test({s: "http" }, true, "http://localhost/");
+ test({s: "http" }, true, "http://localhost:8080/");
+ test({s: "http" }, false, "about:blank");
+ test({s: "http" }, false, "file:///etc/hosts");
+ test({s: "http" }, false, "https://localhost/");
+ test({s: "about" }, true, "about:blank");
+ test({s: "about" }, false, "http://localhost/");
+
+ // Any Scheme
+ test({s: "*" }, true, "http://localhost/");
+ test({s: "*" }, true, "http://localhost:8080/");
+ test({s: "*" }, true, "about:blank");
+ test({s: "*" }, true, "file:///etc/hosts");
+
+ run_next_test();
+});
diff --git a/tests/xpcshell/xpcshell.ini b/tests/xpcshell/xpcshell.ini
index e040452..84db432 100644
--- a/tests/xpcshell/xpcshell.ini
+++ b/tests/xpcshell/xpcshell.ini
@@ -9,6 +9,7 @@ tail =
;[test_policymanager.js]
[test_policystorage.js]
[test_requestresult.js]
+[test_rule_match.js]
[test_ruleutils.js]
;[test_subscription.js]
[test_wrap_function.js]
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-mozext/requestpolicy.git
More information about the Pkg-mozext-commits
mailing list