From a9e99bef15d8b2369d07f466ee11592aa10b4c95 Mon Sep 17 00:00:00 2001 From: Sergey Galich Date: Thu, 21 Jul 2022 21:00:11 +0000 Subject: [PATCH] Bug 1642570 - capture only user input for Form History r=dimi We can skip checks for `input.value != input.defaultValue` by using `lastInteractiveValue` which tells us what user typed in the field. Differential Revision: https://phabricator.services.mozilla.com/D150990 --- .../test/mochitest/pwmgr_common.js | 4 +- .../components/satchel/FormHistoryChild.jsm | 13 +- toolkit/components/satchel/test/mochitest.ini | 3 - .../components/satchel/test/satchel_common.js | 53 +- .../test/subtst_form_submission_1.html | 7 +- .../satchel/test/subtst_privbrowsing.html | 4 +- .../satchel/test/test_bug_511615.html | 4 +- .../satchel/test/test_bug_787624.html | 2 +- .../test/test_datalist_with_caching.html | 2 +- .../satchel/test/test_form_autocomplete.html | 42 +- .../test_form_autocomplete_with_list.html | 6 +- .../satchel/test/test_form_submission.html | 687 ++++++++++-------- .../test/test_form_submission_cap.html | 2 +- .../test/test_form_submission_cap2.html | 2 +- .../test/test_password_autocomplete.html | 8 +- .../satchel/test/test_popup_direction.html | 2 +- .../satchel/test/test_popup_enter_event.html | 2 +- .../test/test_submit_on_keydown_enter.html | 2 +- 18 files changed, 478 insertions(+), 367 deletions(-) diff --git a/toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js b/toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js index 33591f8f0103..bec5af27406f 100644 --- a/toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js +++ b/toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js @@ -51,9 +51,9 @@ function $_(formNum, name) { return null; } - var element = form.children.namedItem(name); + var element = form.querySelector(`:is([name="${name}"], [id="${name}"])`); if (!element) { - ok(false, "$_ couldn't find requested element " + name); + ok(false, `$_ couldn't find requested element "${name}"`); return null; } diff --git a/toolkit/components/satchel/FormHistoryChild.jsm b/toolkit/components/satchel/FormHistoryChild.jsm index ece589497fad..2ce470cf59f1 100644 --- a/toolkit/components/satchel/FormHistoryChild.jsm +++ b/toolkit/components/satchel/FormHistoryChild.jsm @@ -95,10 +95,17 @@ class FormHistoryChild extends JSWindowActorChild { continue; } - let value = input.value.trim(); + const value = input.lastInteractiveValue?.trim(); - // Don't save empty or unchanged values. - if (!value || value == input.defaultValue.trim()) { + // Only save user entered values even if they match the default value. + // Any script input is ignored. + // See Bug 1642570 for details. + if (!value) { + continue; + } + + // Save only when user input was last. + if (value != input.value.trim()) { continue; } diff --git a/toolkit/components/satchel/test/mochitest.ini b/toolkit/components/satchel/test/mochitest.ini index cf345f4dd690..1cfc59bc0375 100644 --- a/toolkit/components/satchel/test/mochitest.ini +++ b/toolkit/components/satchel/test/mochitest.ini @@ -18,9 +18,6 @@ skip-if = (verify && debug && (os == 'win')) || (os == 'mac') # Bug 1514249 [test_form_autocomplete_validation_at_input_event.html] [test_form_autocomplete_with_list.html] [test_form_submission.html] -skip-if = - os == "mac" && bits == 64 && debug # Bug 1481802 - os == "linux" && bits == 64 # Bug 1481802 [test_form_submission_cap.html] [test_form_submission_cap2.html] [test_input_valid_state_with_autocomplete.html] diff --git a/toolkit/components/satchel/test/satchel_common.js b/toolkit/components/satchel/test/satchel_common.js index 548209e0f0e8..38a216d67c0f 100644 --- a/toolkit/components/satchel/test/satchel_common.js +++ b/toolkit/components/satchel/test/satchel_common.js @@ -25,16 +25,16 @@ const TelemetryFilterPropsAC = Object.freeze({ /* * Returns the element with the specified |name| attribute. */ -function $_(formNum, name) { +function getFormElementByName(formNum, name) { let form = document.getElementById("form" + formNum); if (!form) { - ok(false, "$_ couldn't find requested form " + formNum); + ok(false, "getFormElementByName couldn't find requested form " + formNum); return null; } let element = form.elements.namedItem(name); if (!element) { - ok(false, "$_ couldn't find requested element " + name); + ok(false, "getFormElementByName couldn't find requested element " + name); return null; } @@ -43,11 +43,11 @@ function $_(formNum, name) { // the element. if (element.hasAttribute("name") && element.getAttribute("name") != name) { - ok(false, "$_ got confused."); + ok(false, "getFormElementByName got confused."); return null; } - return element; + return SpecialPowers.wrap(element); } function registerPopupShownListener(listener) { @@ -79,7 +79,7 @@ function checkArrayValues(actualValues, expectedValues, msg) { } } -var checkObserver = { +var gCheckObserver = { verifyStack: [], callback: null, @@ -122,17 +122,41 @@ var checkObserver = { countEntries(expected.name, expected.value, function(num) { ok(num > 0, expected.message); - if (!checkObserver.verifyStack.length) { - let callback = checkObserver.callback; - checkObserver.callback = null; + if (!gCheckObserver.verifyStack.length) { + let callback = gCheckObserver.callback; + gCheckObserver.callback = null; callback(); } }); }, }; +class StorageEventsObserver { + promisesToResolve = []; + + constructor() { + gChromeScript.sendAsyncMessage("addObserver"); + gChromeScript.addMessageListener( + "satchel-storage-changed", + this.observe.bind(this) + ); + } + + cleanup() { + gChromeScript.sendAsyncMessage("removeObserver"); + } + + observe({ subject, topic, data }) { + this.promisesToResolve.shift()?.({ subject, topic, data }); + } + + promiseNextStorageEvent() { + return new Promise(resolve => this.promisesToResolve.push(resolve)); + } +} + function checkForSave(name, value, message) { - checkObserver.verifyStack.push({ name, value, message }); + gCheckObserver.verifyStack.push({ name, value, message }); } function getFormSubmitButton(formNum) { @@ -297,6 +321,12 @@ function checkACTelemetryEvent(actualEvent, input, augmentedExtra) { isDeeply(actualEvent[5], expectedExtra, "Check event extra object"); } +let gStorageEventsObserver; + +function promiseNextStorageEvent() { + return gStorageEventsObserver.promiseNextStorageEvent(); +} + function satchelCommonSetup() { let chromeURL = SimpleTest.getTestFileURL("parent_utils.js"); gChromeScript = SpecialPowers.loadChromeScript(chromeURL); @@ -307,7 +337,10 @@ function satchelCommonSetup() { } }); + gStorageEventsObserver = new StorageEventsObserver(); + SimpleTest.registerCleanupFunction(() => { + gStorageEventsObserver.cleanup(); gChromeScript.sendAsyncMessage("cleanup"); return new Promise(resolve => { gChromeScript.addMessageListener("cleanup-done", function done() { diff --git a/toolkit/components/satchel/test/subtst_form_submission_1.html b/toolkit/components/satchel/test/subtst_form_submission_1.html index c06c5e0ce38a..c1b902d6bce8 100644 --- a/toolkit/components/satchel/test/subtst_form_submission_1.html +++ b/toolkit/components/satchel/test/subtst_form_submission_1.html @@ -6,15 +6,10 @@ -
+
- - diff --git a/toolkit/components/satchel/test/subtst_privbrowsing.html b/toolkit/components/satchel/test/subtst_privbrowsing.html index f0f4f38ad475..a61da1a714f1 100644 --- a/toolkit/components/satchel/test/subtst_privbrowsing.html +++ b/toolkit/components/satchel/test/subtst_privbrowsing.html @@ -7,8 +7,8 @@ function submitForm() { if (!location.search.includes("field")) { let form = document.getElementById("form"); - let field = document.getElementById("field"); - field.value = "value"; + let field = SpecialPowers.wrap(document.getElementById("field")); + field.setUserInput("value"); form.submit(); } } diff --git a/toolkit/components/satchel/test/test_bug_511615.html b/toolkit/components/satchel/test/test_bug_511615.html index 41bb4aca2023..584ddc35a8d8 100644 --- a/toolkit/components/satchel/test/test_bug_511615.html +++ b/toolkit/components/satchel/test/test_bug_511615.html @@ -76,7 +76,9 @@ function checkSelectedIndexAfterResponseTime(expectedIndex) { }); } -let input = $_(1, "field1"); +// getFormElementByName() returns wrapped input, +// but in this test we want to test untrusted events, so we must unwrap +const input = SpecialPowers.unwrap(getFormElementByName(1, "field1")); function doKeyUnprivileged(key) { let keyName = "DOM_VK_" + key.toUpperCase(); diff --git a/toolkit/components/satchel/test/test_bug_787624.html b/toolkit/components/satchel/test/test_bug_787624.html index 6599e0c849a4..63e8f688f4cf 100644 --- a/toolkit/components/satchel/test/test_bug_787624.html +++ b/toolkit/components/satchel/test/test_bug_787624.html @@ -60,7 +60,7 @@ function waitForNextPopup() { } add_task(async function test_popup_not_move_input() { - let input = $_(1, "field1"); + let input = getFormElementByName(1, "field1"); let rect = input.getBoundingClientRect(); await new Promise(resolve => updateFormHistory([ diff --git a/toolkit/components/satchel/test/test_datalist_with_caching.html b/toolkit/components/satchel/test/test_datalist_with_caching.html index fe090ec81a32..b4383985f817 100644 --- a/toolkit/components/satchel/test/test_datalist_with_caching.html +++ b/toolkit/components/satchel/test/test_datalist_with_caching.html @@ -29,7 +29,7 @@ Form History test: form field autocomplete
 
     
   
 
-  
-  
+
- -
+
- -
+
- -
+
- -
- + +
- -
- + +
- -
+
- -
+
- -
+
+
+ + + +
+ +
+ + +
+ - -
+
- -
+
- -
+
- -
+
- -
+
- -
+
- -
+
- -
+
- -
+
@@ -255,9 +261,7 @@
diff --git a/toolkit/components/satchel/test/test_form_submission_cap.html b/toolkit/components/satchel/test/test_form_submission_cap.html index bd34e7b03f0a..a1f60899ca7e 100644 --- a/toolkit/components/satchel/test/test_form_submission_cap.html +++ b/toolkit/components/satchel/test/test_form_submission_cap.html @@ -48,7 +48,7 @@ function startTest() { // value attribute, but we don't save default form values (and we want to // ensure unsaved values are because of autocomplete=off or whatever). for (let i = 1; i <= numInputFields; i++) { - $_(1, "test" + i).value = i; + getFormElementByName(1, "test" + i).value = i; } // submit the first form. diff --git a/toolkit/components/satchel/test/test_form_submission_cap2.html b/toolkit/components/satchel/test/test_form_submission_cap2.html index 1e31e64ca7e1..ab6fdf75a59f 100644 --- a/toolkit/components/satchel/test/test_form_submission_cap2.html +++ b/toolkit/components/satchel/test/test_form_submission_cap2.html @@ -137,7 +137,7 @@ function startTest() { // Fill in values for the various fields. We could just set the 's // value attribute, but we don't save default form values (and we want to // ensure unsaved values are because of autocomplete=off or whatever). - $_(1, "test" + numInputFields).value = numInputFields + " changed"; + getFormElementByName(1, "test" + numInputFields).value = numInputFields + " changed"; // submit the first form. let button = getFormSubmitButton(1); diff --git a/toolkit/components/satchel/test/test_password_autocomplete.html b/toolkit/components/satchel/test/test_password_autocomplete.html index c6830071938d..603196126068 100644 --- a/toolkit/components/satchel/test/test_password_autocomplete.html +++ b/toolkit/components/satchel/test/test_password_autocomplete.html @@ -79,8 +79,8 @@ add_task(async function test_initialize() { is(window.location.protocol, "https:", "This test must run on HTTPS"); // Now that rememberSignons is false, create the password fields. - $_(1, "field1").type = "password"; - $_(2, "field1").type = "password"; + getFormElementByName(1, "field1").type = "password"; + getFormElementByName(2, "field1").type = "password"; await new Promise(resolve => updateFormHistory([ { op: "remove" }, @@ -97,7 +97,7 @@ add_task(async function test_initialize() { }); add_task(async function test_secure_noFormHistoryOrWarning() { - let input = $_(1, "field1"); + let input = getFormElementByName(1, "field1"); // The autocomplete popup should not open under any circumstances on // type=password with password manager disabled. @@ -122,7 +122,7 @@ add_task(async function test_secure_noFormHistoryOrWarning() { add_task(async function test_insecure_focusWarning() { // Form 2 has an insecure action so should show the warning even if password manager is disabled. - let input = $_(2, "field1"); + let input = getFormElementByName(2, "field1"); let shownPromise = waitForNextPopup(); input.focus(); await shownPromise; diff --git a/toolkit/components/satchel/test/test_popup_direction.html b/toolkit/components/satchel/test/test_popup_direction.html index 412b75df52e2..c07d20a3bd72 100644 --- a/toolkit/components/satchel/test/test_popup_direction.html +++ b/toolkit/components/satchel/test/test_popup_direction.html @@ -30,7 +30,7 @@ function waitForNextPopup() { } add_task(async function test_popup_direction() { - let input = $_(1, "field1"); + let input = getFormElementByName(1, "field1"); await new Promise(resolve => updateFormHistory([ { op: "remove" }, diff --git a/toolkit/components/satchel/test/test_popup_enter_event.html b/toolkit/components/satchel/test/test_popup_enter_event.html index 5125f7399797..7c2cda7c4fe7 100644 --- a/toolkit/components/satchel/test/test_popup_enter_event.html +++ b/toolkit/components/satchel/test/test_popup_enter_event.html @@ -22,7 +22,7 @@ Form History test: Test for events while the form history popup is open