[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