TextInput: Verify TextRange and make method call fail loudly (#104711)

* Fix

* Tests

* Format

* Empty line

* Fix range test

* Fix

* Comments

* trailing spaces

* Fix tests

* Comments

Author: dkwingsmt

packages/flutter/lib/src/services/text_input.dart

@@ -751,7 +751,8 @@ class RawFloatingCursorPoint {
 class TextEditingValue {
   /// Creates information for editing a run of text.
   ///
-  /// The selection and composing range must be within the text.
+  /// The selection and composing range must be within the text. This is not
+  /// checked during construction, and must be guaranteed by the caller.
   ///
   /// The [text], [selection], and [composing] arguments must not be null but
   /// each have default values.
@@ -763,23 +764,32 @@ class TextEditingValue {
     this.selection = const TextSelection.collapsed(offset: -1),
     this.composing = TextRange.empty,
   }) : assert(text != null),
+       // The constructor does not verify that `selection` and `composing` are
+       // valid ranges within `text`, and is unable to do so due to limitation
+       // of const constructors. Some checks are performed by assertion in
+       // other occasions. See `_textRangeIsValid`.
        assert(selection != null),
        assert(composing != null);
 
   /// Creates an instance of this class from a JSON object.
   factory TextEditingValue.fromJSON(Map<String, dynamic> encoded) {
+    final String text = encoded['text'] as String;
+    final TextSelection selection = TextSelection(
+      baseOffset: encoded['selectionBase'] as int? ?? -1,
+      extentOffset: encoded['selectionExtent'] as int? ?? -1,
+      affinity: _toTextAffinity(encoded['selectionAffinity'] as String?) ?? TextAffinity.downstream,
+      isDirectional: encoded['selectionIsDirectional'] as bool? ?? false,
+    );
+    final TextRange composing = TextRange(
+      start: encoded['composingBase'] as int? ?? -1,
+      end: encoded['composingExtent'] as int? ?? -1,
+    );
+    assert(_textRangeIsValid(selection, text));
+    assert(_textRangeIsValid(composing, text));
     return TextEditingValue(
-      text: encoded['text'] as String,
-      selection: TextSelection(
-        baseOffset: encoded['selectionBase'] as int? ?? -1,
-        extentOffset: encoded['selectionExtent'] as int? ?? -1,
-        affinity: _toTextAffinity(encoded['selectionAffinity'] as String?) ?? TextAffinity.downstream,
-        isDirectional: encoded['selectionIsDirectional'] as bool? ?? false,
-      ),
-      composing: TextRange(
-        start: encoded['composingBase'] as int? ?? -1,
-        end: encoded['composingExtent'] as int? ?? -1,
-      ),
+      text: text,
+      selection: selection,
+      composing: composing,
     );
   }
 
@@ -884,21 +894,27 @@ class TextEditingValue {
       return originalIndex + replacedLength - removedLength;
     }
 
+    final TextSelection adjustedSelection = TextSelection(
+      baseOffset: adjustIndex(selection.baseOffset),
+      extentOffset: adjustIndex(selection.extentOffset),
+    );
+    final TextRange adjustedComposing = TextRange(
+      start: adjustIndex(composing.start),
+      end: adjustIndex(composing.end),
+    );
+    assert(_textRangeIsValid(adjustedSelection, newText));
+    assert(_textRangeIsValid(adjustedComposing, newText));
     return TextEditingValue(
       text: newText,
-      selection: TextSelection(
-        baseOffset: adjustIndex(selection.baseOffset),
-        extentOffset: adjustIndex(selection.extentOffset),
-      ),
-      composing: TextRange(
-        start: adjustIndex(composing.start),
-        end: adjustIndex(composing.end),
-      ),
+      selection: adjustedSelection,
+      composing: adjustedComposing,
     );
   }
 
   /// Returns a representation of this object as a JSON object.
   Map<String, dynamic> toJSON() {
+    assert(_textRangeIsValid(selection, text));
+    assert(_textRangeIsValid(composing, text));
     return <String, dynamic>{
       'text': text,
       'selectionBase': selection.baseOffset,
@@ -930,6 +946,24 @@ class TextEditingValue {
     selection.hashCode,
     composing.hashCode,
   );
+
+  // Verify that the given range is within the text.
+  //
+  // The verification can't be perform during the constructor of
+  // [TextEditingValue], which are `const` and are allowed to retrieve
+  // properties of [TextRange]s. [TextEditingValue] should perform this
+  // wherever it is building other values (such as toJson) or is built in a
+  // non-const way (such as fromJson).
+  static bool _textRangeIsValid(TextRange range, String text) {
+    if (range.start == -1 && range.end == -1) {
+      return true;
+    }
+    assert(range.start >= 0 && range.start <= text.length,
+        'Range start ${range.start} is out of text of length ${text.length}');
+    assert(range.end >= 0 && range.end <= text.length,
+        'Range end ${range.end} is out of text of length ${text.length}');
+    return true;
+  }
 }
 
 /// Indicates what triggered the change in selected text (including changes to
@@ -1440,7 +1474,7 @@ TextInputAction _toTextInputAction(String action) {
       return TextInputAction.next;
     case 'TextInputAction.previous':
       return TextInputAction.previous;
-    case 'TextInputAction.continue_action':
+    case 'TextInputAction.continueAction':
       return TextInputAction.continueAction;
     case 'TextInputAction.join':
       return TextInputAction.join;
@@ -1534,7 +1568,7 @@ RawFloatingCursorPoint _toTextPoint(FloatingCursorDragState state, Map<String, d
 class TextInput {
   TextInput._() {
     _channel = SystemChannels.textInput;
-    _channel.setMethodCallHandler(_handleTextInputInvocation);
+    _channel.setMethodCallHandler(_loudlyHandleTextInputInvocation);
   }
 
   /// Set the [MethodChannel] used to communicate with the system's text input
@@ -1546,7 +1580,7 @@ class TextInput {
   @visibleForTesting
   static void setChannel(MethodChannel newChannel) {
     assert(() {
-      _instance._channel = newChannel..setMethodCallHandler(_instance._handleTextInputInvocation);
+      _instance._channel = newChannel..setMethodCallHandler(_instance._loudlyHandleTextInputInvocation);
       return true;
     }());
   }
@@ -1603,17 +1637,20 @@ class TextInput {
     return connection;
   }
 
-  /// This method actually notifies the embedding of the client. It is utilized
-  /// by [attach] and by [_handleTextInputInvocation] for the
-  /// `TextInputClient.requestExistingInputState` method.
+  // This method actually notifies the embedding of the client. It is utilized
+  // by [attach] and by [_handleTextInputInvocation] for the
+  // `TextInputClient.requestExistingInputState` method.
   void _attach(TextInputConnection connection, TextInputConfiguration configuration) {
     assert(connection != null);
     assert(connection._client != null);
     assert(configuration != null);
     assert(_debugEnsureInputActionWorksOnPlatform(configuration.inputAction));
     _channel.invokeMethod<void>(
       'TextInput.setClient',
-      <dynamic>[ connection._id, configuration.toJson() ],
+      <Object>[
+        connection._id,
+        configuration.toJson(),
+      ],
     );
     _currentConnection = connection;
     _currentConfiguration = configuration;
@@ -1656,6 +1693,23 @@ class TextInput {
   /// Returns true if a scribble interaction is currently happening.
   bool get scribbleInProgress => _scribbleInProgress;
 
+  Future<dynamic> _loudlyHandleTextInputInvocation(MethodCall call) async {
+    try {
+      return await _handleTextInputInvocation(call);
+    } catch (exception, stack) {
+      FlutterError.reportError(FlutterErrorDetails(
+        exception: exception,
+        stack: stack,
+        library: 'services library',
+        context: ErrorDescription('during method call ${call.method}'),
+        informationCollector: () => <DiagnosticsNode>[
+          DiagnosticsProperty<MethodCall>('call', call, style: DiagnosticsTreeStyle.errorProperty),
+        ],
+      ));
+      rethrow;
+    }
+  }
+
   Future<dynamic> _handleTextInputInvocation(MethodCall methodCall) async {
     final String method = methodCall.method;
     if (method == 'TextInputClient.focusElement') {

packages/flutter/test/services/text_input_test.dart

@@ -6,6 +6,7 @@
 import 'dart:convert' show jsonDecode;
 import 'dart:ui';
 
+import 'package:flutter/foundation.dart';
 import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
 
@@ -210,6 +211,37 @@ void main() {
         ),
       ]);
     });
+
+    test('Invalid TextRange fails loudly when being converted to JSON', () async {
+      final List<FlutterErrorDetails> record = <FlutterErrorDetails>[];
+      FlutterError.onError = (FlutterErrorDetails details) {
+        record.add(details);
+      };
+
+      final FakeTextInputClient client = FakeTextInputClient(const TextEditingValue(text: 'test3'));
+      const TextInputConfiguration configuration = TextInputConfiguration();
+      TextInput.attach(client, configuration);
+
+      final ByteData? messageBytes = const JSONMessageCodec().encodeMessage(<String, dynamic>{
+        'method': 'TextInputClient.updateEditingState',
+        'args': <dynamic>[-1, <String, dynamic>{
+          'text': '1',
+          'selectionBase': 2,
+          'selectionExtent': 3,
+        }],
+      });
+
+      await ServicesBinding.instance.defaultBinaryMessenger.handlePlatformMessage(
+        'flutter/textinput',
+        messageBytes,
+        (ByteData? _) {},
+      );
+      expect(record.length, 1);
+      // Verify the error message in parts because Web formats the message
+      // differently from others.
+      expect(record[0].exception.toString(), matches(RegExp(r'\brange.start >= 0 && range.start <= text.length\b')));
+      expect(record[0].exception.toString(), matches(RegExp(r'\bRange start 2 is out of text of length 1\b')));
+    });
   });
 
   group('TextInputConfiguration', () {