Skip to content

fix: correct notification ID handling and validation #1417

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

Merged
18202781743 merged 1 commit into from
Jan 30, 2026
Merged

fix: correct notification ID handling and validation #1417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion panels/notification/bubble/bubblemodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ QVariant BubbleModel::data(const QModelIndex &index, int role) const
case BubbleModel::AppName:
return m_bubbles[row]->appName();
case BubbleModel::Id:
return m_bubbles[row]->bubbleId();
return m_bubbles[row]->id();
case BubbleModel::Body:
return m_bubbles[row]->body();
case BubbleModel::Summary:
Expand Down
2 changes: 1 addition & 1 deletion panels/notification/bubble/bubblepanel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ void BubblePanel::addBubble(qint64 id)

void BubblePanel::closeBubble(qint64 id)
{
if (id > 0) {
if (id != NotifyEntity::InvalidId) {
m_bubbles->removeById(id);
} else {
qWarning(notifyLog) << "Failed to close bubble: invalid bubble id for entity id:" << id;
Expand Down
3 changes: 1 addition & 2 deletions panels/notification/common/dataaccessorproxy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ NotifyEntity DataAccessorProxy::fetchLastEntity(uint notifyId)
auto entity = m_impl->fetchLastEntity(notifyId);
if (entity.isValid())
return entity;

return m_source->fetchLastEntity(notifyId);
return {};
}

QList<NotifyEntity> DataAccessorProxy::fetchEntities(const QString &appName, int processedType, int maxCount)
Expand Down
3 changes: 2 additions & 1 deletion panels/notification/common/memoryaccessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ MemoryAccessor::~MemoryAccessor()

qint64 MemoryAccessor::addEntity(const NotifyEntity &entity)
{
static qint64 g_Id = NotifyEntity::InvalidId; // use negative id for in-memory entities
QMutexLocker locker(&m_mutex);
m_entities << entity;
return entity.bubbleId();
return --g_Id;
Comment on lines +18 to +21
Copy link

@sourcery-ai sourcery-ai bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Consider updating the stored entity's id to match the generated in-memory id.

In addEntity, we store entity unchanged but return a decremented g_Id. Unless NotifyEntity is updated elsewhere, entity.id() will stay InvalidId while callers may rely on the returned negative id. Any code that later uses entity.id() (e.g., for comparisons or lookups) will then see a different id than the one returned here. Consider cloning/updating the entity with the generated id before storing it so the stored entity and returned id are consistent.

}

qint64 MemoryAccessor::replaceEntity(qint64 id, const NotifyEntity &entity)
Expand Down
4 changes: 2 additions & 2 deletions panels/notification/common/notifyentity.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class NotifyData : public QSharedData
uint replacesId = NoReplaceId;
int expireTimeout = 0;

qint64 id = -1;
qint64 id = NotifyEntity::InvalidId;
qint64 cTime = 0;
int processedType = NotifyEntity::NotProcessed;
};
Expand Down Expand Up @@ -118,7 +118,7 @@ bool NotifyEntity::operator!=(const NotifyEntity &other) const

bool NotifyEntity::isValid() const
{
return d && d->id > 0 && d->cTime > 0;
return d && d->id != NotifyEntity::InvalidId && d->cTime > 0;
}

qint64 NotifyEntity::id() const
Expand Down
2 changes: 2 additions & 0 deletions panels/notification/common/notifyentity.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class NotifyEntity
Timeout = 5,
};

static constexpr qint64 InvalidId = 0;

NotifyEntity();
explicit NotifyEntity(qint64 id, const QString &appName);
explicit NotifyEntity(const QString &appName, uint replacesId, const QString &appIcon, const QString &summary,
Expand Down
7 changes: 4 additions & 3 deletions panels/notification/server/notificationmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ bool NotificationManager::isDoNotDisturb() const

bool NotificationManager::recordNotification(NotifyEntity &entity)
{
qint64 id = -1;
qint64 id = NotifyEntity::InvalidId;
if (entity.isReplace()) {
auto lastEntity = m_persistence->fetchLastEntity(entity.bubbleId());
if (lastEntity.isValid()) {
Expand All @@ -393,13 +393,14 @@ bool NotificationManager::recordNotification(NotifyEntity &entity)
}
} else {
qWarning() << "Not exist notification to replace for the replaceId" << entity.replacesId();
return false;
}
}
if (id == -1) {
if (id == NotifyEntity::InvalidId) {
id = m_persistence->addEntity(entity);
}

if (id == -1) {
if (id == NotifyEntity::InvalidId) {
qWarning(notifyLog) << "Failed on saving DB, bubbleId:" << entity.bubbleId() << ", appName" << entity.appName();
return false;
}
Expand Down