From 710f713511e1d24343bc3a53bb588e9ffdbfee0c Mon Sep 17 00:00:00 2001 From: Hugo Lefeuvre <hugo.lefeuvre@savoirfairelinux.com> Date: Fri, 25 May 2018 09:39:47 -0400 Subject: [PATCH] conversationmodel: send file to create contact The sendFile() method contains contradictory code: "if (peerUri.empty()) return" forbids sending files to non contacts, but a line later "pimpl_->sendContactRequest(peerUri)" is called even if this line can only be executed in the case where peerUri is already a contact. In this commit we allow sending files to non contacts (this makes sense if peer has already accepted the sender as contact), and protect it with the same synchronization mechanism as af04c34. Change-Id: I25432fcfdd3f46068add27009a77108ac75d3124 Reviewed-by: Andreas Traczyk <andreas.traczyk@savoirfairelinux.com> --- src/conversationmodel.cpp | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/src/conversationmodel.cpp b/src/conversationmodel.cpp index 899d1cf8..3562b137 100644 --- a/src/conversationmodel.cpp +++ b/src/conversationmodel.cpp @@ -1690,13 +1690,43 @@ ConversationModel::sendFile(const std::string& convUid, if (conversationIdx == -1) return; - const auto& peerUri = pimpl_->conversations[conversationIdx].participants.front(); - if (peerUri.empty()) - return; + const auto peerUri = pimpl_->conversations[conversationIdx].participants.front(); + bool isTemporary = peerUri.empty(); + + /* It is necessary to make a copy of convUid since it may very well point to + a field in the temporary conversation, which is going to be destroyed by + slotContactAdded() (indirectly triggered by sendContactrequest(). Not doing + so may result in use after free/crash. */ + auto convUidCopy = convUid; pimpl_->sendContactRequest(peerUri); + + if (isTemporary) { + /* Block until we are sure that the final conversation was created by + slotContactAdded(). If adding contact failed we should not process + any further */ + std::unique_lock<std::mutex> lock(pimpl_->m_mutex_conversations); + auto res = pimpl_->m_condVar_conversation_creation.wait_for(lock, std::chrono::seconds(2), + [&]() { return pimpl_->indexOfContact(convUidCopy) >= 0; }); + lock.unlock(); + + if (!res) { + qDebug() << "ConversationModel::sendFile reached timeout while waiting for contact to be added. Couldn't send file."; + return; + } + } + + int contactIndex; + if (isTemporary && (contactIndex = pimpl_->indexOfContact(convUidCopy)) < 0) { + qDebug() << "Can't send file: Other participant is not a contact (removed while sending file ?)"; + return; + } + + // Retrieve final peer uri after creation of the conversation + const auto& newPeerUri = isTemporary ? pimpl_->conversations.at(contactIndex).participants.front() : peerUri; + pimpl_->lrc.getDataTransferModel().sendFile(owner.id.c_str(), - peerUri.c_str(), + newPeerUri.c_str(), path.c_str(), filename.c_str()); } -- GitLab