Skip to content

Commit

Permalink
Merge pull request #4764: FIX(talking-ui): Crash after rename
Browse files Browse the repository at this point in the history
If the local user was added to the TalkingUI, renamed after that and
then changes its talking status, the client would crash.

The reason for this is that in TalkingUI::findOrAddUser we first create
the user's channel. Therefore we expect the channel to exist at a later
point in this function. However we also check whether the current name
of the user matches the name we have stored for it. If not (which is the
case after a rename), we remove that user. If the user was the only one
in its channel, the channel gets removed as well. In this case however
the original assumption of the user's channel already existing is not
valid anymore, ultimately leading to the crash.

The fix is to only create the user's channel after the name check in
order to make sure the assumption is always correct. Furthermore we also
explicitly check for the existence of the channel instead of simply
relying on it being present.
  • Loading branch information
Krzmbrzl authored Feb 16, 2021
2 parents cd7e9f9 + dab3d11 commit 1808273
Showing 1 changed file with 16 additions and 14 deletions.
30 changes: 16 additions & 14 deletions src/mumble/TalkingUI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -347,17 +347,11 @@ void TalkingUI::addChannel(const Channel *channel) {

layout()->addWidget(channelWidget);


m_containers.push_back(std::move(channelContainer));
}
}

TalkingUIUser *TalkingUI::findOrAddUser(const ClientUser *user) {
// In a first step, it has to be made sure that the user's channel
// exists in this UI.
addChannel(user->cChannel);


TalkingUIUser *oldUserEntry = findUser(user->uiSession);
bool nameMatches = true;

Expand All @@ -368,24 +362,32 @@ TalkingUIUser *TalkingUI::findOrAddUser(const ClientUser *user) {
nameMatches = oldUserEntry->getName() == user->qsName;

if (!nameMatches) {
// Hide the stale user
// Hide and remove the stale user
hideUser(user->uiSession);
// Remove the old user
removeUser(user->uiSession);

// reset pointer
oldUserEntry = nullptr;
}
}

// Make sure that the user's channel exists in this UI.
// Note that this has to be done **after** the name check above as
// the user might have been renamed in which case the abovementioned
// code block removes the old user entry. If that user was the only
// client in its channel, the channel gets removed as well. However
// the code below expects the user's channel to exist.
addChannel(user->cChannel);

if (!oldUserEntry || !nameMatches) {
// Create an entry for this user
bool isSelf = g.uiSession == user->uiSession;
// Create an Entry for this user (alongside the respective labels)
// We initially set the labels to not be visible, so that we'll
// enter the code-block further down.

std::unique_ptr< TalkingUIContainer > &channelContainer =
m_containers[findContainer(user->cChannel->iId, ContainerType::CHANNEL)];
int channelIndex = findContainer(user->cChannel->iId, ContainerType::CHANNEL);
if (channelIndex) {
qCritical("TalkingUI::findOrAddUser User's channel does not exist!");
return nullptr;
}
std::unique_ptr< TalkingUIContainer > &channelContainer = m_containers[channelIndex];
if (!channelContainer) {
qCritical("TalkingUI::findOrAddUser requesting unknown channel!");
return nullptr;
Expand Down

0 comments on commit 1808273

Please sign in to comment.