Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-27 20:25:44 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1401401 - Bookmarks with an invalid url cannot be removed (part 2). r=Standard8

Browse Source 
Differential Revision: https://phabricator.services.mozilla.com/D34744

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Marco Bonardo 2019-06-16 12:06:25 +00:00
parent dcef68ed28
commit 30de429e05
5 changed files with 107 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
toolkit/components/places/Bookmarks.jsm
View File

@ -2652,7 +2652,7 @@ async function(db, folderGuids, options) {
JOIN moz_bookmarks p ON p.id = b.parent
LEFT JOIN moz_places h ON b.fk = h.id`, { folderGuid });
itemsRemoved = itemsRemoved.concat(rowsToItemsArray(rows));
itemsRemoved = itemsRemoved.concat(rowsToItemsArray(rows, true));
await db.executeCached(
`WITH RECURSIVE

17
toolkit/components/places/PlacesTransactions.jsm
View File

@ -1324,8 +1324,10 @@ PT.Remove.prototype = {
// we do need it for the possibility of undo().
removedItems.push(await PlacesUtils.promiseBookmarksTree(guid));
} catch (ex) {
throw new Error("Failed to get info for the specified item (guid: " +
guid + "): " + ex);
if (!ex.becauseInvalidURL) {
throw new Error(`Failed to get info for the guid: ${guid}: ${ex}`);
}
removedItems.push({guid});
}
}
@ -1334,16 +1336,19 @@ PT.Remove.prototype = {
// We have to pass just the guids as although remove() accepts full
// info items, promiseBookmarksTree returns dateAdded and lastModified
// as PRTime rather than date types.
await PlacesUtils.bookmarks.remove(removedItems.map(info => {
return { guid: info.guid};
}));
await PlacesUtils.bookmarks.remove(
removedItems.map(info => ({ guid: info.guid })));
}
};
await removeThem();
this.undo = async function() {
for (let info of removedItems) {
await createItemsFromBookmarksTree(info, true);
try {
await createItemsFromBookmarksTree(info, true);
} catch (ex) {
Cu.reportError(`Unable to undo removal of ${info.guid}`);
}
}
};
this.redo = removeThem;

11
toolkit/components/places/PlacesUtils.jsm
View File

