allowedSites was checked as a container while its entries were dereferenced unchecked. A stored {"0x...": "notalist"} passed the state gate and rendered a completely healthy popup, then threw "base.map is not a function" inside saveState()'s per-hostname merge, so every save from that moment on failed and the user went on operating a wallet that was persisting nothing. Measured against the previous head: the popup showed the main view with no page errors, and chrome.storage.local.set was never called at all. deniedSites has the identical shape; fraudContracts the same class with a milder consequence, throwing "(state.fraudContracts || []).map is not a function" on the send screen; and the sweep for the class turned up selectedToken, which is truthiness-gated on restore and then dereferenced as text, blanking the popup outright with "tokenId.toLowerCase is not a function".
All four now get the floor issue 311 settled -- the container AND its entries, with a malformed entry dropped -- through textList() and siteMap() beside the existing tokenRefs() in persistedState.js, rather than a third mechanism. Site-map keys are written with defineProperty for the same reason networkEndpoints' keys are: a stored own "__proto__" key would otherwise be handed to the prototype setter. The background's allowed.includes(hostname) gate is covered by the same floor, where a stored string would have answered a substring match rather than merely throwing.
A save that fails is no longer swallowed. onSaveFailure() in state.js reports every failed save, awaited or not -- the save queue has to attach a rejection handler to keep advancing, which is what made a failure disappear entirely -- and the popup raises a persistent "NOT SAVED" banner naming the reason. The popup's background refresh loop no longer turns a save failure into an unhandled rejection instead of a report. Both halves are needed: the floor only covers the causes it knows about, and storage can still fail for a quota or a revoked permission.
The field-by-field categorisation in the header of stateSchema.js, and its mirror in README.md, were re-verified against the code and moved with the change; the fields left on a loose floor now carry the reason each one is still safe. The popup boot harness moved to tests/support/popupBoot.js so the new tests drive the real entry point rather than duplicating it.
188 lines
6.8 KiB
JavaScript
188 lines
6.8 KiB
JavaScript
// Every element id the popup views look up must exist in the markup they
|
|
// look it up in.
|
|
//
|
|
// The failure this catches: `$("settings-hide-dsut")` is valid JavaScript
|
|
// referring to a defined function, so neither jest (node environment, no
|
|
// DOM) nor a linter has anything to object to. At runtime `$()` returns
|
|
// null and the next property access throws, which in `init()` aborts the
|
|
// rest of that view's wiring and takes the whole screen down. Settings is
|
|
// the densest concentration of these lookups in the codebase.
|
|
//
|
|
// This is the cheap general half of the guard: it runs in `make check`
|
|
// with no browser and covers every id in every view, not the ones some
|
|
// test happens to click. The expensive specific half is the Settings
|
|
// section of the end-to-end suite (tests/e2e/run.js), which proves the
|
|
// screen actually comes up and its controls work.
|
|
//
|
|
// Scope and limits, stated rather than implied:
|
|
// - Only literal string arguments are resolvable statically. A call
|
|
// like `$(containerId)` is invisible here; those are covered by the
|
|
// e2e run instead.
|
|
// - `document.getElementById()` is checked too, minus the ids listed in
|
|
// RUNTIME_CREATED_IDS, which name nodes the code creates itself and
|
|
// which are legitimately absent from the static markup.
|
|
|
|
"use strict";
|
|
|
|
const fs = require("fs");
|
|
const path = require("path");
|
|
|
|
const POPUP_DIR = path.join(__dirname, "..", "src", "popup");
|
|
const POPUP_HTML_PATH = path.join(POPUP_DIR, "index.html");
|
|
|
|
// Nodes built at runtime rather than authored in index.html. Each one must
|
|
// be created unconditionally by the code before it is ever looked up.
|
|
const RUNTIME_CREATED_IDS = new Set([
|
|
// Created by updateDebugBanner() in src/popup/views/helpers.js.
|
|
"debug-banner",
|
|
// Created by showSaveFailureBanner() in the same file, on the first save
|
|
// that fails. Absent from the markup on purpose: a popup where nothing has
|
|
// failed must not have to carry an empty banner
|
|
// (https://git.eeqj.de/sneak/AutistMask/issues/362).
|
|
"save-failure-banner",
|
|
]);
|
|
|
|
// Every id lookup the popup performs with a literal argument, as
|
|
// {id, file, line, source} records.
|
|
//
|
|
// showView("x") is included because it resolves to the element id
|
|
// "view-x": a view name with no matching section is the same defect one
|
|
// indirection further out.
|
|
const PATTERNS = [
|
|
{ re: /\$\(\s*"([^"\n]+)"\s*\)/g, id: (m) => m[1], source: "$()" },
|
|
{
|
|
re: /document\.getElementById\(\s*"([^"\n]+)"\s*\)/g,
|
|
id: (m) => m[1],
|
|
source: "getElementById()",
|
|
},
|
|
{
|
|
re: /\b(?:showError|hideError)\(\s*"([^"\n]+)"/g,
|
|
id: (m) => m[1],
|
|
source: "showError()/hideError()",
|
|
},
|
|
{
|
|
re: /\bshowView\(\s*"([^"\n]+)"\s*\)/g,
|
|
id: (m) => "view-" + m[1],
|
|
source: "showView()",
|
|
},
|
|
];
|
|
|
|
function jsFilesUnder(dir) {
|
|
const out = [];
|
|
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
|
const full = path.join(dir, entry.name);
|
|
if (entry.isDirectory()) {
|
|
out.push(...jsFilesUnder(full));
|
|
} else if (entry.name.endsWith(".js")) {
|
|
out.push(full);
|
|
}
|
|
}
|
|
return out.sort();
|
|
}
|
|
|
|
function lineOf(text, index) {
|
|
return text.slice(0, index).split("\n").length;
|
|
}
|
|
|
|
function collectReferences() {
|
|
const refs = [];
|
|
for (const file of jsFilesUnder(POPUP_DIR)) {
|
|
const text = fs.readFileSync(file, "utf8");
|
|
const rel = path.relative(path.join(__dirname, ".."), file);
|
|
for (const { re, id, source } of PATTERNS) {
|
|
re.lastIndex = 0;
|
|
let m;
|
|
while ((m = re.exec(text)) !== null) {
|
|
refs.push({
|
|
id: id(m),
|
|
file: rel,
|
|
line: lineOf(text, m.index),
|
|
source,
|
|
});
|
|
}
|
|
}
|
|
}
|
|
return refs;
|
|
}
|
|
|
|
function collectHtmlIds(html) {
|
|
const ids = [];
|
|
const re = /\bid="([^"]+)"/g;
|
|
let m;
|
|
while ((m = re.exec(html)) !== null) ids.push(m[1]);
|
|
return ids;
|
|
}
|
|
|
|
const HTML = fs.readFileSync(POPUP_HTML_PATH, "utf8");
|
|
const HTML_IDS = collectHtmlIds(HTML);
|
|
const HTML_ID_SET = new Set(HTML_IDS);
|
|
const REFERENCES = collectReferences();
|
|
|
|
describe("every element id the popup looks up exists in its markup", () => {
|
|
// A guard that found nothing to check would pass forever. If a
|
|
// refactor renames the directory, changes the helper, or moves the
|
|
// markup, this fails instead of quietly covering zero call sites.
|
|
// The floors are far below the counts measured when this was written
|
|
// (434 lookups across 20 of the 24 files under src/popup/, against 274
|
|
// ids in the markup), so ordinary churn does not trip them.
|
|
test("the scan actually found the code and the markup", () => {
|
|
const files = new Set(REFERENCES.map((r) => r.file));
|
|
expect(files.size).toBeGreaterThanOrEqual(15);
|
|
expect(REFERENCES.length).toBeGreaterThanOrEqual(300);
|
|
expect(HTML_IDS.length).toBeGreaterThanOrEqual(200);
|
|
|
|
// The densest screen, named explicitly: a scan that stopped
|
|
// covering src/popup/views/settings.js is the exact regression
|
|
// this file was written for.
|
|
expect(
|
|
files.has(path.join("src", "popup", "views", "settings.js")),
|
|
).toBe(true);
|
|
expect(
|
|
REFERENCES.some((r) => r.id === "settings-hide-spoofed-symbols"),
|
|
).toBe(true);
|
|
expect(REFERENCES.some((r) => r.id === "view-settings")).toBe(true);
|
|
});
|
|
|
|
test("no lookup names an id that src/popup/index.html does not define", () => {
|
|
const missing = REFERENCES.filter(
|
|
(r) => !HTML_ID_SET.has(r.id) && !RUNTIME_CREATED_IDS.has(r.id),
|
|
).map(
|
|
(r) =>
|
|
r.file +
|
|
":" +
|
|
r.line +
|
|
" " +
|
|
r.source +
|
|
' looks up id "' +
|
|
r.id +
|
|
'", which is not in src/popup/index.html',
|
|
);
|
|
|
|
expect(missing).toEqual([]);
|
|
});
|
|
|
|
test("every id excused as runtime-created is still looked up somewhere", () => {
|
|
// Otherwise the exception list becomes a place stale names
|
|
// accumulate, and the next real miss can be waved through by
|
|
// adding one more.
|
|
for (const id of RUNTIME_CREATED_IDS) {
|
|
expect(REFERENCES.some((r) => r.id === id)).toBe(true);
|
|
expect(HTML_ID_SET.has(id)).toBe(false);
|
|
}
|
|
});
|
|
|
|
test("index.html defines no id twice", () => {
|
|
// getElementById returns the first match, so a duplicate id means
|
|
// one of the two elements can never be reached by the code that
|
|
// thinks it owns it.
|
|
const seen = new Set();
|
|
const duplicated = [];
|
|
for (const id of HTML_IDS) {
|
|
if (seen.has(id)) duplicated.push(id);
|
|
seen.add(id);
|
|
}
|
|
|
|
expect(duplicated).toEqual([]);
|
|
});
|
|
});
|