Skip to content

Commit

Permalink
Added bookmark id validation check to pull sync and restore, fixes in…
Browse files Browse the repository at this point in the history 
…valid ids if found.

Removed id validation from restore form.
  • Loading branch information
@nero120
nero120 committed Jan 8, 2020
1 parent 89abcf7 commit ed354e7
Show file tree
Hide file tree
Showing 13 changed files with 70 additions and 100 deletions.
12 changes: 2 additions & 10 deletions js/app.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@ xBrowserSync.App.Controller = function ($q, $timeout, platform, globals, api, ut
};

var backupRestoreForm_DataToRestore_Change = function () {
vm.restoreForm.dataToRestore.$setValidity('DuplicateIds', true);
vm.restoreForm.dataToRestore.$setValidity('InvalidData', true);
};

Expand All @@ -312,7 +311,6 @@ xBrowserSync.App.Controller = function ($q, $timeout, platform, globals, api, ut
vm.settings.dataToRestore = '';
vm.settings.displayRestoreForm = true;
document.querySelector('#backupFile').value = null;
vm.restoreForm.dataToRestore.$setValidity('DuplicateIds', true);
vm.restoreForm.dataToRestore.$setValidity('InvalidData', true);

// Focus on restore textarea
Expand Down Expand Up @@ -2365,7 +2363,7 @@ xBrowserSync.App.Controller = function ($q, $timeout, platform, globals, api, ut
};

var validateBackupData = function () {
var xBookmarks, restoreData, validateData = false, validateUniqueIds = true;
var xBookmarks, restoreData, validateData = false;

if (!vm.settings.dataToRestore) {
validateData = false;
Expand All @@ -2382,13 +2380,7 @@ xBrowserSync.App.Controller = function ($q, $timeout, platform, globals, api, ut
catch (err) { }
vm.restoreForm.dataToRestore.$setValidity('InvalidData', validateData);

// Check for duplicate bookmark ids
if (validateData && xBookmarks) {
validateUniqueIds = bookmarks.CheckBookmarksHaveUniqueIds(xBookmarks);
vm.restoreForm.dataToRestore.$setValidity('DuplicateIds', validateUniqueIds);
}

return validateData && validateUniqueIds;
return validateData;
};

// Call constructor
Expand Down
99 changes: 57 additions & 42 deletions js/bookmarks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,42 +59,6 @@ xBrowserSync.App.Bookmarks = function ($q, $timeout, platform, globals, api, uti
});
};

var checkBookmarksHaveUniqueIds = function (bookmarks) {
// Find any bookmark without an id
var bookmarksHaveIds = true;
eachBookmark(bookmarks, function (bookmark) {
if (_.isUndefined(bookmark.id)) {
bookmarksHaveIds = false;
}
});

if (!bookmarksHaveIds) {
utility.LogWarning('Bookmarks missing ids');
return false;
}

// Get all local bookmarks into flat array
var allBookmarks = [];
eachBookmark(bookmarks, function (bookmark) {
allBookmarks.push(bookmark);
});

// Find a bookmark with a duplicate id
var duplicateId = _.chain(allBookmarks)
.countBy('id')
.findKey(function (count) {
return count > 1;
})
.value();

if (!_.isUndefined(duplicateId)) {
utility.LogWarning('Duplicate bookmark id detected: ' + duplicateId);
return false;
}

return true;
};

var checkIfRefreshSyncedDataOnError = function (err) {
return err && (
err.code === globals.ErrorCodes.ContainerChanged ||
Expand Down Expand Up @@ -594,6 +558,56 @@ xBrowserSync.App.Bookmarks = function ($q, $timeout, platform, globals, api, uti
});
};

var validateBookmarkIds = function (bookmarks) {
if (!bookmarks || bookmarks.length === 0) {
return true;
}

// Find any bookmark without an id
var bookmarksHaveIds = true;
eachBookmark(bookmarks, function (bookmark) {
if (_.isUndefined(bookmark.id)) {
bookmarksHaveIds = false;
}
});

if (!bookmarksHaveIds) {
utility.LogWarning('Bookmarks missing ids');
return false;
}

// Get all local bookmarks into flat array
var allBookmarks = [];
eachBookmark(bookmarks, function (bookmark) {
allBookmarks.push(bookmark);
});

// Find a bookmark with a non-numeric id
var invalidId = allBookmarks.find(function (bookmark) {
return typeof bookmark.id !== 'number';
});

if (!_.isUndefined(invalidId)) {
utility.LogWarning('Invalid bookmark id detected: ' + invalidId.id + ' (' + invalidId.url + ')');
return false;
}

// Find a bookmark with a duplicate id
var duplicateId = _.chain(allBookmarks)
.countBy('id')
.findKey(function (count) {
return count > 1;
})
.value();

if (!_.isUndefined(duplicateId)) {
utility.LogWarning('Duplicate bookmark id detected: ' + duplicateId);
return false;
}

return true;
};

var xBookmark = function (title, url, description, tags, children) {
var xBookmark = {};

Expand Down Expand Up @@ -1140,8 +1154,9 @@ xBrowserSync.App.Bookmarks = function ($q, $timeout, platform, globals, api, uti
var getBookmarksToSync, updateLocalBookmarksInfo;

if (syncData.bookmarks) {
// Sync with provided bookmarks
getBookmarksToSync = $q.resolve(syncData.bookmarks || []);
// Sync with provided bookmarks, validate bookmark ids
getBookmarksToSync = validateBookmarkIds(syncData.bookmarks) ?
$q.resolve(syncData.bookmarks) : platform.Bookmarks.AddIds(syncData.bookmarks);
}
else {
updateLocalBookmarksInfo = {
Expand Down Expand Up @@ -1239,10 +1254,10 @@ xBrowserSync.App.Bookmarks = function ($q, $timeout, platform, globals, api, uti
bookmarks = JSON.parse(decryptedData);

// Add new ids if bookmarks don't have unique ids
return !checkBookmarksHaveUniqueIds(bookmarks) && platform.Bookmarks.AddIds(bookmarks)
.then(function (bookmarksWithNewIds) {
return !validateBookmarkIds(bookmarks) && platform.Bookmarks.AddIds(bookmarks)
.then(function (bookmarksWithValidIds) {
// Encrypt bookmarks with new ids
bookmarks = bookmarksWithNewIds;
bookmarks = bookmarksWithValidIds;
return utility.EncryptData(JSON.stringify(bookmarks));
})
.then(function (encryptedBookmarksWithNewIds) {
Expand Down Expand Up @@ -1500,7 +1515,6 @@ xBrowserSync.App.Bookmarks = function ($q, $timeout, platform, globals, api, uti

return {
AddNewInXBookmarks: addNewInXBookmarks,
CheckBookmarksHaveUniqueIds: checkBookmarksHaveUniqueIds,
CheckIfRefreshSyncedDataOnError: checkIfRefreshSyncedDataOnError,
CheckForUpdates: checkForUpdates,
CleanBookmark: cleanBookmark,
Expand All @@ -1523,6 +1537,7 @@ xBrowserSync.App.Bookmarks = function ($q, $timeout, platform, globals, api, uti
SyncSize: getSyncSize,
UpdateExistingInXBookmarks: updateExistingInXBookmarks,
UpgradeContainers: upgradeContainers,
ValidateBookmarkIds: validateBookmarkIds,
XBookmark: xBookmark,
XBookmarkIsContainer: xBookmarkIsContainer,
XSeparator: xSeparator,
Expand Down
3 changes: 1 addition & 2 deletions js/global.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ xBrowserSync.App.Global = function (platform) {
Settings_BackupRestore_RestoreForm_Message: 'settings_BackupRestore_RestoreForm_Message',
Settings_BackupRestore_RestoreForm_DataField_Label: 'settings_BackupRestore_RestoreForm_DataField_Label',
Settings_BackupRestore_RestoreForm_Invalid_Label: 'settings_BackupRestore_RestoreForm_Invalid_Label',
Settings_BackupRestore_RestoreForm_DuplicateIds_Label: 'settings_BackupRestore_RestoreForm_DuplicateIds_Label',
Settings_Sync_Title: 'settings_Sync_Title',
Settings_Sync_Id_Label: 'settings_Sync_Id_Label',
Settings_Sync_DisplayQRCode_Message: 'settings_Sync_DisplayQRCode_Message',
Expand Down Expand Up @@ -315,7 +314,7 @@ xBrowserSync.App.Global = function (platform) {
ContainerChanged: 10112,
LocalContainerNotFound: 10113,
DataOutOfSync: 10114,
DuplicateBookmarkIdsDetected: 10115,
InvalidBookmarkIdsDetected: 10115,
SyncUncommitted: 10200,
InvalidService: 10300,
UnsupportedServiceApiVersion: 10301,
Expand Down
3 changes: 0 additions & 3 deletions platform/android/js/platformImplementation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,6 @@ xBrowserSync.App.PlatformImplementation = function ($interval, $q, $timeout, pla
"settings_BackupRestore_RestoreForm_Invalid_Label": {
"message": "Invalid xBrowserSync backup data"
},
"settings_BackupRestore_RestoreForm_DuplicateIds_Label": {
"message": "Duplicate bookmarks IDs detected"
},
"button_SelectBackupFile_Label": {
"message": "Select file"
},
Expand Down
3 changes: 0 additions & 3 deletions platform/chrome/_locales/de/messages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,6 @@
"settings_BackupRestore_RestoreForm_Invalid_Label": {
"message": "Invalid xBrowserSync backup data"
},
"settings_BackupRestore_RestoreForm_DuplicateIds_Label": {
"message": "Duplicate bookmarks IDs detected"
},
"button_SelectBackupFile_Label": {
"message": "Datei auswählen"
},
Expand Down
3 changes: 0 additions & 3 deletions platform/chrome/_locales/en/messages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,6 @@
"settings_BackupRestore_RestoreForm_Invalid_Label": {
"message": "Invalid xBrowserSync backup data"
},
"settings_BackupRestore_RestoreForm_DuplicateIds_Label": {
"message": "Duplicate bookmarks IDs detected"
},
"button_SelectBackupFile_Label": {
"message": "Select file"
},
Expand Down
3 changes: 0 additions & 3 deletions platform/chrome/_locales/fr/messages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,6 @@
"settings_BackupRestore_RestoreForm_Invalid_Label": {
"message": "Invalid xBrowserSync backup data"
},
"settings_BackupRestore_RestoreForm_DuplicateIds_Label": {
"message": "Duplicate bookmarks IDs detected"
},
"button_SelectBackupFile_Label": {
"message": "Selectionner le Fichier"
},
Expand Down
10 changes: 1 addition & 9 deletions platform/chrome/js/background.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -822,15 +822,7 @@ xBrowserSync.App.Background = function ($q, $timeout, platform, globals, utility
})
.then(function () {
// Upgrade containers to use current container names
var upgradedBookmarks = bookmarks.UpgradeContainers(restoreData.bookmarks || []);

// If bookmarks don't have unique ids, add new ids
return !bookmarks.CheckBookmarksHaveUniqueIds(upgradedBookmarks) ?
platform.Bookmarks.AddIds(upgradedBookmarks)
.then(function (updatedBookmarks) {
return updatedBookmarks;
}) :
upgradedBookmarks;
return bookmarks.UpgradeContainers(restoreData.bookmarks || []);
})
.then(function (bookmarksToRestore) {
// Queue sync
Expand Down
9 changes: 4 additions & 5 deletions platform/chrome/js/platformImplementation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ xBrowserSync.App.PlatformImplementation = function ($interval, $q, $timeout, pla
* ------------------------------------------------------------------------------------ */

var addIdsToBookmarks = function (xBookmarks) {
// Get all local bookmarks into array
return $q.all([
getLocalBookmarkTree(),
getLocalContainerIds()
Expand Down Expand Up @@ -131,10 +130,10 @@ xBrowserSync.App.PlatformImplementation = function ($interval, $q, $timeout, pla
};
_.each(xBookmarks, addIdToBookmark);

// Check that bookmarks now have unique ids
var bookmarksHaveUniqueIds = bookmarks.CheckBookmarksHaveUniqueIds(xBookmarks);
if (!bookmarksHaveUniqueIds) {
return $q.reject({ code: globals.ErrorCodes.DuplicateBookmarkIdsDetected });
// Check that bookmarks now have valid ids
var bookmarksHaveValidIds = bookmarks.ValidateBookmarkIds(xBookmarks);
if (!bookmarksHaveValidIds) {
return $q.reject({ code: globals.ErrorCodes.InvalidBookmarkIdsDetected });
}

return xBookmarks;
Expand Down
3 changes: 0 additions & 3 deletions platform/firefox/_locales/en/messages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,6 @@
"settings_BackupRestore_RestoreForm_Invalid_Label": {
"message": "Invalid xBrowserSync backup data"
},
"settings_BackupRestore_RestoreForm_DuplicateIds_Label": {
"message": "Duplicate bookmarks IDs detected"
},
"button_SelectBackupFile_Label": {
"message": "Select file"
},
Expand Down
10 changes: 1 addition & 9 deletions platform/firefox/js/background.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -735,15 +735,7 @@ xBrowserSync.App.Background = function ($q, $timeout, platform, globals, utility
})
.then(function () {
// Upgrade containers to use current container names
var upgradedBookmarks = bookmarks.UpgradeContainers(restoreData.bookmarks || []);

// If bookmarks don't have unique ids, add new ids
return !bookmarks.CheckBookmarksHaveUniqueIds(upgradedBookmarks) ?
platform.Bookmarks.AddIds(upgradedBookmarks)
.then(function (updatedBookmarks) {
return updatedBookmarks;
}) :
upgradedBookmarks;
return bookmarks.UpgradeContainers(restoreData.bookmarks || []);
})
.then(function (bookmarksToRestore) {
// Queue sync
Expand Down
9 changes: 4 additions & 5 deletions platform/firefox/js/platformImplementation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ xBrowserSync.App.PlatformImplementation = function ($interval, $q, $timeout, pla
* ------------------------------------------------------------------------------------ */

var addIdsToBookmarks = function (xBookmarks) {
// Get all bookmarks into array
return $q.all([
getLocalBookmarkTree(),
getLocalContainerIds()
Expand Down Expand Up @@ -128,10 +127,10 @@ xBrowserSync.App.PlatformImplementation = function ($interval, $q, $timeout, pla
};
_.each(xBookmarks, addIdToBookmark);

// Check that bookmarks now have unique ids
var bookmarksHaveUniqueIds = bookmarks.CheckBookmarksHaveUniqueIds(xBookmarks);
if (!bookmarksHaveUniqueIds) {
return $q.reject({ code: globals.ErrorCodes.DuplicateBookmarkIdsDetected });
// Check that bookmarks now have valid ids
var bookmarksHaveValidIds = bookmarks.ValidateBookmarkIds(xBookmarks);
if (!bookmarksHaveValidIds) {
return $q.reject({ code: globals.ErrorCodes.InvalidBookmarkIdsDetected });
}

return xBookmarks;
Expand Down
3 changes: 0 additions & 3 deletions views/settings.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,6 @@ <h4>{{ vm.platform.GetConstant(vm.globals.Constants.Settings_BackupRestore_Title
<span data-ng-if="vm.restoreForm.dataToRestore.$error.InvalidData">
{{ vm.platform.GetConstant(vm.globals.Constants.Settings_BackupRestore_RestoreForm_Invalid_Label) }}
</span>
<span data-ng-if="vm.restoreForm.dataToRestore.$error.DuplicateIds">
{{ vm.platform.GetConstant(vm.globals.Constants.Settings_BackupRestore_RestoreForm_DuplicateIds_Label) }}
</span>
</div>
<span class="spinner-border spinner-border-sm" data-ng-show="vm.settings.validatingRestoreData"
role="status" aria-hidden="true"></span>
Expand Down

0 comments on commit ed354e7

Please sign in to comment.