Fixes an error with the handled flag for certificate exchange

Summary:
This change fixes a bug with the handled flag during the certificate exchange process.

Explanation:
handled was passed by reference as &handled
Once the function goes out of scope then the reference, well, it just becomes invalid (undefined behaviour)
In some cases, it appears as 'handled' because the reference is invalid and it happens to be 'true'.

Changelog: Fixed an issue where clients would randomly not connect to Flipper. Please update FlipperKit to 0.110.0 to apply the fix: https://fbflipper.com/docs/getting-started/react-native#using-the-latest-flipper-sdk

Reviewed By: mweststrate

Differential Revision: D31017592

fbshipit-source-id: c087a769fa23de1acfd3c198b4db4d6ccdb2be90
This commit is contained in:
Lorenzo Blasa
2021-09-17 07:33:03 -07:00
committed by Facebook GitHub Bot
parent c865446312
commit 10e97a7e98
2 changed files with 8 additions and 5 deletions

View File

@@ -366,12 +366,13 @@ void FlipperConnectionManagerImpl::requestSignedCertFromFlipper() {
"destination", "destination",
contextStore_->getCertificateDirectoryPath().c_str())("medium", medium); contextStore_->getCertificateDirectoryPath().c_str())("medium", medium);
auto gettingCert = flipperState_->start("Getting cert from desktop"); auto gettingCert = flipperState_->start("Getting cert from desktop");
bool handled = false;
flipperEventBase_->add([this, &handled, message, gettingCert]() { certificateExchangeCompleted_ = false;
flipperEventBase_->add([this, message, gettingCert]() {
client_->sendExpectResponse( client_->sendExpectResponse(
folly::toJson(message), folly::toJson(message),
[this, &handled, message, gettingCert]( [this, message, gettingCert](
const std::string& response, bool isError) { const std::string& response, bool isError) {
/** /**
Need to keep track of whether the response has been handled. Need to keep track of whether the response has been handled.
@@ -382,9 +383,9 @@ void FlipperConnectionManagerImpl::requestSignedCertFromFlipper() {
interrupted because we are effectively still handing the response interrupted because we are effectively still handing the response
read. So, if already handled, ignore and return; read. So, if already handled, ignore and return;
*/ */
if (handled) if (certificateExchangeCompleted_)
return; return;
handled = true; certificateExchangeCompleted_ = true;
if (isError) { if (isError) {
if (response.compare("not implemented")) { if (response.compare("not implemented")) {
auto error = auto error =

View File

@@ -68,6 +68,8 @@ class FlipperConnectionManagerImpl : public FlipperConnectionManager {
std::unique_ptr<FlipperSocket> client_; std::unique_ptr<FlipperSocket> client_;
bool connectionIsTrusted_; bool connectionIsTrusted_;
bool certificateExchangeCompleted_ = false;
int failedConnectionAttempts_ = 0; int failedConnectionAttempts_ = 0;
std::shared_ptr<ConnectionContextStore> contextStore_; std::shared_ptr<ConnectionContextStore> contextStore_;
std::shared_ptr<FlipperConnectionManagerWrapper> implWrapper_; std::shared_ptr<FlipperConnectionManagerWrapper> implWrapper_;