Skip to content

Commit

Permalink
Fixed duplicate common settings bkmk problem
Browse files Browse the repository at this point in the history 
I was properly handling duplicate regular bookmarks but not duplicate common settings ones.  The fix was a bit tricky because parsing a common settings bookmark is more complicated than parsing a regular one, which is just the settings.  The common settings one is an object containing a defaultSettings object.  It took a while to find the right place to deal with that because of a poorly named function.
  • Loading branch information
@alanhkarp
alanhkarp committed Jan 4, 2025
1 parent e1f9c10 commit f66bbaa
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 28 deletions.
2 changes: 1 addition & 1 deletion manifest.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"name": "Site Password",
"short_name": "Site Password for Chrome",
"description": "Don't remember your passwords. Don't store them. Calculate them!",
"version": "3.0.11",
"version": "3.0.12",
"background": {
"service_worker": "src/bg.js",
"type": "module"
Expand Down
2 changes: 1 addition & 1 deletion manifestChrome.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"name": "Site Password",
"short_name": "Site Password for Chrome",
"description": "Don't remember your passwords. Don't store them. Calculate them!",
"version": "3.0.11",
"version": "3.0.12",
"background": {
"service_worker": "src/bg.js",
"type": "module"
Expand Down
48 changes: 27 additions & 21 deletions src/bg.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,8 @@ async function persistMetadata(sendResponse) {
if (logging) console.log("bg created common settings bookmark", commonBkmk.title, commonSettings.pwlength);
}
} else {
let existing = parseSettings(commonSettings[0].url);
if (isLegacy(commonSettings[0].url) || !sameSettings(common, existing)) {
let existing = parseURL(commonSettings[0].url); // Common settings bookmark
if (isLegacy(commonSettings[0].url) || !identicalObjects(common, existing)) {
if (isSafari) {
bkmksSafari[commonSettingsTitle].url = url;
await chrome.storage.sync.set(bkmksSafari);
Expand All @@ -408,8 +408,8 @@ async function persistMetadata(sendResponse) {
let url = webpage + "?bkmk=ssp://" + stringifySettings(settings);
let found = domains.find((item) => item.title === domainnames[i]);
if (found) {
let foundSettings = parseSettings(found.url);
if (isLegacy(found.url) || !sameSettings(settings, foundSettings)) {
let foundSettings = parseURL(found.url);
if (isLegacy(found.url) || !identicalObjects(settings, foundSettings)) {
if (isSafari) {
// Handle Safari bookmarks
if (bkmksSafari[found.title] && bkmksSafari[found.title].url !== url) {
Expand Down Expand Up @@ -521,35 +521,31 @@ async function parseBkmk(rootFolderId, callback, sendResponse) {
if (chrome.runtime.lastError) console.log("bg remove legacy lastError", chrome.runtime.lastError);
}
}
if (seenTitles[title] !== undefined) { // Because 0 tests as false
if (seenTitles[title]) {
if (logging) console.log("bg duplicate bookmark", children[i]);
let seen = parseSettings(children[seenTitles[title]].url);
seen.specials = array2string(seen.specials); // For legacy bookmarks
let dupl = parseSettings(children[i].url);
dupl.specials = array2string(dupl.specials); // For legacy bookmarks
if (sameSettings(seen, dupl)) {
let dupl = parseURL(children[i].url);
if (identicalObjects(seenTitles[title], dupl)) {
if (isSafari) {
delete bkmksSafari[children[i]];
} else {
chrome.bookmarks.remove(children[i].id);
}
} else {
sendResponse({"duplicate": children[i].title});
continue;
}
} else {
seenTitles[title] = i;
seenTitles[title] = parseURL(children[i].url);
}
if (title === commonSettingsTitle) {
if (logging) console.log("bg common settings from bookmark", children[i]);
let common = parseSettings(children[i].url);
let common = parseURL(children[i].url);
if (logging) console.log("bg common settings from bookmark", common);
defaultSettings = common.defaultSettings || defaultSettings;
common.defaultSettings = defaultSettings;
newdb.common = common;
} else {
if (logging && i < 3) console.log("bg settings from bookmark", children[i]);
let settings = parseSettings(children[i].url);
let settings = parseURL(children[i].url);
if (logging) console.log("bg settings from bookmark", settings);
if (settings.sitename) {
newdb.domains[title] = normalize(settings.sitename);
Expand Down Expand Up @@ -657,25 +653,35 @@ function stringifySettings(settings) {
console.log("bad URI", settings);
}
}
function parseSettings(url) {
function parseURL(url) {
// Returns settings or common settings object
let str = sspUrl(url);
let settings;
try {
return JSON.parse(decodeURIComponent(str));
settings = JSON.parse(decodeURIComponent(str));
} catch (e) {
let settings = JSON.parse(str.replace(/%22/g, '"').replace(/%20/g, " "));
if (settings.specials) settings.specials = array2string(settings.specials);
return settings; // To handle legacy bookmarks
settings = JSON.parse(str.replace(/%22/g, '"').replace(/%20/g, " "));
}
if (settings.specials) { // For regular legacy bookmarks
settings.specials = array2string(settings.specials);
} else { // For common settings legacy bookmarks
settings.defaultSettings.specials = array2string(settings.defaultSettings.specials);
}
return settings; // To handle legacy bookmarks
}
function sameSettings(a, b) {
function identicalObjects(a, b) {
if (!a || !b) return false; // Assumes one or the other is set
if (Object.keys(a).length !== Object.keys(b).length) return false;
for (let key in a) {
// The domain name may change for domains that share setting
if (key === "domainname") continue;
if (key === "updateTime") continue;
if (key === "specials") { // For legacy bookmarks
a[key] = array2string(a[key]);
b[key] = array2string(b[key]);
}
if (typeof a[key] === "object") {
if (!sameSettings(a[key], b[key])) return false;
if (!identicalObjects(a[key], b[key])) return false;
} else {
if (a[key] !== b[key]) {
return false;
Expand Down
41 changes: 36 additions & 5 deletions src/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ if (logging) {
loggingForget = loggingPhishing = loggingProvide =
loggingReset = loggingTrigger = loggingWrapHandler = true;
}

export async function runTests() {
// #region Fields needed for tests
const $mainpanel = get("mainpanel");
Expand Down Expand Up @@ -366,13 +366,14 @@ export async function runTests() {
}
}
// Test proper handling of duplicate bookmarks
// For testing duplicate bookmarks
async function testDuplicateBkmks() {
await resetState();
// Create a duplicate bookmark
let title = "duplicate.bkmk.com";
let url = "https://sitepassword.info/?bkmk=ssp://{%22sitename%22:%22usps%22,%22username%22:%22fred%22,%22providesitepw%22:false,%22xor%22:[0,0,0,0,0,0,0,0,0,0,0,0],%22pwlength%22:12,%22domainname%22:%22reg.usps.com%22,%22pwdomainname%22:%22reg.usps.com%22,%22startwithletter%22:true,%22allowlower%22:true,%22allowupper%22:true,%22allownumber%22:true,%22allowspecial%22:false,%22minlower%22:1,%22minupper%22:1,%22minnumber%22:1,%22minspecial%22:1,%22specials%22:%22$/!=@?._-%22,%22characters%22:%220123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz%22}";
let url = "https://sitepassword.info/?bkmk=ssp://%7B%22sitename%22%3A%22usps%22%2C%22username%22%3A%22fred%22%2C%22providesitepw%22%3Afalse%2C%22xor%22%3A%5B0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%5D%2C%22domainname%22%3A%22reg.usps.com%22%2C%22pwdomainname%22%3A%22reg.usps.com%22%2C%22pwlength%22%3A%2212%22%2C%22startwithletter%22%3Atrue%2C%22allowlower%22%3Atrue%2C%22allowupper%22%3Atrue%2C%22allownumber%22%3Atrue%2C%22allowspecial%22%3Afalse%2C%22minlower%22%3A%221%22%2C%22minupper%22%3A%221%22%2C%22minnumber%22%3A%221%22%2C%22minspecial%22%3A%221%22%2C%22specials%22%3A%22%24%2F!%3D%40%3F._-%22%2C%22characters%22%3A%220123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz%22%7D";
await resetState();
let rootFolder = await getRootFolder();
await triggerEvent("mouseleave", $mainpanel, "mouseleaveResolver");
// Create a duplicate bookmark
let rootFolder = await getRootFolder();
await chrome.bookmarks.create({ "parentId": rootFolder[0].id, "title": title, "url": url });
await chrome.bookmarks.create({ "parentId": rootFolder[0].id, "title": title, "url": url });
await triggerEvent("mouseleave", $mainpanel, "mouseleaveResolver");
Expand All @@ -399,7 +400,37 @@ export async function runTests() {
console.warn("Failed: Different duplicate bookmark not handled");
failed += 1;
}
// Test duplicate common settings bookmark
await resetState(); // Don't leave duplicate bookmarks
await triggerEvent("mouseleave", $mainpanel, "mouseleaveResolver");
rootFolder = await getRootFolder();
await chrome.bookmarks.create({ "parentId": rootFolder[0].id, "title": title, "url": url });
url = "ssp://%7B%22clearsuperpw%22%3Afalse%2C%22hidesitepw%22%3Afalse%2C%22defaultSettings%22%3A%7B%22sitename%22%3A%22%22%2C%22username%22%3A%22%22%2C%22providesitepw%22%3Afalse%2C%22xor%22%3A%5B0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%2C0%5D%2C%22pwlength%22%3A12%2C%22domainname%22%3A%22%22%2C%22pwdomainname%22%3A%22%22%2C%22startwithletter%22%3Atrue%2C%22allowlower%22%3Atrue%2C%22allowupper%22%3Atrue%2C%22allownumber%22%3Atrue%2C%22allowspecial%22%3Afalse%2C%22minlower%22%3A1%2C%22minupper%22%3A1%2C%22minnumber%22%3A1%2C%22minspecial%22%3A1%2C%22specials%22%3A%22%24%2F!%3D%40%3F._-%22%7D%7D";
title = "CommonSettings";
// Create a duplicate common settings bookmark}
await chrome.bookmarks.create({ "parentId": rootFolder[0].id, "title": title, "url": url });
await triggerEvent("mouseleave", $mainpanel, "mouseleaveResolver");
children = await chrome.bookmarks.getChildren(rootFolder[0].id);
test = children.length === 2;
if (test) {
console.log("Passed: Identical duplicate common settings bookmark handled");
passed += 1;
} else {
console.warn("Failed: Identical duplicate common settings bookmark not handled");
failed += 1;
}
newUrl = url.replace("12", "15");
await chrome.bookmarks.create({ "parentId": rootFolder[0].id, "title": title, "url": newUrl });
await triggerEvent("mouseleave", $mainpanel, "mouseleaveResolver");
children = await chrome.bookmarks.getChildren(rootFolder[0].id);
test = children.length === 3; // because of the common settings bookmark
if (test) {
console.log("Passed: Different duplicate common settomgs bookmark handled");
passed += 1;
} else {
console.warn("Failed: Different duplicate common settings bookmark not handled");
failed += 1;
}
}
// Test save as default
async function testSaveAsDefault() {
Expand Down

0 comments on commit f66bbaa

Please sign in to comment.