Fix database merge crash when fdosecrets is enabled (#10136)

* Entry: re-parent before adding to new group

Adding the Entry to the Group will emit signals about the action.
Present the object with the correct parent already.

* fdosecrets: Item::Create() can fail

If an entry cannot be registered on DBus, Item::Create() will return a
nullptr. Basically, this can only happen if there is already an item
with the same UUID in the collection. The only viable option here is to
ignore the new entry.

* Merger: prevent duplicate entry when merging histories

If the source entry is newer, a copy of the entry is made. But before
moving the merged entry to the target group, it must be removed.
Otherwise there will be briefly two entries with the same UUID
in the same group/database.

Even though this is only the case during the transaction, it can still
be observed because the operations emit signals. A notable problem is
the fdosecrets feature that relies on the uniqueness of the UUID or will
otherwise run into problems because the UUID is used as part of the DBus
path.

src/core/Entry.cpp

@@ -1279,11 +1279,11 @@ void Entry::setGroup(Group* group, bool trackPrevious)
         }
     }
 
+    QObject::setParent(group);
+
     m_group = group;
     group->addEntry(this);
 
-    QObject::setParent(group);
-
     if (m_updateTimeinfo) {
         m_data.timeInfo.setLocationChanged(Clock::currentDateTimeUtc());
     }

src/core/Merger.cpp

@@ -338,6 +338,7 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex
     const int comparison = compare(targetEntry->timeInfo().lastModificationTime(),
                                    sourceEntry->timeInfo().lastModificationTime(),
                                    CompareItemIgnoreMilliseconds);
+    const int maxItems = targetEntry->database()->metadata()->historyMaxItems();
     if (comparison < 0) {
         Group* currentGroup = targetEntry->group();
         Entry* clonedEntry = sourceEntry->clone(Entry::CloneIncludeHistory);
@@ -346,15 +347,15 @@ Merger::ChangeList Merger::resolveEntryConflict_MergeHistories(const MergeContex
                qPrintable(sourceEntry->title()),
                qPrintable(currentGroup->name()));
         changes << tr("Synchronizing from newer source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex());
-        moveEntry(clonedEntry, currentGroup);
-        mergeHistory(targetEntry, clonedEntry, mergeMethod);
+        mergeHistory(targetEntry, clonedEntry, mergeMethod, maxItems);
         eraseEntry(targetEntry);
+        moveEntry(clonedEntry, currentGroup);
     } else {
         qDebug("Merge %s/%s with local on top/under %s",
                qPrintable(targetEntry->title()),
                qPrintable(sourceEntry->title()),
                qPrintable(targetEntry->group()->name()));
-        const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod);
+        const bool changed = mergeHistory(sourceEntry, targetEntry, mergeMethod, maxItems);
         if (changed) {
             changes
                 << tr("Synchronizing from older source %1 [%2]").arg(targetEntry->title(), targetEntry->uuidToHex());
@@ -400,7 +401,10 @@ Merger::resolveEntryConflict(const MergeContext& context, const Entry* sourceEnt
     return changes;
 }
 
-bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod)
+bool Merger::mergeHistory(const Entry* sourceEntry,
+                          Entry* targetEntry,
+                          Group::MergeMode mergeMethod,
+                          const int maxItems)
 {
     Q_UNUSED(mergeMethod);
     const auto targetHistoryItems = targetEntry->historyItems();
@@ -473,7 +477,6 @@ bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::M
     }
 
     bool changed = false;
-    const int maxItems = targetEntry->database()->metadata()->historyMaxItems();
     const auto updatedHistoryItems = merged.values();
     for (int i = 0; i < maxItems; ++i) {
         const Entry* oldEntry = targetHistoryItems.value(targetHistoryItems.count() - i);

src/core/Merger.h

@@ -50,7 +50,7 @@ class Merger : public QObject
     ChangeList mergeDeletions(const MergeContext& context);
     ChangeList mergeMetadata(const MergeContext& context);
     bool markOlderEntry(Entry* entry);
-    bool mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod);
+    bool mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::MergeMode mergeMethod, const int maxItems);
     void moveEntry(Entry* entry, Group* targetGroup);
     void moveGroup(Group* group, Group* targetGroup);
     // remove an entry without a trace in the deletedObjects - needed for elemination cloned entries

src/fdosecrets/objects/Collection.cpp

@@ -495,6 +495,10 @@ namespace FdoSecrets
         }
 
         auto item = Item::Create(this, entry);
+        if (!item) {
+            return;
+        }
+
         m_items << item;
         m_entryToItem[entry] = item;