Prevent stack corruption when using C++ EventChannel (flutter/engine#36882)

This commit is contained in:
Loïc Sharma 2022-10-21 13:54:58 -07:00 committed by GitHub
parent 1a28581852
commit 888a2fd9cd
2 changed files with 123 additions and 67 deletions

View File

@ -33,7 +33,9 @@ class TestBinaryMessenger : public BinaryMessenger {
return last_message_handler_channel_;
}
BinaryMessageHandler last_message_handler() { return last_message_handler_; }
const BinaryMessageHandler& last_message_handler() {
return last_message_handler_;
}
private:
std::string last_message_handler_channel_;
@ -64,7 +66,7 @@ TEST(EventChannelTest, Registration) {
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
EXPECT_NE(messenger.last_message_handler(), nullptr);
// Send dummy listen message.
// Send test listen message.
MethodCall<> call("listen", nullptr);
auto message = codec.EncodeMethodCall(call);
messenger.last_message_handler()(
@ -121,7 +123,7 @@ TEST(EventChannelTest, Cancel) {
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
EXPECT_NE(messenger.last_message_handler(), nullptr);
// Send dummy listen message.
// Send test listen message.
MethodCall<> call_listen("listen", nullptr);
auto message = codec.EncodeMethodCall(call_listen);
messenger.last_message_handler()(
@ -129,7 +131,7 @@ TEST(EventChannelTest, Cancel) {
[](const uint8_t* reply, const size_t reply_size) {});
EXPECT_EQ(on_listen_called, true);
// Send dummy cancel message.
// Send test cancel message.
MethodCall<> call_cancel("cancel", nullptr);
message = codec.EncodeMethodCall(call_cancel);
messenger.last_message_handler()(
@ -165,7 +167,7 @@ TEST(EventChannelTest, ListenNotCancel) {
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
EXPECT_NE(messenger.last_message_handler(), nullptr);
// Send dummy listen message.
// Send test listen message.
MethodCall<> call_listen("listen", nullptr);
auto message = codec.EncodeMethodCall(call_listen);
messenger.last_message_handler()(
@ -205,7 +207,7 @@ TEST(EventChannelTest, ReRegistration) {
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
EXPECT_NE(messenger.last_message_handler(), nullptr);
// Send dummy listen message.
// Send test listen message.
MethodCall<> call("listen", nullptr);
auto message = codec.EncodeMethodCall(call);
messenger.last_message_handler()(
@ -213,7 +215,7 @@ TEST(EventChannelTest, ReRegistration) {
[](const uint8_t* reply, const size_t reply_size) {});
EXPECT_EQ(on_listen_called, true);
// Send second dummy message to test StreamHandler's OnCancel
// Send second test message to test StreamHandler's OnCancel
// method is called before OnListen method is called.
on_listen_called = false;
message = codec.EncodeMethodCall(call);
@ -226,4 +228,50 @@ TEST(EventChannelTest, ReRegistration) {
EXPECT_EQ(on_listen_called, true);
}
// Test that the handler is called even if the event channel is destroyed.
TEST(EventChannelTest, HandlerOutlivesEventChannel) {
TestBinaryMessenger messenger;
const std::string channel_name("some_channel");
const StandardMethodCodec& codec = StandardMethodCodec::GetInstance();
bool on_listen_called = false;
bool on_cancel_called = false;
{
EventChannel channel(&messenger, channel_name, &codec);
auto handler = std::make_unique<StreamHandlerFunctions<>>(
[&on_listen_called](const EncodableValue* arguments,
std::unique_ptr<EventSink<>>&& events)
-> std::unique_ptr<StreamHandlerError<>> {
on_listen_called = true;
return nullptr;
},
[&on_cancel_called](const EncodableValue* arguments)
-> std::unique_ptr<StreamHandlerError<>> {
on_cancel_called = true;
return nullptr;
});
channel.SetStreamHandler(std::move(handler));
}
// The event channel was destroyed but the handler should still be alive.
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
EXPECT_NE(messenger.last_message_handler(), nullptr);
// Send test listen message.
MethodCall<> call_listen("listen", nullptr);
auto message = codec.EncodeMethodCall(call_listen);
messenger.last_message_handler()(
message->data(), message->size(),
[](const uint8_t* reply, const size_t reply_size) {});
EXPECT_EQ(on_listen_called, true);
// Send test cancel message.
MethodCall<> call_cancel("cancel", nullptr);
message = codec.EncodeMethodCall(call_cancel);
messenger.last_message_handler()(
message->data(), message->size(),
[](const uint8_t* reply, const size_t reply_size) {});
EXPECT_EQ(on_cancel_called, true);
}
} // namespace flutter

View File

@ -47,10 +47,13 @@ class EventChannel {
// Registers a stream handler on this channel.
// If no handler has been registered, any incoming stream setup requests will
// be handled silently by providing an empty stream.
//
// Note that the EventChannel does not own the handler and will not
// unregister it on destruction. The caller is responsible for unregistering
// the handler if it should no longer be called.
void SetStreamHandler(std::unique_ptr<StreamHandler<T>> handler) {
if (!handler) {
messenger_->SetMessageHandler(name_, nullptr);
is_listening_ = false;
return;
}
@ -61,69 +64,75 @@ class EventChannel {
const MethodCodec<T>* codec = codec_;
const std::string channel_name = name_;
const BinaryMessenger* messenger = messenger_;
BinaryMessageHandler binary_handler = [shared_handler, codec, channel_name,
messenger,
this](const uint8_t* message,
const size_t message_size,
BinaryReply reply) {
constexpr char kOnListenMethod[] = "listen";
constexpr char kOnCancelMethod[] = "cancel";
BinaryMessageHandler binary_handler =
[shared_handler, codec, channel_name, messenger,
// Mutable state to track the handler's listening status.
is_listening = bool(false)](const uint8_t* message,
const size_t message_size,
BinaryReply reply) mutable {
constexpr char kOnListenMethod[] = "listen";
constexpr char kOnCancelMethod[] = "cancel";
std::unique_ptr<MethodCall<T>> method_call =
codec->DecodeMethodCall(message, message_size);
if (!method_call) {
std::cerr << "Unable to construct method call from message on channel: "
<< channel_name << std::endl;
reply(nullptr, 0);
return;
}
const std::string& method = method_call->method_name();
if (method.compare(kOnListenMethod) == 0) {
if (is_listening_) {
std::unique_ptr<StreamHandlerError<T>> error =
shared_handler->OnCancel(nullptr);
if (error) {
std::cerr << "Failed to cancel existing stream: "
<< (error->error_code) << ", " << (error->error_message)
<< ", " << (error->error_details);
std::unique_ptr<MethodCall<T>> method_call =
codec->DecodeMethodCall(message, message_size);
if (!method_call) {
std::cerr
<< "Unable to construct method call from message on channel: "
<< channel_name << std::endl;
reply(nullptr, 0);
return;
}
}
is_listening_ = true;
std::unique_ptr<std::vector<uint8_t>> result;
auto sink = std::make_unique<EventSinkImplementation>(
messenger, channel_name, codec);
std::unique_ptr<StreamHandlerError<T>> error =
shared_handler->OnListen(method_call->arguments(), std::move(sink));
if (error) {
result = codec->EncodeErrorEnvelope(
error->error_code, error->error_message, error->error_details);
} else {
result = codec->EncodeSuccessEnvelope();
}
reply(result->data(), result->size());
} else if (method.compare(kOnCancelMethod) == 0) {
std::unique_ptr<std::vector<uint8_t>> result;
if (is_listening_) {
std::unique_ptr<StreamHandlerError<T>> error =
shared_handler->OnCancel(method_call->arguments());
if (error) {
result = codec->EncodeErrorEnvelope(
error->error_code, error->error_message, error->error_details);
const std::string& method = method_call->method_name();
if (method.compare(kOnListenMethod) == 0) {
if (is_listening) {
std::unique_ptr<StreamHandlerError<T>> error =
shared_handler->OnCancel(nullptr);
if (error) {
std::cerr << "Failed to cancel existing stream: "
<< (error->error_code) << ", "
<< (error->error_message) << ", "
<< (error->error_details);
}
}
is_listening = true;
std::unique_ptr<std::vector<uint8_t>> result;
auto sink = std::make_unique<EventSinkImplementation>(
messenger, channel_name, codec);
std::unique_ptr<StreamHandlerError<T>> error =
shared_handler->OnListen(method_call->arguments(),
std::move(sink));
if (error) {
result = codec->EncodeErrorEnvelope(error->error_code,
error->error_message,
error->error_details);
} else {
result = codec->EncodeSuccessEnvelope();
}
reply(result->data(), result->size());
} else if (method.compare(kOnCancelMethod) == 0) {
std::unique_ptr<std::vector<uint8_t>> result;
if (is_listening) {
std::unique_ptr<StreamHandlerError<T>> error =
shared_handler->OnCancel(method_call->arguments());
if (error) {
result = codec->EncodeErrorEnvelope(error->error_code,
error->error_message,
error->error_details);
} else {
result = codec->EncodeSuccessEnvelope();
}
is_listening = false;
} else {
result = codec->EncodeErrorEnvelope(
"error", "No active stream to cancel", nullptr);
}
reply(result->data(), result->size());
} else {
result = codec->EncodeSuccessEnvelope();
reply(nullptr, 0);
}
is_listening_ = false;
} else {
result = codec->EncodeErrorEnvelope(
"error", "No active stream to cancel", nullptr);
}
reply(result->data(), result->size());
} else {
reply(nullptr, 0);
}
};
};
messenger_->SetMessageHandler(name_, std::move(binary_handler));
}
@ -165,7 +174,6 @@ class EventChannel {
BinaryMessenger* messenger_;
const std::string name_;
const MethodCodec<T>* codec_;
bool is_listening_ = false;
};
} // namespace flutter