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

Log FlutterJSONMessageCodec decode errors before asserting #50163

Merged
merged 1 commit into from
Jan 31, 2024
Merged

Log FlutterJSONMessageCodec decode errors before asserting #50163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions shell/platform/darwin/common/framework/Source/FlutterCodecs.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ - (id)decode:(NSData*)message {
}
BOOL isSimpleValue = NO;
id decoded = nil;
NSError* error;
if (0 < message.length) {
UInt8 first;
[message getBytes:&first length:1];
Expand All @@ -105,9 +106,9 @@ - (id)decode:(NSData*)message {
[expandedMessage replaceBytesInRange:NSMakeRange(message.length + 1, 1) withBytes:&end];
message = expandedMessage;
}
decoded = [NSJSONSerialization JSONObjectWithData:message options:0 error:nil];
decoded = [NSJSONSerialization JSONObjectWithData:message options:0 error:&error];
}
NSAssert(decoded, @"Invalid JSON message, decoding failed");
NSAssert(decoded, @"Invalid JSON message, decoding failed: %@", error);
Copy link
Member Author

@jmagman jmagman Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to what's already done in encode

engine/shell/platform/darwin/common/framework/Source/FlutterCodecs.mm

Line 82 in ed73d40

NSAssert(encoding, @"Invalid JSON message, encoding failed: %@", error);

return isSimpleValue ? ((NSArray*)decoded)[0] : decoded;
}
@end
Expand Down
15 changes: 15 additions & 0 deletions shell/platform/darwin/common/framework/Source/flutter_codecs_unittest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,26 @@
ASSERT_TRUE([value isEqualTo:decoded]);
}

TEST(FlutterJSONCodec, ThrowsOnInvalidEncode) {
NSString* value = [[NSString alloc] initWithBytes:"\xdf\xff"
length:2
encoding:NSUTF16StringEncoding];
FlutterJSONMessageCodec* codec = [FlutterJSONMessageCodec sharedInstance];
EXPECT_EXIT([codec encode:value], testing::KilledBySignal(SIGABRT), "failed to convert to UTF8");
}

TEST(FlutterJSONCodec, CanDecodeZeroLength) {
FlutterJSONMessageCodec* codec = [FlutterJSONMessageCodec sharedInstance];
ASSERT_TRUE([codec decode:[NSData data]] == nil);
}

TEST(FlutterJSONCodec, ThrowsOnInvalidDecode) {
NSString* value = @"{{{";
FlutterJSONMessageCodec* codec = [FlutterJSONMessageCodec sharedInstance];
EXPECT_EXIT([codec decode:[value dataUsingEncoding:value.fastestEncoding]],
testing::KilledBySignal(SIGABRT), "No string key for value in object around line 1");
}

TEST(FlutterJSONCodec, CanEncodeAndDecodeNil) {
FlutterJSONMessageCodec* codec = [FlutterJSONMessageCodec sharedInstance];
ASSERT_TRUE([codec encode:nil] == nil);
Expand Down
1 change: 1 addition & 0 deletions testing/run_all_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ int main(int argc, char** argv) {
#endif // FML_OS_IOS

::testing::InitGoogleTest(&argc, argv);
GTEST_FLAG_SET(death_test_style, "threadsafe");
Copy link
Member Author

@jmagman jmagman Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to add this flag to allow EXPECT_EXIT.
https://github.com/google/googletest/blob/main/docs/advanced.md#death-test-styles

Otherwise:

[WARNING] ../../third_party/googletest/googletest/src/gtest-death-test.cc:1102:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 2 threads. See https://github.com/google/googletest/blob/main/docs/advanced.md#death-tests-and-threads for more explanation and suggested solutions, especially if this is the last message you see before your test times out.
../../flutter/shell/platform/darwin/common/framework/Source/flutter_codecs_unittest.mm:52: Failure
Death test: [codec encode:value]
    Result: died but not with expected error.
  Expected: contains regular expression "failed to convert to UTF8"
Actual msg:
[  DEATH   ] 2024-01-29 18:38:36.256 framework_common_unittests[31544:1799550] *** Assertion failure in -[FlutterJSONMessageCodec encode:], FlutterCodecs.mm:82
[  DEATH   ] objc[31544]: +[NSNumber initialize] may have been in progress in another thread when fork() was called.
[  DEATH   ] objc[31544]: +[NSNumber initialize] may have been in progress in another thread when fork() was called. We cannot safely call it or ignore it in the fork() child process. Crashing instead. Set a breakpoint on objc_initializeAfterForkError to debug.


// Check if the user has specified a timeout.
const auto timeout = GetTestTimeout();
Expand Down