[Pkg-mozext-commits] [requestpolicy] 08/50: [ref] endpointSpecToDisplayString
David Prévot
taffit at moszumanska.debian.org
Mon Jun 27 12:50:29 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 15bde2151c6efd5043f3f718ce68c9282dd2051d
Author: Martin Kimmerle <dev at 256k.de>
Date: Mon Mar 28 01:01:16 2016 +0200
[ref] endpointSpecToDisplayString
Some refactoring, and some changes to the display string.
Instead of `scheme "${scheme}"`, now
`${scheme}:<path> (${info})` is displayed, with `${info}` being
either "host optional", "no host" or "empty host".
Testing the expected string is no longer done in a marionette test
(test_rules_table.py : TestRuleRow.test_origin_and_dest_properties)
but instead in an xpcshell test (test_ruleutils.js)
---
src/content/lib/utils/rules.jsm | 78 ++++++++++++++++++++++
src/content/settings/common.js | 41 ------------
src/content/settings/oldrules.js | 6 +-
src/content/settings/yourpolicy.js | 7 +-
src/content/ui/menu.js | 24 -------
.../rp_puppeteer/tests/test_rules_table.py | 6 --
.../rp_puppeteer/ui/settings/rules_table.py | 22 +++++-
tests/marionette/rp_ui_harness/test_data/rules.py | 21 ++----
tests/xpcshell/.jscsrc | 9 ++-
tests/xpcshell/test_ruleutils.js | 41 ++++++++++++
tests/xpcshell/xpcshell.ini | 1 +
11 files changed, 162 insertions(+), 94 deletions(-)
diff --git a/src/content/lib/utils/rules.jsm b/src/content/lib/utils/rules.jsm
new file mode 100644
index 0000000..9bd407c
--- /dev/null
+++ b/src/content/lib/utils/rules.jsm
@@ -0,0 +1,78 @@
+/*
+ * ***** BEGIN LICENSE BLOCK *****
+ *
+ * RequestPolicy - A Firefox extension for control over cross-site requests.
+ * Copyright (c) 2008-2012 Justin Samuel
+ * Copyright (c) 2014-2016 Martin Kimmerle
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation, either version 3 of the License, or (at your option) any later
+ * version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * ***** END LICENSE BLOCK *****
+ */
+
+/* exported RuleUtils */
+this.EXPORTED_SYMBOLS = ["RuleUtils"];
+
+//==============================================================================
+// RuleUtils
+//==============================================================================
+
+var RuleUtils = (function() {
+ let self = {};
+
+ /**
+ * Get a string representation of an endpoint (origin or dest) specification.
+ *
+ * Example cases can be found in the unit test corresponding
+ * to this function.
+ */
+ self.endpointSpecToDisplayString = function(aEndpointSpec) {
+ if (aEndpointSpec.port !== undefined &&
+ (aEndpointSpec.h === null || aEndpointSpec.h === "")) {
+ return "[invalid endpoint specification]";
+ }
+ let scheme = aEndpointSpec.s ? String(aEndpointSpec.s) : "*";
+ if (aEndpointSpec.port === undefined) {
+ // Special cases.
+ switch (aEndpointSpec.h) {
+ case undefined:
+ if (aEndpointSpec.s === undefined) {
+ return "";
+ }
+ return scheme + ":<path> (host optional)";
+
+ case null:
+ return scheme + ":<path> (no host)";
+
+ case "":
+ return scheme + "://<path> (empty host)";
+
+ default:
+ break;
+ }
+ }
+ var str = "";
+ if (scheme !== "*" || aEndpointSpec.port) {
+ str += scheme + "://";
+ }
+ str += aEndpointSpec.h || "*";
+ if (aEndpointSpec.port) {
+ str += ":" + aEndpointSpec.port;
+ }
+ // TODO: path
+ return str;
+ };
+
+ return self;
+}());
diff --git a/src/content/settings/common.js b/src/content/settings/common.js
index b633832..3d530c2 100644
--- a/src/content/settings/common.js
+++ b/src/content/settings/common.js
@@ -35,47 +35,6 @@ var {common, WinEnv, elManager, $id, $str} = (function() {
var common = {};
- /**
- * Get a string representation of an endpoint (origin or dest) specification.
- *
- * The following list shows a mapping of the possible endpoint specs
- * to the corresponding string representation. Each endpoint spec contains at
- * least one of the parts "scheme", "host" and "port". The list shows the
- * strings by example: scheme "http"; host "www.example.com"; port "80".
- *
- * - s__: `scheme "http"`
- * - _h_: `www.example.com`
- * - __p: `*://*:80`
- * - sh_: `http://www.example.com`
- * - s_p: `http://*:80`
- * - _hp: `*://www.example.com:80`
- * - shp: `http://www.example.com:80`
- *
- * TODO: remove code duplication with menu.js
- */
- common.ruleDataPartToDisplayString = function(ruleDataPart) {
- if (ruleDataPart.s && !ruleDataPart.h && !ruleDataPart.port) {
- // Special case: Only a scheme is specified.
- // The result string will be `scheme "..."`.
- // Background info: The string could be `http:*`, but this could however
- // be confused with `*://http:*`. The string `http://*`
- // wouldn't be correct for all cases, since there are
- // URIs _without_ a host.
- return "scheme \"" + ruleDataPart.s + "\"";
- }
- var str = "";
- if (ruleDataPart.s || ruleDataPart.port) {
- str += ruleDataPart.s || "*";
- str += "://";
- }
- str += ruleDataPart.h || "*";
- if (ruleDataPart.port) {
- str += ":" + ruleDataPart.port;
- }
- // TODO: path
- return str;
- };
-
common.localize = function(stringNames) {
stringNames.forEach(function(name) {
$("[data-string=\"" + name + "\"]").each(function() {
diff --git a/src/content/settings/oldrules.js b/src/content/settings/oldrules.js
index 7340c96..ed9fb2f 100644
--- a/src/content/settings/oldrules.js
+++ b/src/content/settings/oldrules.js
@@ -9,6 +9,7 @@
var {Prefs} = importModule("models/prefs");
var {PolicyManager} = importModule("lib/policy-manager");
var {OldRules} = importModule("lib/old-rules");
+ var {RuleUtils} = importModule("lib/utils/rules");
//============================================================================
@@ -47,8 +48,9 @@
for (var i = 0; i < rules.length; i++) {
var entry = rules[i];
- var origin = entry.o ? common.ruleDataPartToDisplayString(entry.o) : "";
- var dest = entry.d ? common.ruleDataPartToDisplayString(entry.d) : "";
+ var origin = entry.o ?
+ RuleUtils.endpointSpecToDisplayString(entry.o) : "";
+ var dest = entry.d ? RuleUtils.endpointSpecToDisplayString(entry.d) : "";
addRulesTableRow(table, "allow", origin, dest, entry);
}
}
diff --git a/src/content/settings/yourpolicy.js b/src/content/settings/yourpolicy.js
index 2ada85a..765cd64 100644
--- a/src/content/settings/yourpolicy.js
+++ b/src/content/settings/yourpolicy.js
@@ -8,6 +8,7 @@
"chrome://rpcontinued/content/lib/script-loader.jsm", {});
var {Prefs} = importModule("models/prefs");
var {PolicyManager} = importModule("lib/policy-manager");
+ var {RuleUtils} = importModule("lib/utils/rules");
//============================================================================
@@ -80,8 +81,10 @@
for (var entryType in entries) {
for (var i = 0; i < entries[entryType].length; i++) {
var entry = entries[entryType][i];
- var origin = entry.o ? common.ruleDataPartToDisplayString(entry.o) : "";
- var dest = entry.d ? common.ruleDataPartToDisplayString(entry.d) : "";
+ var origin = entry.o ?
+ RuleUtils.endpointSpecToDisplayString(entry.o) : "";
+ var dest = entry.d ?
+ RuleUtils.endpointSpecToDisplayString(entry.d) : "";
if (filter) {
if (origin.indexOf(filter) === -1 && dest.indexOf(filter) === -1) {
continue;
diff --git a/src/content/ui/menu.js b/src/content/ui/menu.js
index 9623e42..d1b8b3a 100644
--- a/src/content/ui/menu.js
+++ b/src/content/ui/menu.js
@@ -1070,30 +1070,6 @@ window.rpcontinued.menu = (function() {
return item;
};
- self._ruleDataPartToDisplayString = function(ruleDataPart) {
- var str = "";
- if (ruleDataPart.s) {
- str += ruleDataPart.s + "://";
- }
- str += ruleDataPart.h || "*";
- if (ruleDataPart.port) {
- str += ":" + ruleDataPart.port;
- }
- // TODO: path
- return str;
- };
-
- self._ruleDataToFormatVariables = function(rawRule) {
- var fmtVars = [];
- if (rawRule.o) {
- fmtVars.push(self._ruleDataPartToDisplayString(rawRule.o));
- }
- if (rawRule.d) {
- fmtVars.push(self._ruleDataPartToDisplayString(rawRule.d));
- }
- return fmtVars;
- };
-
self._addMenuItemRemoveAllowRule = function(list, rawRule,
subscriptionOverride) {
if (rawRule.o && rawRule.d) {
diff --git a/tests/marionette/rp_puppeteer/tests/test_rules_table.py b/tests/marionette/rp_puppeteer/tests/test_rules_table.py
index d1d34e0..0af3b02 100644
--- a/tests/marionette/rp_puppeteer/tests/test_rules_table.py
+++ b/tests/marionette/rp_puppeteer/tests/test_rules_table.py
@@ -150,12 +150,6 @@ class TestRuleRow(RulesTableTestCase):
def test(spec_id):
test_pre_path_spec(endpoint, self.data.pre_path_specs[spec_id])
- test("s")
- test("h")
- test("p")
- test("sh")
- test("sp")
- test("hp")
test("shp")
test_endpoint("origin")
diff --git a/tests/marionette/rp_puppeteer/ui/settings/rules_table.py b/tests/marionette/rp_puppeteer/ui/settings/rules_table.py
index 438cfed..535f755 100644
--- a/tests/marionette/rp_puppeteer/ui/settings/rules_table.py
+++ b/tests/marionette/rp_puppeteer/ui/settings/rules_table.py
@@ -60,10 +60,26 @@ class RuleRow(ElementBaseLib):
if string == "":
return None
- # Case: Only a scheme is specified.
- match = re.match('^scheme "([^"]+)"$', string)
+ # Case: No host, empty host or host optional
+ match = re.match(r"""^
+ ([^:]+): # scheme
+ (//)?<path>
+ \s
+ \(([^)]+)\)
+ $""", string, flags=re.X)
if match:
- return {"s": match.group(1)}
+ scheme = match.group(1)
+ info = match.group(3)
+ spec = {}
+ if scheme != "*":
+ spec["s"] = scheme
+ if info == "no host":
+ spec["h"] = None
+ elif info == "empty host":
+ spec["h"] = ""
+ elif info != "host optional":
+ raise SyntaxError
+ return spec
# Case: URI without host, but with path
match = re.match(r"""^
diff --git a/tests/marionette/rp_ui_harness/test_data/rules.py b/tests/marionette/rp_ui_harness/test_data/rules.py
index 323d15f..ba8c865 100644
--- a/tests/marionette/rp_ui_harness/test_data/rules.py
+++ b/tests/marionette/rp_ui_harness/test_data/rules.py
@@ -64,22 +64,15 @@ class ExemplaryRules(ExemplaryRules_Meta):
@lazyprop
def pre_path_specs(self):
- """A list of all possible pre-paths, including the expected string."""
+ """A list of all possible pre-paths."""
return {
- "s": {"spec": {"s": "s1"},
- # The string "s1:*" could be confused with "*://s1:*"
- "expected_string": 'scheme "s1"'},
- "h": {"spec": {"h": "h2"},
- "expected_string": "h2"},
- "p": {"spec": {"port": 3},
- "expected_string": "*://*:3"},
- "sh": {"spec": {"s": "s4", "h": "h4"},
- "expected_string": "s4://h4"},
- "sp": {"spec": {"s": "s5", "port": 5},
- "expected_string": "s5://*:5"},
- "hp": {"spec": {"h": "h6", "port": 6},
- "expected_string": "*://h6:6"},
+ "s": {"spec": {"s": "s1"}},
+ "h": {"spec": {"h": "h2"}},
+ "p": {"spec": {"port": 3}},
+ "sh": {"spec": {"s": "s4", "h": "h4"}},
+ "sp": {"spec": {"s": "s5", "port": 5}},
+ "hp": {"spec": {"h": "h6", "port": 6}},
"shp": {"spec": {"s": "s7", "h": "h7", "port": 7},
"expected_string": "s7://h7:7"}
}
diff --git a/tests/xpcshell/.jscsrc b/tests/xpcshell/.jscsrc
index 48ecdee..110ae95 100644
--- a/tests/xpcshell/.jscsrc
+++ b/tests/xpcshell/.jscsrc
@@ -5,6 +5,11 @@
"validateQuoteMarks": "\"",
"requireParenthesesAroundIIFE": true,
- "requireCamelCaseOrUpperCaseIdentifiers": null,
- "disallowMultipleLineBreaks": null
+ "disallowMultipleLineBreaks": null,
+ "disallowSpacesInsideObjectBrackets": null,
+ "maximumLineLength": {
+ "value": 100,
+ "allExcept": ["comments", "regex"]
+ },
+ "requireCamelCaseOrUpperCaseIdentifiers": null
}
diff --git a/tests/xpcshell/test_ruleutils.js b/tests/xpcshell/test_ruleutils.js
new file mode 100644
index 0000000..6821d14
--- /dev/null
+++ b/tests/xpcshell/test_ruleutils.js
@@ -0,0 +1,41 @@
+/* exported run_test */
+Components.utils.import("chrome://rpcontinued/content/lib/utils/rules.jsm");
+
+function run_test() {
+ run_next_test();
+}
+
+// endpointSpecToDisplayString
+add_test(function() {
+ function test(endpointSpec, expectedString) {
+ strictEqual(expectedString,
+ RuleUtils.endpointSpecToDisplayString(endpointSpec));
+ }
+
+ let invalid = "[invalid endpoint specification]";
+
+ // About the display-strings where the host is `undefined`, `null` or "":
+ // Do not use `http:*` as a display-string! It could be confused
+ // with `*://http:*`, with "http" being the hostname.
+ // The string `http://*` wouldn't be correct for all cases, since there are
+ // URIs _without_ a host.
+
+ test({ }, "");
+ test({ port: 80}, "*://*:80");
+ test({ h: "www.example.com" }, "www.example.com");
+ test({ h: "www.example.com", port: 80}, "*://www.example.com:80");
+ test({ h: null }, "*:<path> (no host)");
+ test({ h: null, port: 80}, invalid);
+ test({ h: "" }, "*://<path> (empty host)");
+ test({ h: "", port: 80}, invalid);
+ test({s: "http" }, "http:<path> (host optional)");
+ test({s: "http", port: 80}, "http://*:80");
+ test({s: "http", h: "www.example.com" }, "http://www.example.com");
+ test({s: "http", h: "www.example.com", port: 80}, "http://www.example.com:80");
+ test({s: "http", h: null }, "http:<path> (no host)");
+ test({s: "http", h: null, port: 80}, invalid);
+ test({s: "http", h: "" }, "http://<path> (empty host)");
+ test({s: "http", h: "", port: 80}, invalid);
+
+ run_next_test();
+});
diff --git a/tests/xpcshell/xpcshell.ini b/tests/xpcshell/xpcshell.ini
index f65d03d..e040452 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_ruleutils.js]
;[test_subscription.js]
[test_wrap_function.js]
[test_xulutils_keyboard_shortcuts.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