bug 1606612: remote: move payload sanitization to Protocol r=remote-protocol-reviewers,whimboo

Moving the JSON payload sanitisation function to Protocol.jsm
means we can share it across modules.

The patch also adds new tests.

Differential Revision: https://phabricator.services.mozilla.com/D58505

--HG--
extra : moz-landing-system : lando

remote/JSONHandler.jsm

@@ -60,7 +60,7 @@ class JSONHandler {
       const body = this.routes[request.path]();
       const payload = JSON.stringify(
         body,
-        sanitise,
+        Protocol.sanitize,
         Log.verbose ? "\t" : undefined
       );
 
@@ -79,11 +79,3 @@ class JSONHandler {
     return ChromeUtils.generateQI([Ci.nsIHttpRequestHandler]);
   }
 }
-
-// Filters out null and empty strings
-function sanitise(key, value) {
-  if (value === null || (typeof value == "string" && value.length == 0)) {
-    return undefined;
-  }
-  return value;
-}

remote/Protocol.jsm

@@ -6,6 +6,19 @@
 
 var EXPORTED_SYMBOLS = ["Protocol"];
 
+/**
+ * Sanitizes outgoing JSON payloads by filtering out null and empty
+ * strings in values.
+ *
+ * Used as replacer for JSON.stringify.
+ */
+function sanitize(key, value) {
+  if (value === null || (typeof value == "string" && value.length == 0)) {
+    return undefined;
+  }
+  return value;
+}
+
 // TODO(ato): We send back a description of the protocol
 // when the user makes the initial HTTP request,
 // but the following is pure fiction.
@@ -17357,4 +17370,4 @@ const Description = {
     }
 };
 
-const Protocol = {Description};
+const Protocol = {Description, sanitize};

remote/test/unit/test_Protocol.js

@@ -0,0 +1,46 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const { Protocol } = ChromeUtils.import("chrome://remote/content/Protocol.jsm");
+
+add_test(function test_sanitize() {
+  const tests = [
+    [42, 42],
+    [{}, {}],
+    [[], []],
+    [undefined, undefined],
+    ["foo", "foo"],
+    ["", undefined],
+  ];
+
+  // we currently don't do anything based on the key
+  const bogusKey = "bogusKey";
+
+  for (const [input, output] of tests) {
+    deepEqual(Protocol.sanitize(bogusKey, input), output);
+  }
+
+  run_next_test();
+});
+
+add_test(function test_sanitize_JSON() {
+  const tests = [
+    [{}, {}],
+    [[], []],
+    [{ foo: "bar" }, { foo: "bar" }],
+    [{ foo: undefined }, {}],
+    [{ foo: { bar: undefined } }, { foo: {} }],
+    [{ foo: "" }, {}],
+    [{ foo: { bar: "" } }, { foo: {} }],
+  ];
+
+  for (const [input, output] of tests) {
+    const actual = JSON.stringify(input, Protocol.sanitize);
+    const expected = JSON.stringify(output);
+    deepEqual(actual, expected);
+  }
+
+  run_next_test();
+});

remote/test/unit/xpcshell.ini

@@ -8,5 +8,6 @@ skip-if = appname == "thunderbird"
 [test_Connection.js]
 [test_Domains.js]
 [test_Error.js]
+[test_Protocol.js]
 [test_Session.js]
 [test_StreamRegistry.js]