sneak/AutistMask
0
0
Fork 0
Code Issues 16 Pull Requests 7 Actions 162 Packages Projects Releases Wiki Activity

fix: Settings ignores an endpoint check once the same "Save" is tapped again (closes #698) #699

Open
clawbot wants to merge 1 commits from issue-698-latest-save-wins into next
pull from: issue-698-latest-save-wins
merge into: :next
Conversation 6 Commits 1 Files Changed 3
+256 -3
3 changed files with 256 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
README.md
+2
View File
@@ -2282,6 +2282,8 @@ view would leave a wallet one click from deletion.
- Either "Save" stores its URL only if the input still shows that URL when
the endpoint answers. Otherwise a flash message says that the URL was not
saved
- If the user taps the same "Save" again before the endpoint answers, the
earlier tap stores nothing and shows no flash message
- Etherscan:
- "Check recipients on Etherscan" checkbox, unticked until the user
ticks it
src/popup/views/settings.js
+30 -3
View File
@@ -50,6 +50,11 @@ let versionClickTimer = null;
// not the one in `state`.
let shownNetwork = null;
// Each endpoint "Save" counts its taps here, under the field that it stores. A
// tap stores its URL and shows a flash message only while it is the latest tap
// of its "Save".
const saveTaps = { rpcUrl: 0, blockscoutUrl: 0 };
// One row per site origin, however many addresses it appears under, each with
// an [x] that hands it to onRemove.
function renderSiteList(containerId, origins, onRemove) {
@@ -347,10 +352,16 @@ function showNewSettings() {
// shows `url`, nothing is saved. The flash message then says that the URL was
// changed during the check. The save compares the input and sets `url` in
// `state` in one step. So a redraw after the comparison shows `url`.
async function saveEndpoint(field, input, url, networkId) {
//
// `tap` is the number of the "Save" tap that checked `url`. If the user taps
// the same "Save" again before this save holds the lock, nothing is stored. If
// the user taps the same "Save" again before the write ends, no flash message
// is shown.
async function saveEndpoint(field, input, url, networkId, tap) {
let refused = false;
let inputChanged = false;
await saveState((stored) => {
if (saveTaps[field] !== tap) return false;
refused =
state.networkId !== networkId || stored.networkId !== networkId;
inputChanged = $(input).value.trim() !== url;
@@ -358,6 +369,7 @@ async function saveEndpoint(field, input, url, networkId) {
state[field] = url;
stored[field] = url;
});
if (saveTaps[field] !== tap) return;
if (refused) {
showFlash("The URL was not saved because the network changed.");
} else if (inputChanged) {
@@ -375,6 +387,8 @@ function init(_ctx) {
showPhrase.init();
$("btn-save-rpc").addEventListener("click", async () => {
saveTaps.rpcUrl++;
const tap = saveTaps.rpcUrl;
const url = $("settings-rpc").value.trim();
if (!url) {
showFlash("Please enter an RPC URL.");
@@ -395,6 +409,8 @@ function init(_ctx) {
}),
});
const json = await resp.json();
// The user has tapped this "Save" again since the check began.
if (tap !== saveTaps.rpcUrl) return;
if (json.error) {
log.errorf("RPC validation error:", json.error);
showFlash("Endpoint returned an error.");
@@ -408,6 +424,7 @@ function init(_ctx) {
// Cancelled by the popup closing, not failed: see pageClosed in
// src/popup/index.js.
if (ctx.pageClosed.aborted) return;
if (tap !== saveTaps.rpcUrl) return;
// Not the error's message: fetch puts the whole URL, password and
// key included, in the message of the error it throws for a URL
// with a user name and password or one it cannot parse.
@@ -415,10 +432,12 @@ function init(_ctx) {
showFlash("Could not reach endpoint.");
return;
}
await saveEndpoint("rpcUrl", "settings-rpc", url, net.id);
await saveEndpoint("rpcUrl", "settings-rpc", url, net.id, tap);
});
$("btn-save-blockscout").addEventListener("click", async () => {
saveTaps.blockscoutUrl++;
const tap = saveTaps.blockscoutUrl;
const url = $("settings-blockscout").value.trim();
if (!url) {
showFlash("Please enter a Blockscout API URL.");
@@ -428,18 +447,26 @@ function init(_ctx) {
showFlash("Testing endpoint...");
try {
const resp = await debugFetch(url + "/stats");
if (tap !== saveTaps.blockscoutUrl) return;
if (!resp.ok) {
showFlash("Endpoint returned HTTP " + resp.status + ".");
return;
}
} catch {
if (ctx.pageClosed.aborted) return;
if (tap !== saveTaps.blockscoutUrl) return;
// Not the error's message, as for the RPC check above.
log.errorf("Blockscout validation failed:", urlOrigin(url));
showFlash("Could not reach endpoint.");
return;
}
await saveEndpoint("blockscoutUrl", "settings-blockscout", url, net.id);
await saveEndpoint(
"blockscoutUrl",
"settings-blockscout",
url,
net.id,
tap,
);
});
// Each change is made inside the save, holding the lock. So
tests/settingsEndpointSave.test.js
+224
View File
@@ -167,3 +167,227 @@ test.each([
);
},
);
// This answer passes both endpoint checks on Ethereum Mainnet.
const PASSING_ANSWER = {
ok: true,
json: async () => ({ result: NETWORKS.mainnet.chainId }),
};
// Both endpoint checks read this answer as an error. The RPC check reads the
// answer's `error`. The Blockscout check reads the answer's `ok`.
const ERROR_ANSWER = {
ok: false,
status: 500,
json: async () => ({ error: { code: -32603, message: "Internal error" } }),
};
// The user types a URL in Settings and taps "Save". Before the endpoint answers,
// the user types a second URL and taps "Save" again. The second check passes
// first. The second URL is stored, and the flash message reads "Saved.". The
// first check then ends in one of three ways:
//
// - It passes.
// - It fails, because fetch cannot reach the endpoint.
// - It gets an answer that holds an error.
//
// None of the three changes the stored URL or the flash message
// (https://git.eeqj.de/sneak/AutistMask/issues/698).
test.each([
{
button: "btn-save-rpc",
input: "settings-rpc",
field: "rpcUrl",
first: "passes",
},
{
button: "btn-save-rpc",
input: "settings-rpc",
field: "rpcUrl",
first: "fails",
},
{
button: "btn-save-rpc",
input: "settings-rpc",
field: "rpcUrl",
first: "answers with an error",
},
{
button: "btn-save-blockscout",
input: "settings-blockscout",
field: "blockscoutUrl",
first: "passes",
},
{
button: "btn-save-blockscout",
input: "settings-blockscout",
field: "blockscoutUrl",
first: "fails",
},
{
button: "btn-save-blockscout",
input: "settings-blockscout",
field: "blockscoutUrl",
first: "answers with an error",
},
])(
"$button keeps the second tap's result when the first check $first last",
async ({ button, input, field, first }) => {
const page = await bootPopup(unversionedValidProfile());
await page.click("btn-settings");
const checks = [];
globalThis.fetch.mockImplementation(
() =>
new Promise((resolve, reject) => {
checks.push({ resolve, reject });
}),
);
const secondUrl = "https://second.example.invalid";
page.node(input).value = "https://first.example.invalid";
const firstTap = page.node(button).listeners.click.map((fn) => fn());
await page.settle();
page.node(input).value = secondUrl;
const secondTap = page.node(button).listeners.click.map((fn) => fn());
await page.settle();
checks[1].resolve(PASSING_ANSWER);
await Promise.all(secondTap);
await page.settle();
expect(page.text("flash-msg")).toBe("Saved.");
if (first === "passes") {
checks[0].resolve(PASSING_ANSWER);
} else if (first === "answers with an error") {
checks[0].resolve(ERROR_ANSWER);
} else {
// fetch rejects when it cannot reach the endpoint.
checks[0].reject(new TypeError("Failed to fetch"));
}
await Promise.all(firstTap);
await page.settle();
expect(checks).toHaveLength(2);
expect(page.storage.read("autistmask")[field]).toBe(secondUrl);
expect(page.text("flash-msg")).toBe("Saved.");
},
);
// The user types a URL in Settings and taps "Save". Another write holds the
// lock. So the save waits for the lock once the check passes. The user then
// taps "Save" again on the same URL. When the lock is free, the first save
// stores nothing and shows no flash message. The second check then passes, and
// the second save stores the URL.
test.each([
{ button: "btn-save-rpc", input: "settings-rpc", field: "rpcUrl" },
{
button: "btn-save-blockscout",
input: "settings-blockscout",
field: "blockscoutUrl",
},
])(
"$button stores nothing for an earlier tap still waiting for the lock",
async ({ button, input, field }) => {
const page = await bootPopup(unversionedValidProfile());
await page.click("btn-settings");
const storedUrl = page.value(input);
const checks = [];
globalThis.fetch.mockImplementation(
() =>
new Promise((resolve) => {
checks.push(resolve);
}),
);
const typedUrl = "https://typed.example.invalid";
page.node(input).value = typedUrl;
const firstTap = page.node(button).listeners.click.map((fn) => fn());
await page.settle();
let releaseLock;
const lockHeld = navigator.locks.request(
"autistmask",
() =>
new Promise((resolve) => {
releaseLock = resolve;
}),
);
checks[0](PASSING_ANSWER);
await page.settle();
const secondTap = page.node(button).listeners.click.map((fn) => fn());
await page.settle();
releaseLock();
await lockHeld;
await Promise.all(firstTap);
await page.settle();
expect(page.storage.read("autistmask")[field]).toBe(storedUrl);
expect(page.text("flash-msg")).toBe("Testing endpoint...");
checks[1](PASSING_ANSWER);
await Promise.all(secondTap);
await page.settle();
expect(checks).toHaveLength(2);
expect(page.storage.read("autistmask")[field]).toBe(typedUrl);
expect(page.text("flash-msg")).toBe("Saved.");
},
);
// The user taps "Save", and the check passes. The save then begins its storage
// write, and this test keeps the write waiting. The user taps "Save" again, and
// the second check cannot reach the endpoint. When the write ends, the flash
// message still reads "Could not reach endpoint.". The write stores the URL
// anyway, because the write began before the second tap.
test.each([
{ button: "btn-save-rpc", input: "settings-rpc", field: "rpcUrl" },
{
button: "btn-save-blockscout",
input: "settings-blockscout",
field: "blockscoutUrl",
},
])(
"$button keeps the second tap's message when tapped again during the write",
async ({ button, input, field }) => {
const page = await bootPopup(unversionedValidProfile());
await page.click("btn-settings");
const checks = [];
globalThis.fetch.mockImplementation(
() =>
new Promise((resolve, reject) => {
checks.push({ resolve, reject });
}),
);
const typedUrl = "https://typed.example.invalid";
page.node(input).value = typedUrl;
const firstTap = page.node(button).listeners.click.map((fn) => fn());
await page.settle();
const { set } = page.storage.local;
const write = set.getMockImplementation();
let writeBegan = false;
let releaseWrite;
const writeHeld = new Promise((resolve) => {
releaseWrite = resolve;
});
set.mockImplementationOnce(async (items) => {
writeBegan = true;
await writeHeld;
await write(items);
});
checks[0].resolve(PASSING_ANSWER);
await page.settle();
expect(writeBegan).toBe(true);
const secondTap = page.node(button).listeners.click.map((fn) => fn());
await page.settle();
// fetch rejects when it cannot reach the endpoint.
checks[1].reject(new TypeError("Failed to fetch"));
await Promise.all(secondTap);
await page.settle();
expect(page.text("flash-msg")).toBe("Could not reach endpoint.");
releaseWrite();
await Promise.all(firstTap);
await page.settle();
expect(checks).toHaveLength(2);
expect(page.storage.read("autistmask")[field]).toBe(typedUrl);
expect(page.text("flash-msg")).toBe("Could not reach endpoint.");
},
);