Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix database merge crash when fdosecrets is enabled #10136

Merged
merged 3 commits into from
Jan 2, 2024
Merged

Fix database merge crash when fdosecrets is enabled #10136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
86695bd
Entry: re-parent before adding to new group
jkloetzke Dec 29, 2023
33bf377
fdosecrets: Item::Create() can fail
jkloetzke Dec 29, 2023
e18a186
Merger: prevent duplicate entry when merging histories
jkloetzke Dec 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/core/Entry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,11 +1277,11 @@ void Entry::setGroup(Group* group, bool trackPrevious)
}
}

QObject::setParent(group);
droidmonkey marked this conversation as resolved.
Show resolved Hide resolved

m_group = group;
group->addEntry(this);

QObject::setParent(group);

if (m_updateTimeinfo) {
m_data.timeInfo.setLocationChanged(Clock::currentDateTimeUtc());
}
Expand Down
13 changes: 8 additions & 5 deletions src/core/Merger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,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);
Expand All @@ -269,15 +270,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());
Expand All @@ -297,7 +298,10 @@ Merger::resolveEntryConflict(const MergeContext& context, const Entry* sourceEnt
return resolveEntryConflict_MergeHistories(context, sourceEntry, targetEntry, mergeMode);
}

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();
Expand Down Expand Up @@ -370,7 +374,6 @@ bool Merger::mergeHistory(const Entry* sourceEntry, Entry* targetEntry, Group::M
}

bool changed = false;
const int maxItems = targetEntry->database()->metadata()->historyMaxItems();
droidmonkey marked this conversation as resolved.
Show resolved Hide resolved
const auto updatedHistoryItems = merged.values();
for (int i = 0; i < maxItems; ++i) {
const Entry* oldEntry = targetHistoryItems.value(targetHistoryItems.count() - i);
Expand Down
2 changes: 1 addition & 1 deletion src/core/Merger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class Merger : public QObject
ChangeList mergeGroup(const MergeContext& context);
ChangeList mergeDeletions(const MergeContext& context);
ChangeList mergeMetadata(const MergeContext& context);
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
Expand Down
4 changes: 4 additions & 0 deletions src/fdosecrets/objects/Collection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,10 @@ namespace FdoSecrets
}

auto item = Item::Create(this, entry);
if (!item) {
return;
}

m_items << item;
m_entryToItem[entry] = item;

Expand Down