From 88df2a5398e51a2743b0b750f365b8faf4819438 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 25 Apr 2019 16:05:40 -0700 Subject: [PATCH 1/9] Fix crash when cursor ends up at invalid position --- ci/licenses_golden/licenses_flutter | 3 ++ fml/BUILD.gn | 39 ++++++++++--------- .../darwin/string_range_sanitization.h | 26 +++++++++++++ .../darwin/string_range_sanitization.mm | 23 +++++++++++ .../string_range_sanitization_unittests.mm | 18 +++++++++ .../Source/FlutterTextInputPlugin.mm | 24 ++++++------ .../flutter_text_input_plugin_unittest.mm | 14 +++++++ 7 files changed, 117 insertions(+), 30 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 create mode 100644 shell/platform/darwin/ios/framework/Source/flutter_text_input_plugin_unittest.mm diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 521cf0e238fd0..7601fcc9ae26b 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_unittests.mm +FILE: ../../../flutter/fml/platform/darwin/string_range_sanitization.h +FILE: ../../../flutter/fml/platform/darwin/string_range_sanitization.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 614f1b1d5decb..4f4eb63de50ce 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" ] @@ -180,24 +182,25 @@ executable("fml_unittests") { testonly = true sources = [ - "base32_unittest.cc", - "command_line_unittest.cc", - "file_unittest.cc", - "memory/ref_counted_unittest.cc", - "memory/weak_ptr_unittest.cc", - "message_loop_unittests.cc", - "message_unittests.cc", - "paths_unittests.cc", - "string_view_unittest.cc", - "synchronization/count_down_latch_unittests.cc", - "synchronization/semaphore_unittest.cc", - "synchronization/thread_annotations_unittest.cc", - "synchronization/waitable_event_unittest.cc", - "thread_local_unittests.cc", - "thread_unittests.cc", - "time/time_delta_unittest.cc", - "time/time_point_unittest.cc", - "time/time_unittest.cc", + # "base32_unittest.cc", + # "command_line_unittest.cc", + # "file_unittest.cc", + # "memory/ref_counted_unittest.cc", + # "memory/weak_ptr_unittest.cc", + # "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", + # "synchronization/thread_annotations_unittest.cc", + # "synchronization/waitable_event_unittest.cc", + # "thread_local_unittests.cc", + # "thread_unittests.cc", + # "time/time_delta_unittest.cc", + # "time/time_point_unittest.cc", + # "time/time_unittest.cc", ] deps = [ diff --git a/fml/platform/darwin/string_range_sanitization.h b/fml/platform/darwin/string_range_sanitization.h new file mode 100644 index 0000000000000..b01b249f50d2b --- /dev/null +++ b/fml/platform/darwin/string_range_sanitization.h @@ -0,0 +1,26 @@ +// 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 { + +// Helper to get the correct boundary of a character position in an NSString +// given a byte index. +NSRange rangeForCharacterAtIndex(NSString* text, NSUInteger index); + +// Helper to get the correct boundaries around one or more character positions +// in an NSString given an NSRange. +// +// 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. +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 0000000000000..64b1b4f6213bb --- /dev/null +++ b/fml/platform/darwin/string_range_sanitization.mm @@ -0,0 +1,23 @@ +// 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 (index < text.length) + return [text rangeOfComposedCharacterSequenceAtIndex:index]; + return NSMakeRange(index, 0); +} + +NSRange rangeForCharactersInRange(NSString* text, NSRange range) { + 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 \ No newline at end of file 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 0000000000000..b2311446439d5 --- /dev/null +++ b/fml/platform/darwin/string_range_sanitization_unittests.mm @@ -0,0 +1,18 @@ +// 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" +#include "gtest/gtest.h" + +TEST(StringRangeSanitizationTest, CanHandleUnicode) { + auto result = fml::rangeForCharacterAtIndex(@"😠", 1); + EXPECT_EQ(result.location, 0UL); + EXPECT_EQ(result.length, 2UL); +} + +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 dd8971f402e58..6717dbc51409d 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,14 @@ - (void)setSelectedTextRange:(UITextRange*)selectedTextRange { - (void)setSelectedTextRange:(UITextRange*)selectedTextRange updateEditingState:(BOOL)update { if (_selectedTextRange != selectedTextRange) { UITextRange* oldSelectedRange = _selectedTextRange; - _selectedTextRange = [selectedTextRange copy]; + if (self.hasText) { + FlutterTextRange* flutterTextRange = (FlutterTextRange*)selectedTextRange; + NSLog(@"%lu %lu %lu %lu", flutterTextRange.range.location, flutterTextRange.range.length, fml::rangeForCharactersInRange(self.text, flutterTextRange.range).location, fml::rangeForCharactersInRange(self.text, flutterTextRange.range).length); + _selectedTextRange = [[FlutterTextRange + rangeWithNSRange:fml::rangeForCharactersInRange(self.text, flutterTextRange.range)] copy]; + } else { + _selectedTextRange = [selectedTextRange copy]; + } [oldSelectedRange release]; if (update) @@ -415,20 +423,12 @@ - (UITextRange*)textRangeFromPosition:(UITextPosition*)fromPosition 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 +565,7 @@ - (UITextPosition*)closestPositionToPoint:(CGPoint)point withinRange:(UITextRang - (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 { diff --git a/shell/platform/darwin/ios/framework/Source/flutter_text_input_plugin_unittest.mm b/shell/platform/darwin/ios/framework/Source/flutter_text_input_plugin_unittest.mm new file mode 100644 index 0000000000000..304b7ad1cb003 --- /dev/null +++ b/shell/platform/darwin/ios/framework/Source/flutter_text_input_plugin_unittest.mm @@ -0,0 +1,14 @@ +// 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/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm" +#include "gtest/gtest.h" + +TEST(FlutterTextInputView, SanitizesSelectedTextRange) { + FlutterTextInputView* view = [[FlutterTextInputView alloc] init]; + [view insertText:@"😠"]; + [view setSelectedTextRange:[FlutterTextRange rangeWithNSRange:NSMakeRange(1, 0)]]; + FlutterTextRange* selectedRange = (FlutterTextRange*)[view selectedTextRange]; + EXPECT_EQ(selectedRange.range.location, 2); +} From 1c81df5f9a0344cecfa92dbd8757d04af8abc6b7 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 25 Apr 2019 16:14:37 -0700 Subject: [PATCH 2/9] remove unintended commenting out --- fml/BUILD.gn | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 4f4eb63de50ce..cb53afa12da56 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -182,25 +182,25 @@ executable("fml_unittests") { testonly = true sources = [ - # "base32_unittest.cc", - # "command_line_unittest.cc", - # "file_unittest.cc", - # "memory/ref_counted_unittest.cc", - # "memory/weak_ptr_unittest.cc", - # "message_loop_unittests.cc", - # "message_unittests.cc", - # "paths_unittests.cc", + "base32_unittest.cc", + "command_line_unittest.cc", + "file_unittest.cc", + "memory/ref_counted_unittest.cc", + "memory/weak_ptr_unittest.cc", + "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", - # "synchronization/thread_annotations_unittest.cc", - # "synchronization/waitable_event_unittest.cc", - # "thread_local_unittests.cc", - # "thread_unittests.cc", - # "time/time_delta_unittest.cc", - # "time/time_point_unittest.cc", - # "time/time_unittest.cc", + "string_view_unittest.cc", + "synchronization/count_down_latch_unittests.cc", + "synchronization/semaphore_unittest.cc", + "synchronization/thread_annotations_unittest.cc", + "synchronization/waitable_event_unittest.cc", + "thread_local_unittests.cc", + "thread_unittests.cc", + "time/time_delta_unittest.cc", + "time/time_point_unittest.cc", + "time/time_unittest.cc", ] deps = [ From 8f75d25f2077128b3ee6475f475b698a981b144b Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 25 Apr 2019 16:15:08 -0700 Subject: [PATCH 3/9] remove debug logging --- .../darwin/ios/framework/Source/FlutterTextInputPlugin.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm index 6717dbc51409d..e2f6e17b2899a 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm @@ -286,7 +286,6 @@ - (void)setSelectedTextRange:(UITextRange*)selectedTextRange updateEditingState: UITextRange* oldSelectedRange = _selectedTextRange; if (self.hasText) { FlutterTextRange* flutterTextRange = (FlutterTextRange*)selectedTextRange; - NSLog(@"%lu %lu %lu %lu", flutterTextRange.range.location, flutterTextRange.range.length, fml::rangeForCharactersInRange(self.text, flutterTextRange.range).location, fml::rangeForCharactersInRange(self.text, flutterTextRange.range).length); _selectedTextRange = [[FlutterTextRange rangeWithNSRange:fml::rangeForCharactersInRange(self.text, flutterTextRange.range)] copy]; } else { From caf2f2885d8e9db1ed9e0cece4bb71624f02f430 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 25 Apr 2019 16:21:09 -0700 Subject: [PATCH 4/9] casing --- fml/platform/darwin/string_range_sanitization.h | 4 ++-- fml/platform/darwin/string_range_sanitization.mm | 4 ++-- .../darwin/string_range_sanitization_unittests.mm | 4 ++-- .../darwin/ios/framework/Source/FlutterTextInputPlugin.mm | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/fml/platform/darwin/string_range_sanitization.h b/fml/platform/darwin/string_range_sanitization.h index b01b249f50d2b..9191b7eab8e5f 100644 --- a/fml/platform/darwin/string_range_sanitization.h +++ b/fml/platform/darwin/string_range_sanitization.h @@ -11,7 +11,7 @@ namespace fml { // Helper to get the correct boundary of a character position in an NSString // given a byte index. -NSRange rangeForCharacterAtIndex(NSString* text, NSUInteger index); +NSRange RangeForCharacterAtIndex(NSString* text, NSUInteger index); // Helper to get the correct boundaries around one or more character positions // in an NSString given an NSRange. @@ -19,7 +19,7 @@ NSRange rangeForCharacterAtIndex(NSString* text, NSUInteger index); // 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. -NSRange rangeForCharactersInRange(NSString* text, NSRange range); +NSRange RangeForCharactersInRange(NSString* text, NSRange range); } // namespace fml diff --git a/fml/platform/darwin/string_range_sanitization.mm b/fml/platform/darwin/string_range_sanitization.mm index 64b1b4f6213bb..6c42860439959 100644 --- a/fml/platform/darwin/string_range_sanitization.mm +++ b/fml/platform/darwin/string_range_sanitization.mm @@ -6,13 +6,13 @@ namespace fml { -NSRange rangeForCharacterAtIndex(NSString* text, NSUInteger index) { +NSRange RangeForCharacterAtIndex(NSString* text, NSUInteger index) { if (index < text.length) return [text rangeOfComposedCharacterSequenceAtIndex:index]; return NSMakeRange(index, 0); } -NSRange rangeForCharactersInRange(NSString* text, NSRange range) { +NSRange RangeForCharactersInRange(NSString* text, NSRange range) { 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 diff --git a/fml/platform/darwin/string_range_sanitization_unittests.mm b/fml/platform/darwin/string_range_sanitization_unittests.mm index b2311446439d5..90a83bfd1fc89 100644 --- a/fml/platform/darwin/string_range_sanitization_unittests.mm +++ b/fml/platform/darwin/string_range_sanitization_unittests.mm @@ -6,13 +6,13 @@ #include "gtest/gtest.h" TEST(StringRangeSanitizationTest, CanHandleUnicode) { - auto result = fml::rangeForCharacterAtIndex(@"😠", 1); + auto result = fml::RangeForCharacterAtIndex(@"😠", 1); EXPECT_EQ(result.location, 0UL); EXPECT_EQ(result.length, 2UL); } TEST(StringRangeSanitizationTest, CanHandleUnicodeRange) { - auto result = fml::rangeForCharactersInRange(@"😠", NSMakeRange(1, 0)); + 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 e2f6e17b2899a..f21ff26864c7b 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm @@ -287,7 +287,7 @@ - (void)setSelectedTextRange:(UITextRange*)selectedTextRange updateEditingState: if (self.hasText) { FlutterTextRange* flutterTextRange = (FlutterTextRange*)selectedTextRange; _selectedTextRange = [[FlutterTextRange - rangeWithNSRange:fml::rangeForCharactersInRange(self.text, flutterTextRange.range)] copy]; + rangeWithNSRange:fml::RangeForCharactersInRange(self.text, flutterTextRange.range)] copy]; } else { _selectedTextRange = [selectedTextRange copy]; } @@ -423,11 +423,11 @@ - (UITextRange*)textRangeFromPosition:(UITextPosition*)fromPosition } - (NSUInteger)decrementOffsetPosition:(NSUInteger)position { - return fml::rangeForCharacterAtIndex(self.text, MAX(0, position - 1)).location; + return fml::RangeForCharacterAtIndex(self.text, MAX(0, position - 1)).location; } - (NSUInteger)incrementOffsetPosition:(NSUInteger)position { - NSRange charRange = fml::rangeForCharacterAtIndex(self.text, position); + NSRange charRange = fml::RangeForCharacterAtIndex(self.text, position); return MIN(position + charRange.length, self.text.length); } @@ -564,7 +564,7 @@ - (UITextPosition*)closestPositionToPoint:(CGPoint)point withinRange:(UITextRang - (UITextRange*)characterRangeAtPoint:(CGPoint)point { // TODO(cbracken) Implement. NSUInteger currentIndex = ((FlutterTextPosition*)_selectedTextRange.start).index; - return [FlutterTextRange rangeWithNSRange:fml::rangeForCharacterAtIndex(self.text, currentIndex)]; + return [FlutterTextRange rangeWithNSRange:fml::RangeForCharacterAtIndex(self.text, currentIndex)]; } - (void)beginFloatingCursorAtPoint:(CGPoint)point { From b7758bcbfc11ed77f178e95acb0bc1261eb81880 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 25 Apr 2019 16:22:16 -0700 Subject: [PATCH 5/9] format --- fml/platform/darwin/string_range_sanitization.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fml/platform/darwin/string_range_sanitization.mm b/fml/platform/darwin/string_range_sanitization.mm index 6c42860439959..a08c07fa9aabc 100644 --- a/fml/platform/darwin/string_range_sanitization.mm +++ b/fml/platform/darwin/string_range_sanitization.mm @@ -20,4 +20,4 @@ NSRange RangeForCharactersInRange(NSString* text, NSRange range) { return NSMakeRange(sanitizedRange.location, range.length); } -} // namespace fml \ No newline at end of file +} // namespace fml From d6c6e6f89ea2c4bd6b67fb0ca4f4108b5b393590 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 25 Apr 2019 16:53:11 -0700 Subject: [PATCH 6/9] improve tests and error handling --- ci/licenses_golden/licenses_flutter | 2 +- fml/platform/darwin/string_range_sanitization.h | 10 ++++++---- fml/platform/darwin/string_range_sanitization.mm | 6 ++++++ .../darwin/string_range_sanitization_unittests.mm | 15 +++++++++++++++ .../Source/flutter_text_input_plugin_unittest.mm | 14 -------------- 5 files changed, 28 insertions(+), 19 deletions(-) delete mode 100644 shell/platform/darwin/ios/framework/Source/flutter_text_input_plugin_unittest.mm diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 7601fcc9ae26b..495476655d7ff 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -190,9 +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_unittests.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/platform/darwin/string_range_sanitization.h b/fml/platform/darwin/string_range_sanitization.h index 9191b7eab8e5f..1df9ba681fb33 100644 --- a/fml/platform/darwin/string_range_sanitization.h +++ b/fml/platform/darwin/string_range_sanitization.h @@ -9,16 +9,18 @@ namespace fml { -// Helper to get the correct boundary of a character position in an NSString -// given a byte index. +// Returns a range encompassing the grapheme cluster in which |index| is located. +// +// An invalid index will result in `NSRange(NSNotFound, 0)`. NSRange RangeForCharacterAtIndex(NSString* text, NSUInteger index); -// Helper to get the correct boundaries around one or more character positions -// in an NSString given an NSRange. +// 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 diff --git a/fml/platform/darwin/string_range_sanitization.mm b/fml/platform/darwin/string_range_sanitization.mm index a08c07fa9aabc..8100ba9154bf7 100644 --- a/fml/platform/darwin/string_range_sanitization.mm +++ b/fml/platform/darwin/string_range_sanitization.mm @@ -7,12 +7,18 @@ 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 diff --git a/fml/platform/darwin/string_range_sanitization_unittests.mm b/fml/platform/darwin/string_range_sanitization_unittests.mm index 90a83bfd1fc89..c9b8c7b4fe159 100644 --- a/fml/platform/darwin/string_range_sanitization_unittests.mm +++ b/fml/platform/darwin/string_range_sanitization_unittests.mm @@ -2,6 +2,8 @@ // 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" @@ -11,6 +13,19 @@ 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); diff --git a/shell/platform/darwin/ios/framework/Source/flutter_text_input_plugin_unittest.mm b/shell/platform/darwin/ios/framework/Source/flutter_text_input_plugin_unittest.mm deleted file mode 100644 index 304b7ad1cb003..0000000000000 --- a/shell/platform/darwin/ios/framework/Source/flutter_text_input_plugin_unittest.mm +++ /dev/null @@ -1,14 +0,0 @@ -// 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/shell/platform/darwin/ios/framework/Source/FlutterTextInputPlugin.mm" -#include "gtest/gtest.h" - -TEST(FlutterTextInputView, SanitizesSelectedTextRange) { - FlutterTextInputView* view = [[FlutterTextInputView alloc] init]; - [view insertText:@"😠"]; - [view setSelectedTextRange:[FlutterTextRange rangeWithNSRange:NSMakeRange(1, 0)]]; - FlutterTextRange* selectedRange = (FlutterTextRange*)[view selectedTextRange]; - EXPECT_EQ(selectedRange.range.location, 2); -} From d9ef2d22bf2a065f7a3265b651d32e048ffd7a71 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 25 Apr 2019 20:08:22 -0700 Subject: [PATCH 7/9] Format again --- fml/platform/darwin/string_range_sanitization_unittests.mm | 2 -- 1 file changed, 2 deletions(-) diff --git a/fml/platform/darwin/string_range_sanitization_unittests.mm b/fml/platform/darwin/string_range_sanitization_unittests.mm index c9b8c7b4fe159..2461601a4f781 100644 --- a/fml/platform/darwin/string_range_sanitization_unittests.mm +++ b/fml/platform/darwin/string_range_sanitization_unittests.mm @@ -19,13 +19,11 @@ 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); From 2e30be742039fe967e1366cac79aa7a2fe43bf16 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 25 Apr 2019 23:19:38 -0700 Subject: [PATCH 8/9] Format --- fml/platform/darwin/string_range_sanitization_unittests.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/fml/platform/darwin/string_range_sanitization_unittests.mm b/fml/platform/darwin/string_range_sanitization_unittests.mm index 2461601a4f781..22d3cc537c9b8 100644 --- a/fml/platform/darwin/string_range_sanitization_unittests.mm +++ b/fml/platform/darwin/string_range_sanitization_unittests.mm @@ -18,7 +18,6 @@ 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); From 15d934d24038f541bd82ef07be97616a3751bf86 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Fri, 26 Apr 2019 14:48:45 -0700 Subject: [PATCH 9/9] better check --- fml/platform/darwin/string_range_sanitization.h | 3 ++- fml/platform/darwin/string_range_sanitization.mm | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/fml/platform/darwin/string_range_sanitization.h b/fml/platform/darwin/string_range_sanitization.h index 1df9ba681fb33..95c28b76b6380 100644 --- a/fml/platform/darwin/string_range_sanitization.h +++ b/fml/platform/darwin/string_range_sanitization.h @@ -11,7 +11,8 @@ namespace fml { // Returns a range encompassing the grapheme cluster in which |index| is located. // -// An invalid index will result in `NSRange(NSNotFound, 0)`. +// 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|. diff --git a/fml/platform/darwin/string_range_sanitization.mm b/fml/platform/darwin/string_range_sanitization.mm index 8100ba9154bf7..2694fdc568524 100644 --- a/fml/platform/darwin/string_range_sanitization.mm +++ b/fml/platform/darwin/string_range_sanitization.mm @@ -7,7 +7,7 @@ namespace fml { NSRange RangeForCharacterAtIndex(NSString* text, NSUInteger index) { - if (text == nil || index > text.length) { + if (text == nil || index >= text.length) { return NSMakeRange(NSNotFound, 0); } if (index < text.length)