From d8b736ec945c1470278a74cbca45de97ddc1d135 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 24 Jan 2024 11:18:07 -0800 Subject: [PATCH] [macOS] Fix: Memory sanitizer violated when encoding indirect strings (flutter/engine#49995) Fixes https://github.com/flutter/flutter/issues/142101 @cbracken However, this unit test requires the unit tests to be compiled with `--asan` to work. Can we add this flag to CI? ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I signed the [CLA]. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- .../Source/FlutterStandardCodecHelper.cc | 2 +- .../Source/flutter_standard_codec_unittest.mm | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterStandardCodecHelper.cc b/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterStandardCodecHelper.cc index 38b9127f41f..ef872f249bd 100644 --- a/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterStandardCodecHelper.cc +++ b/engine/src/flutter/shell/platform/darwin/common/framework/Source/FlutterStandardCodecHelper.cc @@ -221,7 +221,7 @@ void FlutterStandardCodecHelperWriteUTF8(CFMutableDataRef data, // UTF16 length times 3 will fit all UTF8. CFIndex buffer_length = length * 3; std::vector buffer; - buffer.reserve(buffer_length); + buffer.resize(buffer_length); CFStringGetBytes(value, CFRangeMake(0, length), kCFStringEncodingUTF8, 0, false, buffer.data(), buffer_length, &used_length); FlutterStandardCodecHelperWriteSize(data, used_length); diff --git a/engine/src/flutter/shell/platform/darwin/common/framework/Source/flutter_standard_codec_unittest.mm b/engine/src/flutter/shell/platform/darwin/common/framework/Source/flutter_standard_codec_unittest.mm index f4ce192e800..3489cec7c00 100644 --- a/engine/src/flutter/shell/platform/darwin/common/framework/Source/flutter_standard_codec_unittest.mm +++ b/engine/src/flutter/shell/platform/darwin/common/framework/Source/flutter_standard_codec_unittest.mm @@ -4,6 +4,8 @@ #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterCodecs.h" +#include + #include "gtest/gtest.h" FLUTTER_ASSERT_ARC @@ -215,6 +217,19 @@ TEST(FlutterStandardCodec, CanEncodeAndDecodeStringWithNonBMPCodePoint) { CheckEncodeDecode(@"h\U0001F602w", [NSData dataWithBytes:bytes length:8]); } +TEST(FlutterStandardCodec, CanEncodeAndDecodeIndirectString) { + // This test ensures that an indirect NSString, whose internal string buffer + // can't be simply returned by `CFStringGetCStringPtr`, can be encoded without + // violating the memory sanitizer. This test only works with `--asan` flag. + // See https://github.com/flutter/flutter/issues/142101 + uint8_t bytes[7] = {0x07, 0x05, 0x68, 0xe2, 0x98, 0xba, 0x77}; + NSString* target = @"h\u263Aw"; + // Ensures that this is an indirect string so that this test makes sense. + ASSERT_TRUE(CFStringGetCStringPtr((__bridge CFStringRef)target, kCFStringEncodingUTF8) == + nullptr); + CheckEncodeDecode(target, [NSData dataWithBytes:bytes length:7]); +} + TEST(FlutterStandardCodec, CanEncodeAndDecodeArray) { NSArray* value = @[ [NSNull null], @"hello", @3.14, @47, @{@42 : @"nested"} ]; CheckEncodeDecode(value);