Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 6aa683e

Browse files
loic-sharmaloic-sharma
authored
Prevent stack corruption when using C++ EventChannel (#36882)
1 parent 1d09802 commit 6aa683e

File tree

2 files changed

+125
-69
lines changed

2 files changed

+125
-69
lines changed

‎shell/platform/common/client_wrapper/event_channel_unittests.cc‎

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ class TestBinaryMessenger : public BinaryMessenger {
3333
return last_message_handler_channel_;
3434
}
3535

36-
BinaryMessageHandler last_message_handler() { return last_message_handler_; }
36+
const BinaryMessageHandler& last_message_handler() {
37+
return last_message_handler_;
38+
}
3739

3840
private:
3941
std::string last_message_handler_channel_;
@@ -64,7 +66,7 @@ TEST(EventChannelTest, Registration) {
6466
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
6567
EXPECT_NE(messenger.last_message_handler(), nullptr);
6668

67-
// Send dummy listen message.
69+
// Send test listen message.
6870
MethodCall<> call("listen", nullptr);
6971
auto message = codec.EncodeMethodCall(call);
7072
messenger.last_message_handler()(
@@ -121,15 +123,15 @@ TEST(EventChannelTest, Cancel) {
121123
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
122124
EXPECT_NE(messenger.last_message_handler(), nullptr);
123125

124-
// Send dummy listen message.
126+
// Send test listen message.
125127
MethodCall<> call_listen("listen", nullptr);
126128
auto message = codec.EncodeMethodCall(call_listen);
127129
messenger.last_message_handler()(
128130
message->data(), message->size(),
129131
[](const uint8_t* reply, const size_t reply_size) {});
130132
EXPECT_EQ(on_listen_called, true);
131133

132-
// Send dummy cancel message.
134+
// Send test cancel message.
133135
MethodCall<> call_cancel("cancel", nullptr);
134136
message = codec.EncodeMethodCall(call_cancel);
135137
messenger.last_message_handler()(
@@ -165,7 +167,7 @@ TEST(EventChannelTest, ListenNotCancel) {
165167
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
166168
EXPECT_NE(messenger.last_message_handler(), nullptr);
167169

168-
// Send dummy listen message.
170+
// Send test listen message.
169171
MethodCall<> call_listen("listen", nullptr);
170172
auto message = codec.EncodeMethodCall(call_listen);
171173
messenger.last_message_handler()(
@@ -205,15 +207,15 @@ TEST(EventChannelTest, ReRegistration) {
205207
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
206208
EXPECT_NE(messenger.last_message_handler(), nullptr);
207209

208-
// Send dummy listen message.
210+
// Send test listen message.
209211
MethodCall<> call("listen", nullptr);
210212
auto message = codec.EncodeMethodCall(call);
211213
messenger.last_message_handler()(
212214
message->data(), message->size(),
213215
[](const uint8_t* reply, const size_t reply_size) {});
214216
EXPECT_EQ(on_listen_called, true);
215217

216-
// Send second dummy message to test StreamHandler's OnCancel
218+
// Send second test message to test StreamHandler's OnCancel
217219
// method is called before OnListen method is called.
218220
on_listen_called = false;
219221
message = codec.EncodeMethodCall(call);
@@ -226,4 +228,50 @@ TEST(EventChannelTest, ReRegistration) {
226228
EXPECT_EQ(on_listen_called, true);
227229
}
228230

231+
// Test that the handler is called even if the event channel is destroyed.
232+
TEST(EventChannelTest, HandlerOutlivesEventChannel) {
233+
TestBinaryMessenger messenger;
234+
const std::string channel_name("some_channel");
235+
const StandardMethodCodec& codec = StandardMethodCodec::GetInstance();
236+
237+
bool on_listen_called = false;
238+
bool on_cancel_called = false;
239+
{
240+
EventChannel channel(&messenger, channel_name, &codec);
241+
auto handler = std::make_unique<StreamHandlerFunctions<>>(
242+
[&on_listen_called](const EncodableValue* arguments,
243+
std::unique_ptr<EventSink<>>&& events)
244+
-> std::unique_ptr<StreamHandlerError<>> {
245+
on_listen_called = true;
246+
return nullptr;
247+
},
248+
[&on_cancel_called](const EncodableValue* arguments)
249+
-> std::unique_ptr<StreamHandlerError<>> {
250+
on_cancel_called = true;
251+
return nullptr;
252+
});
253+
channel.SetStreamHandler(std::move(handler));
254+
}
255+
256+
// The event channel was destroyed but the handler should still be alive.
257+
EXPECT_EQ(messenger.last_message_handler_channel(), channel_name);
258+
EXPECT_NE(messenger.last_message_handler(), nullptr);
259+
260+
// Send test listen message.
261+
MethodCall<> call_listen("listen", nullptr);
262+
auto message = codec.EncodeMethodCall(call_listen);
263+
messenger.last_message_handler()(
264+
message->data(), message->size(),
265+
[](const uint8_t* reply, const size_t reply_size) {});
266+
EXPECT_EQ(on_listen_called, true);
267+
268+
// Send test cancel message.
269+
MethodCall<> call_cancel("cancel", nullptr);
270+
message = codec.EncodeMethodCall(call_cancel);
271+
messenger.last_message_handler()(
272+
message->data(), message->size(),
273+
[](const uint8_t* reply, const size_t reply_size) {});
274+
EXPECT_EQ(on_cancel_called, true);
275+
}
276+
229277
} // namespace flutter

‎shell/platform/common/client_wrapper/include/flutter/event_channel.h‎

Lines changed: 70 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,13 @@ class EventChannel {
4747
// Registers a stream handler on this channel.
4848
// If no handler has been registered, any incoming stream setup requests will
4949
// be handled silently by providing an empty stream.
50+
//
51+
// Note that the EventChannel does not own the handler and will not
52+
// unregister it on destruction. The caller is responsible for unregistering
53+
// the handler if it should no longer be called.
5054
void SetStreamHandler(std::unique_ptr<StreamHandler<T>> handler) {
5155
if (!handler) {
5256
messenger_->SetMessageHandler(name_, nullptr);
53-
is_listening_ = false;
5457
return;
5558
}
5659

@@ -61,69 +64,75 @@ class EventChannel {
6164
const MethodCodec<T>* codec = codec_;
6265
const std::string channel_name = name_;
6366
const BinaryMessenger* messenger = messenger_;
64-
BinaryMessageHandler binary_handler = [shared_handler, codec, channel_name,
65-
messenger,
66-
this](const uint8_t* message,
67-
const size_t message_size,
68-
BinaryReply reply) {
69-
constexpr char kOnListenMethod[] = "listen";
70-
constexpr char kOnCancelMethod[] = "cancel";
71-
72-
std::unique_ptr<MethodCall<T>> method_call =
73-
codec->DecodeMethodCall(message, message_size);
74-
if (!method_call) {
75-
std::cerr << "Unable to construct method call from message on channel: "
76-
<< channel_name << std::endl;
77-
reply(nullptr, 0);
78-
return;
79-
}
80-
81-
const std::string& method = method_call->method_name();
82-
if (method.compare(kOnListenMethod) == 0) {
83-
if (is_listening_) {
84-
std::unique_ptr<StreamHandlerError<T>> error =
85-
shared_handler->OnCancel(nullptr);
86-
if (error) {
87-
std::cerr << "Failed to cancel existing stream: "
88-
<< (error->error_code) << ", " << (error->error_message)
89-
<< ", " << (error->error_details);
67+
BinaryMessageHandler binary_handler =
68+
[shared_handler, codec, channel_name, messenger,
69+
// Mutable state to track the handler's listening status.
70+
is_listening = bool(false)](const uint8_t* message,
71+
const size_t message_size,
72+
BinaryReply reply) mutable {
73+
constexpr char kOnListenMethod[] = "listen";
74+
constexpr char kOnCancelMethod[] = "cancel";
75+
76+
std::unique_ptr<MethodCall<T>> method_call =
77+
codec->DecodeMethodCall(message, message_size);
78+
if (!method_call) {
79+
std::cerr
80+
<< "Unable to construct method call from message on channel: "
81+
<< channel_name << std::endl;
82+
reply(nullptr, 0);
83+
return;
9084
}
91-
}
92-
is_listening_ = true;
93-
94-
std::unique_ptr<std::vector<uint8_t>> result;
95-
auto sink = std::make_unique<EventSinkImplementation>(
96-
messenger, channel_name, codec);
97-
std::unique_ptr<StreamHandlerError<T>> error =
98-
shared_handler->OnListen(method_call->arguments(), std::move(sink));
99-
if (error) {
100-
result = codec->EncodeErrorEnvelope(
101-
error->error_code, error->error_message, error->error_details);
102-
} else {
103-
result = codec->EncodeSuccessEnvelope();
104-
}
105-
reply(result->data(), result->size());
106-
} else if (method.compare(kOnCancelMethod) == 0) {
107-
std::unique_ptr<std::vector<uint8_t>> result;
108-
if (is_listening_) {
109-
std::unique_ptr<StreamHandlerError<T>> error =
110-
shared_handler->OnCancel(method_call->arguments());
111-
if (error) {
112-
result = codec->EncodeErrorEnvelope(
113-
error->error_code, error->error_message, error->error_details);
85+
86+
const std::string& method = method_call->method_name();
87+
if (method.compare(kOnListenMethod) == 0) {
88+
if (is_listening) {
89+
std::unique_ptr<StreamHandlerError<T>> error =
90+
shared_handler->OnCancel(nullptr);
91+
if (error) {
92+
std::cerr << "Failed to cancel existing stream: "
93+
<< (error->error_code) << ", "
94+
<< (error->error_message) << ", "
95+
<< (error->error_details);
96+
}
97+
}
98+
is_listening = true;
99+
100+
std::unique_ptr<std::vector<uint8_t>> result;
101+
auto sink = std::make_unique<EventSinkImplementation>(
102+
messenger, channel_name, codec);
103+
std::unique_ptr<StreamHandlerError<T>> error =
104+
shared_handler->OnListen(method_call->arguments(),
105+
std::move(sink));
106+
if (error) {
107+
result = codec->EncodeErrorEnvelope(error->error_code,
108+
error->error_message,
109+
error->error_details);
110+
} else {
111+
result = codec->EncodeSuccessEnvelope();
112+
}
113+
reply(result->data(), result->size());
114+
} else if (method.compare(kOnCancelMethod) == 0) {
115+
std::unique_ptr<std::vector<uint8_t>> result;
116+
if (is_listening) {
117+
std::unique_ptr<StreamHandlerError<T>> error =
118+
shared_handler->OnCancel(method_call->arguments());
119+
if (error) {
120+
result = codec->EncodeErrorEnvelope(error->error_code,
121+
error->error_message,
122+
error->error_details);
123+
} else {
124+
result = codec->EncodeSuccessEnvelope();
125+
}
126+
is_listening = false;
127+
} else {
128+
result = codec->EncodeErrorEnvelope(
129+
"error", "No active stream to cancel", nullptr);
130+
}
131+
reply(result->data(), result->size());
114132
} else {
115-
result = codec->EncodeSuccessEnvelope();
133+
reply(nullptr, 0);
116134
}
117-
is_listening_ = false;
118-
} else {
119-
result = codec->EncodeErrorEnvelope(
120-
"error", "No active stream to cancel", nullptr);
121-
}
122-
reply(result->data(), result->size());
123-
} else {
124-
reply(nullptr, 0);
125-
}
126-
};
135+
};
127136
messenger_->SetMessageHandler(name_, std::move(binary_handler));
128137
}
129138

@@ -165,7 +174,6 @@ class EventChannel {
165174
BinaryMessenger* messenger_;
166175
const std::string name_;
167176
const MethodCodec<T>* codec_;
168-
bool is_listening_ = false;
169177
};
170178

171179
} // namespace flutter

0 commit comments

Comments
 (0)