@ -1631,7 +1631,13 @@ var PlacesUtils = {
case PlacesUtils.bookmarks.TYPE_BOOKMARK:
item.type = PlacesUtils.TYPE_X_MOZ_PLACE;
// If this throws due to an invalid url, the item will be skipped.
item.uri = NetUtil.newURI(aRow.getResultByName("url")).spec;
try {
item.uri = NetUtil.newURI(aRow.getResultByName("url")).spec;
} catch (ex) {
let error = new Error("Invalid bookmark URL");
error.becauseInvalidURL = true;
throw error;
}
// Keywords are cached, so this should be decently fast.
let entry = await PlacesUtils.keywords.fetch({ url: item.uri });
if (entry) {
@ -1736,7 +1742,8 @@ var PlacesUtils = {
enumerable: false,
configurable: false });
} catch (ex) {
throw new Error("Failed to fetch the data for the root item " + ex);
Cu.reportError("Failed to fetch the data for the root item");
throw ex;
}
} else {
try {

38
toolkit/components/places/tests/bookmarks/test_bookmarks_remove.js
View File

@ -332,12 +332,19 @@ add_task(async function test_nested_content_fails_when_not_allowed() {
});
add_task(async function test_remove_bookmark_with_invalid_url() {
let folder = await PlacesUtils.bookmarks.insert({
type: PlacesUtils.bookmarks.TYPE_FOLDER,
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "folder",
});
let guid = "invalid_____";
let folderedGuid = "invalid____2";
let url = "invalid-uri";
await PlacesUtils.withConnectionWrapper("test_bookmarks_remove", async db => {
await db.execute(`
INSERT INTO moz_places(url, url_hash, title, rev_host, guid)
VALUES ('invalid-uri', hash('invalid-uri'), 'Invalid URI', '.', GENERATE_GUID())
`);
VALUES (:url, hash(:url), 'Invalid URI', '.', GENERATE_GUID())
`, {url});
await db.execute(
`INSERT INTO moz_bookmarks (type, fk, parent, position, guid)
VALUES (:type,
@ -346,13 +353,30 @@ add_task(async function test_remove_bookmark_with_invalid_url() {
(SELECT MAX(position) + 1 FROM moz_bookmarks WHERE parent = (SELECT id FROM moz_bookmarks WHERE guid = :parentGuid)),
:guid)
`, {
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url: "invalid",
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
guid,
});
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url,
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
guid,
});
await db.execute(
`INSERT INTO moz_bookmarks (type, fk, parent, position, guid)
VALUES (:type,
(SELECT id FROM moz_places WHERE url = :url),
(SELECT id FROM moz_bookmarks WHERE guid = :parentGuid),
(SELECT MAX(position) + 1 FROM moz_bookmarks WHERE parent = (SELECT id FROM moz_bookmarks WHERE guid = :parentGuid)),
:guid)
`, {
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url,
parentGuid: folder.guid,
guid: folderedGuid,
});
});
await PlacesUtils.bookmarks.remove(guid);
Assert.strictEqual(await PlacesUtils.bookmarks.fetch(guid), null,
"Should not throw and not find the bookmark");
await PlacesUtils.bookmarks.remove(folder);
Assert.strictEqual(await PlacesUtils.bookmarks.fetch(folderedGuid), null,
"Should not throw and not find the bookmark");
});

55
toolkit/components/places/tests/unit/test_async_transactions.js
View File

@ -1706,3 +1706,58 @@ add_task(async function test_renameTag() {
ensureUndoState();
await PlacesUtils.bookmarks.eraseEverything();
});
add_task(async function test_remove_invalid_url() {
let folderGuid = await PT.NewFolder({ title: "Test Folder",
parentGuid: menuGuid }).transact();
let guid = "invalid_____";
let folderedGuid = "invalid____2";
let url = "invalid-uri";
await PlacesUtils.withConnectionWrapper("test_bookmarks_remove", async db => {
await db.execute(`
INSERT INTO moz_places(url, url_hash, title, rev_host, guid)
VALUES (:url, hash(:url), 'Invalid URI', '.', GENERATE_GUID())
`, {url});
await db.execute(
`INSERT INTO moz_bookmarks (type, fk, parent, position, guid)
VALUES (:type,
(SELECT id FROM moz_places WHERE url = :url),
(SELECT id FROM moz_bookmarks WHERE guid = :parentGuid),
(SELECT MAX(position) + 1 FROM moz_bookmarks WHERE parent = (SELECT id FROM moz_bookmarks WHERE guid = :parentGuid)),
:guid)
`, {
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url,
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
guid,
});
await db.execute(
`INSERT INTO moz_bookmarks (type, fk, parent, position, guid)
VALUES (:type,
(SELECT id FROM moz_places WHERE url = :url),
(SELECT id FROM moz_bookmarks WHERE guid = :parentGuid),
(SELECT MAX(position) + 1 FROM moz_bookmarks WHERE parent = (SELECT id FROM moz_bookmarks WHERE guid = :parentGuid)),
:guid)
`, {
type: PlacesUtils.bookmarks.TYPE_BOOKMARK,
url,
parentGuid: folderGuid,
guid: folderedGuid,
});
});
let guids = [folderGuid, guid];
await PT.Remove(guids).transact();
await ensureNonExistent(...guids, folderedGuid);
// Shouldn't throw, should restore the folder but not the bookmarks.
await PT.undo();
await ensureNonExistent(guid, folderedGuid);
Assert.ok(await PlacesUtils.bookmarks.fetch(folderGuid),
"The folder should have been re-created");
await PT.redo();
await ensureNonExistent(guids, folderedGuid);
// Cleanup
await PT.clearTransactionsHistory();
observer.reset();
});