From 57bf1c09688869c9aed0ebdf13eef20437b30e5b Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 26 Apr 2019 15:43:44 -0700 Subject: [PATCH] Fix crash when cursor ends up at invalid position (#8747) --- ci/licenses_golden/licenses_flutter | 3 ++ fml/BUILD.gn | 3 ++ .../darwin/string_range_sanitization.h | 29 ++++++++++++++++++ .../darwin/string_range_sanitization.mm | 29 ++++++++++++++++++ .../string_range_sanitization_unittests.mm | 30 +++++++++++++++++++ .../Source/FlutterTextInputPlugin.mm | 23 +++++++------- 6 files changed, 105 insertions(+), 12 deletions(-) create mode 100644 fml/platform/darwin/string_range_sanitization.h create mode 100644 fml/platform/darwin/string_range_sanitization.mm create mode 100644 fml/platform/darwin/string_range_sanitization_unittests.mm diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 521cf0e238f..495476655d7 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -190,6 +190,9 @@ FILE: ../../../flutter/fml/platform/darwin/scoped_block.h FILE: ../../../flutter/fml/platform/darwin/scoped_block.mm FILE: ../../../flutter/fml/platform/darwin/scoped_nsobject.h FILE: ../../../flutter/fml/platform/darwin/scoped_nsobject.mm +FILE: ../../../flutter/fml/platform/darwin/string_range_sanitization.h +FILE: ../../../flutter/fml/platform/darwin/string_range_sanitization.mm +FILE: ../../../flutter/fml/platform/darwin/string_range_sanitization_unittests.mm FILE: ../../../flutter/fml/platform/fuchsia/paths_fuchsia.cc FILE: ../../../flutter/fml/platform/linux/message_loop_linux.cc FILE: ../../../flutter/fml/platform/linux/message_loop_linux.h diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 614f1b1d5de..cb53afa12da 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -106,6 +106,8 @@ source_set("fml") { "platform/darwin/scoped_block.mm", "platform/darwin/scoped_nsobject.h", "platform/darwin/scoped_nsobject.mm", + "platform/darwin/string_range_sanitization.h", + "platform/darwin/string_range_sanitization.mm", ] libs += [ "Foundation.framework" ] @@ -188,6 +190,7 @@ executable("fml_unittests") { "message_loop_unittests.cc", "message_unittests.cc", "paths_unittests.cc", + "platform/darwin/string_range_sanitization_unittests.mm", "string_view_unittest.cc", "synchronization/count_down_latch_unittests.cc", "synchronization/semaphore_unittest.cc", diff --git a/fml/platform/darwin/string_range_sanitization.h b/fml/platform/darwin/string_range_sanitization.h new file mode 100644 index 00000000000..95c28b76b63 --- /dev/null +++ b/fml/platform/darwin/string_range_sanitization.h @@ -0,0 +1,29 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_FML_STRING_RANGE_SANITIZATION_H_ +#define FLUTTER_FML_STRING_RANGE_SANITIZATION_H_ + +#include + +namespace fml { + +// Returns a range encompassing the grapheme cluster in which |index| is located. +// +// A nil |text| or an index greater than or equal to text.length will result in +// `NSRange(NSNotFound, 0)`. +NSRange RangeForCharacterAtIndex(NSString* text, NSUInteger index); + +// Returns a range encompassing the grapheme clusters falling in |range|. +// +// This method will not alter the length of the input range, but will ensure +// that the range's location is not in the middle of a multi-byte unicode +// sequence. +// +// An invalid range will result in `NSRange(NSNotFound, 0)`. +NSRange RangeForCharactersInRange(NSString* text, NSRange range); + +} // namespace fml + +#endif // FLUTTER_FML_STRING_RANGE_SANITIZATION_H_ diff --git a/fml/platform/darwin/string_range_sanitization.mm b/fml/platform/darwin/string_range_sanitization.mm new file mode 100644 index 00000000000..2694fdc5685 --- /dev/null +++ b/fml/platform/darwin/string_range_sanitization.mm @@ -0,0 +1,29 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/fml/platform/darwin/string_range_sanitization.h" + +namespace fml { + +NSRange RangeForCharacterAtIndex(NSString* text, NSUInteger index) { + if (text == nil || index >= text.length) { + return NSMakeRange(NSNotFound, 0); + } + if (index < text.length) + return [text rangeOfComposedCharacterSequenceAtIndex:index]; + return NSMakeRange(index, 0); +} + +NSRange RangeForCharactersInRange(NSString* text, NSRange range) { + if (text == nil || range.location + range.length > text.length) { + return NSMakeRange(NSNotFound, 0); + } + NSRange sanitizedRange = [text rangeOfComposedCharacterSequencesForRange:range]; + // We don't want to override the length, we just want to make sure we don't + // select into the middle of a multi-byte character. Taking the + // `sanitizedRange`'s length will end up altering the actual selection. + return NSMakeRange(sanitizedRange.location, range.length); +} + +} // namespace fml diff --git a/fml/platform/darwin/string_range_sanitization_unittests.mm b/fml/platform/darwin/string_range_sanitization_unittests.mm new file mode 100644 index 00000000000..22d3cc537c9 --- /dev/null +++ b/fml/platform/darwin/string_range_sanitization_unittests.mm @@ -0,0 +1,30 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "flutter/fml/platform/darwin/string_range_sanitization.h" +#include "gtest/gtest.h" + +TEST(StringRangeSanitizationTest, CanHandleUnicode) { + auto result = fml::RangeForCharacterAtIndex(@"😠", 1); + EXPECT_EQ(result.location, 0UL); + EXPECT_EQ(result.length, 2UL); +} + +TEST(StringRangeSanitizationTest, HandlesInvalidRanges) { + auto ns_not_found = static_cast(NSNotFound); + EXPECT_EQ(fml::RangeForCharacterAtIndex(@"😠", 3).location, ns_not_found); + EXPECT_EQ(fml::RangeForCharacterAtIndex(@"😠", -1).location, ns_not_found); + EXPECT_EQ(fml::RangeForCharacterAtIndex(nil, 0).location, ns_not_found); + EXPECT_EQ(fml::RangeForCharactersInRange(@"😠", NSMakeRange(1, 2)).location, ns_not_found); + EXPECT_EQ(fml::RangeForCharactersInRange(@"😠", NSMakeRange(3, 0)).location, ns_not_found); + EXPECT_EQ(fml::RangeForCharactersInRange(nil, NSMakeRange(0, 0)).location, ns_not_found); +} + +TEST(StringRangeSanitizationTest, CanHandleUnicodeRange) { + auto result = fml::RangeForCharactersInRange(@"😠", NSMakeRange(1, 0)); + EXPECT_EQ(result.location, 0UL); + EXPECT_EQ(result.length, 0UL); +} diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm index dd8971f402e..f21ff26864c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.h" +#include "flutter/fml/platform/darwin/string_range_sanitization.h" #include #include @@ -283,7 +284,13 @@ static UIReturnKeyType ToUIReturnKeyType(NSString* inputType) { - (void)setSelectedTextRange:(UITextRange*)selectedTextRange updateEditingState:(BOOL)update { if (_selectedTextRange != selectedTextRange) { UITextRange* oldSelectedRange = _selectedTextRange; - _selectedTextRange = [selectedTextRange copy]; + if (self.hasText) { + FlutterTextRange* flutterTextRange = (FlutterTextRange*)selectedTextRange; + _selectedTextRange = [[FlutterTextRange + rangeWithNSRange:fml::RangeForCharactersInRange(self.text, flutterTextRange.range)] copy]; + } else { + _selectedTextRange = [selectedTextRange copy]; + } [oldSelectedRange release]; if (update) @@ -415,20 +422,12 @@ static UIReturnKeyType ToUIReturnKeyType(NSString* inputType) { return [FlutterTextRange rangeWithNSRange:NSMakeRange(fromIndex, toIndex - fromIndex)]; } -/** Returns the range of the character sequence at the specified index in the - * text. */ -- (NSRange)rangeForCharacterAtIndex:(NSUInteger)index { - if (index < self.text.length) - return [self.text rangeOfComposedCharacterSequenceAtIndex:index]; - return NSMakeRange(index, 0); -} - - (NSUInteger)decrementOffsetPosition:(NSUInteger)position { - return [self rangeForCharacterAtIndex:MAX(0, position - 1)].location; + return fml::RangeForCharacterAtIndex(self.text, MAX(0, position - 1)).location; } - (NSUInteger)incrementOffsetPosition:(NSUInteger)position { - NSRange charRange = [self rangeForCharacterAtIndex:position]; + NSRange charRange = fml::RangeForCharacterAtIndex(self.text, position); return MIN(position + charRange.length, self.text.length); } @@ -565,7 +564,7 @@ static UIReturnKeyType ToUIReturnKeyType(NSString* inputType) { - (UITextRange*)characterRangeAtPoint:(CGPoint)point { // TODO(cbracken) Implement. NSUInteger currentIndex = ((FlutterTextPosition*)_selectedTextRange.start).index; - return [FlutterTextRange rangeWithNSRange:[self rangeForCharacterAtIndex:currentIndex]]; + return [FlutterTextRange rangeWithNSRange:fml::RangeForCharacterAtIndex(self.text, currentIndex)]; } - (void)beginFloatingCursorAtPoint:(CGPoint)point {