Skip to content

Commit

Permalink
updateEditingValueWithDeltas should fail loudly when TextRange is inv…
Browse files Browse the repository at this point in the history 
…alid (#107426)

* Make deltas fail loudly

* analyzer fixes

* empty

* updates

* Analyzer fixes

* Make it more obvious what kind of TextRange is failing and where

* update tests

* Add tests for concrete TextEditinDelta apply method

* trailing spaces

* address nits

* fix analyzer

Co-authored-by: Renzo Olivares <[email protected]>
  • Loading branch information
@Renzo-Olivares
Renzo-Olivares and Renzo Olivares authored Jul 12, 2022
1 parent 2600b2d commit 329afbe
Show file tree
Hide file tree
Showing 3 changed files with 245 additions and 11 deletions.
50 changes: 39 additions & 11 deletions packages/flutter/lib/src/services/text_editing_delta.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,21 @@ TextAffinity? _toTextAffinity(String? affinity) {
return null;
}

/// Replaces a range of text in the original string with the text given in the
/// replacement string.
String _replace(String originalText, String replacementText, int start, int end) {
final String textStart = originalText.substring(0, start);
final String textEnd = originalText.substring(end, originalText.length);
final String newText = textStart + replacementText + textEnd;
return newText;
// Replaces a range of text in the original string with the text given in the
// replacement string.
String _replace(String originalText, String replacementText, TextRange replacementRange) {
assert(replacementRange.isValid);
return originalText.replaceRange(replacementRange.start, replacementRange.end, replacementText);
}

// Verify that the given range is within the text.
bool _debugTextRangeIsValid(TextRange range, String text) {
if (!range.isValid) {
return true;
}

return (range.start >= 0 && range.start <= text.length)
&& (range.end >= 0 && range.end <= text.length);
}

/// A structure representing a granular change that has occurred to the editing
Expand Down Expand Up @@ -126,14 +134,23 @@ abstract class TextEditingDelta {
);

if (isNonTextUpdate) {
assert(_debugTextRangeIsValid(newSelection, oldText), 'The selection range: $newSelection is not within the bounds of text: $oldText of length: ${oldText.length}');
assert(_debugTextRangeIsValid(newComposing, oldText), 'The composing range: $newComposing is not within the bounds of text: $oldText of length: ${oldText.length}');

return TextEditingDeltaNonTextUpdate(
oldText: oldText,
selection: newSelection,
composing: newComposing,
);
}

final String newText = _replace(oldText, replacementSource, replacementDestinationStart, replacementDestinationEnd);
assert(_debugTextRangeIsValid(TextRange(start: replacementDestinationStart, end: replacementDestinationEnd), oldText), 'The delta range: ${TextRange(start: replacementSourceStart, end: replacementSourceEnd)} is not within the bounds of text: $oldText of length: ${oldText.length}');

final String newText = _replace(oldText, replacementSource, TextRange(start: replacementDestinationStart, end: replacementDestinationEnd));

assert(_debugTextRangeIsValid(newSelection, newText), 'The selection range: $newSelection is not within the bounds of text: $newText of length: ${newText.length}');
assert(_debugTextRangeIsValid(newComposing, newText), 'The composing range: $newComposing is not within the bounds of text: $newText of length: ${newText.length}');

final bool isEqual = oldText == newText;

final bool isDeletionGreaterThanOne = (replacementDestinationEnd - replacementDestinationStart) - (replacementSourceEnd - replacementSourceStart) > 1;
Expand Down Expand Up @@ -265,7 +282,10 @@ class TextEditingDeltaInsertion extends TextEditingDelta {
// policy and apply the delta to the oldText. This is due to the asyncronous
// nature of the connection between the framework and platform text input plugins.
String newText = oldText;
newText = _replace(newText, textInserted, insertionOffset, insertionOffset);
assert(_debugTextRangeIsValid(TextRange.collapsed(insertionOffset), newText), 'Applying TextEditingDeltaInsertion failed, the insertionOffset: $insertionOffset is not within the bounds of $newText of length: ${newText.length}');
newText = _replace(newText, textInserted, TextRange.collapsed(insertionOffset));
assert(_debugTextRangeIsValid(selection, newText), 'Applying TextEditingDeltaInsertion failed, the selection range: $selection is not within the bounds of $newText of length: ${newText.length}');
assert(_debugTextRangeIsValid(composing, newText), 'Applying TextEditingDeltaInsertion failed, the composing range: $composing is not within the bounds of $newText of length: ${newText.length}');
return value.copyWith(text: newText, selection: selection, composing: composing);
}
}
Expand Down Expand Up @@ -298,7 +318,10 @@ class TextEditingDeltaDeletion extends TextEditingDelta {
// policy and apply the delta to the oldText. This is due to the asyncronous
// nature of the connection between the framework and platform text input plugins.
String newText = oldText;
newText = _replace(newText, '', deletedRange.start, deletedRange.end);
assert(_debugTextRangeIsValid(deletedRange, newText), 'Applying TextEditingDeltaDeletion failed, the deletedRange: $deletedRange is not within the bounds of $newText of length: ${newText.length}');
newText = _replace(newText, '', deletedRange);
assert(_debugTextRangeIsValid(selection, newText), 'Applying TextEditingDeltaDeletion failed, the selection range: $selection is not within the bounds of $newText of length: ${newText.length}');
assert(_debugTextRangeIsValid(composing, newText), 'Applying TextEditingDeltaDeletion failed, the composing range: $composing is not within the bounds of $newText of length: ${newText.length}');
return value.copyWith(text: newText, selection: selection, composing: composing);
}
}
Expand Down Expand Up @@ -341,7 +364,10 @@ class TextEditingDeltaReplacement extends TextEditingDelta {
// policy and apply the delta to the oldText. This is due to the asyncronous
// nature of the connection between the framework and platform text input plugins.
String newText = oldText;
newText = _replace(newText, replacementText, replacedRange.start, replacedRange.end);
assert(_debugTextRangeIsValid(replacedRange, newText), 'Applying TextEditingDeltaReplacement failed, the replacedRange: $replacedRange is not within the bounds of $newText of length: ${newText.length}');
newText = _replace(newText, replacementText, replacedRange);
assert(_debugTextRangeIsValid(selection, newText), 'Applying TextEditingDeltaReplacement failed, the selection range: $selection is not within the bounds of $newText of length: ${newText.length}');
assert(_debugTextRangeIsValid(composing, newText), 'Applying TextEditingDeltaReplacement failed, the composing range: $composing is not within the bounds of $newText of length: ${newText.length}');
return value.copyWith(text: newText, selection: selection, composing: composing);
}
}
Expand Down Expand Up @@ -372,6 +398,8 @@ class TextEditingDeltaNonTextUpdate extends TextEditingDelta {
// To stay inline with the plain text model we should follow a last write wins
// policy and apply the delta to the oldText. This is due to the asyncronous
// nature of the connection between the framework and platform text input plugins.
assert(_debugTextRangeIsValid(selection, oldText), 'Applying TextEditingDeltaNonTextUpdate failed, the selection range: $selection is not within the bounds of $oldText of length: ${oldText.length}');
assert(_debugTextRangeIsValid(composing, oldText), 'Applying TextEditingDeltaNonTextUpdate failed, the composing region: $composing is not within the bounds of $oldText of length: ${oldText.length}');
return TextEditingValue(text: oldText, selection: selection, composing: composing);
}
}
157 changes: 157 additions & 0 deletions packages/flutter/test/services/delta_text_input_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import 'dart:convert' show jsonDecode;

import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';

Expand Down Expand Up @@ -65,6 +66,162 @@ void main() {
expect(client.latestMethodCall, 'updateEditingValueWithDeltas');
},
);

test('Invalid TextRange fails loudly when being converted to JSON - NonTextUpdate', () async {
final List<FlutterErrorDetails> record = <FlutterErrorDetails>[];
FlutterError.onError = (FlutterErrorDetails details) {
record.add(details);
};

final FakeDeltaTextInputClient client = FakeDeltaTextInputClient(const TextEditingValue(text: '1'));
const TextInputConfiguration configuration = TextInputConfiguration(enableDeltaModel: true);
TextInput.attach(client, configuration);

const String jsonDelta = '{'
'"oldText": "1",'
' "deltaText": "",'
' "deltaStart": -1,'
' "deltaEnd": -1,'
' "selectionBase": 3,'
' "selectionExtent": 3,'
' "selectionAffinity" : "TextAffinity.downstream" ,'
' "selectionIsDirectional": false,'
' "composingBase": -1,'
' "composingExtent": -1}';

final ByteData? messageBytes = const JSONMessageCodec().encodeMessage(<String, dynamic>{
'method': 'TextInputClient.updateEditingStateWithDeltas',
'args': <dynamic>[-1, jsonDecode('{"deltas": [$jsonDelta]}')],
});

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'\bThe selection range: TextSelection.collapsed\(offset: 3, affinity: TextAffinity.downstream, isDirectional: false\)(?!\w)')));
expect(record[0].exception.toString(), matches(RegExp(r'\bis not within the bounds of text: 1 of length: 1\b')));
});

test('Invalid TextRange fails loudly when being converted to JSON - Faulty deltaStart and deltaEnd', () async {
final List<FlutterErrorDetails> record = <FlutterErrorDetails>[];
FlutterError.onError = (FlutterErrorDetails details) {
record.add(details);
};

final FakeDeltaTextInputClient client = FakeDeltaTextInputClient(TextEditingValue.empty);
const TextInputConfiguration configuration = TextInputConfiguration(enableDeltaModel: true);
TextInput.attach(client, configuration);

const String jsonDelta = '{'
'"oldText": "",'
' "deltaText": "hello",'
' "deltaStart": 0,'
' "deltaEnd": 1,'
' "selectionBase": 5,'
' "selectionExtent": 5,'
' "selectionAffinity" : "TextAffinity.downstream" ,'
' "selectionIsDirectional": false,'
' "composingBase": -1,'
' "composingExtent": -1}';

final ByteData? messageBytes = const JSONMessageCodec().encodeMessage(<String, dynamic>{
'method': 'TextInputClient.updateEditingStateWithDeltas',
'args': <dynamic>[-1, jsonDecode('{"deltas": [$jsonDelta]}')],
});

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'\bThe delta range: TextRange\(start: 0, end: 5\)(?!\w)')));
expect(record[0].exception.toString(), matches(RegExp(r'\bis not within the bounds of text: of length: 0\b')));
});

test('Invalid TextRange fails loudly when being converted to JSON - Faulty Selection', () async {
final List<FlutterErrorDetails> record = <FlutterErrorDetails>[];
FlutterError.onError = (FlutterErrorDetails details) {
record.add(details);
};

final FakeDeltaTextInputClient client = FakeDeltaTextInputClient(TextEditingValue.empty);
const TextInputConfiguration configuration = TextInputConfiguration(enableDeltaModel: true);
TextInput.attach(client, configuration);

const String jsonDelta = '{'
'"oldText": "",'
' "deltaText": "hello",'
' "deltaStart": 0,'
' "deltaEnd": 0,'
' "selectionBase": 6,'
' "selectionExtent": 6,'
' "selectionAffinity" : "TextAffinity.downstream" ,'
' "selectionIsDirectional": false,'
' "composingBase": -1,'
' "composingExtent": -1}';

final ByteData? messageBytes = const JSONMessageCodec().encodeMessage(<String, dynamic>{
'method': 'TextInputClient.updateEditingStateWithDeltas',
'args': <dynamic>[-1, jsonDecode('{"deltas": [$jsonDelta]}')],
});

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'\bThe selection range: TextSelection.collapsed\(offset: 6, affinity: TextAffinity.downstream, isDirectional: false\)(?!\w)')));
expect(record[0].exception.toString(), matches(RegExp(r'\bis not within the bounds of text: hello of length: 5\b')));
});

test('Invalid TextRange fails loudly when being converted to JSON - Faulty Composing Region', () async {
final List<FlutterErrorDetails> record = <FlutterErrorDetails>[];
FlutterError.onError = (FlutterErrorDetails details) {
record.add(details);
};

final FakeDeltaTextInputClient client = FakeDeltaTextInputClient(const TextEditingValue(text: 'worl'));
const TextInputConfiguration configuration = TextInputConfiguration(enableDeltaModel: true);
TextInput.attach(client, configuration);

const String jsonDelta = '{'
'"oldText": "worl",'
' "deltaText": "world",'
' "deltaStart": 0,'
' "deltaEnd": 4,'
' "selectionBase": 5,'
' "selectionExtent": 5,'
' "selectionAffinity" : "TextAffinity.downstream" ,'
' "selectionIsDirectional": false,'
' "composingBase": 0,'
' "composingExtent": 6}';

final ByteData? messageBytes = const JSONMessageCodec().encodeMessage(<String, dynamic>{
'method': 'TextInputClient.updateEditingStateWithDeltas',
'args': <dynamic>[-1, jsonDecode('{"deltas": [$jsonDelta]}')],
});

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'\bThe composing range: TextRange\(start: 0, end: 6\)(?!\w)')));
expect(record[0].exception.toString(), matches(RegExp(r'\bis not within the bounds of text: world of length: 5\b')));
});
});
}

Expand Down
49 changes: 49 additions & 0 deletions packages/flutter/test/services/text_editing_delta_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,19 @@ void main() {
expect(delta.selection, expectedSelection);
expect(delta.composing, expectedComposing);
});

test('Verify invalid TextEditingDeltaInsertion fails to apply', () {
const TextEditingDeltaInsertion delta =
TextEditingDeltaInsertion(
oldText: 'hello worl',
textInserted: 'd',
insertionOffset: 11,
selection: TextSelection.collapsed(offset: 11),
composing: TextRange.empty,
);

expect(() { delta.apply(TextEditingValue.empty); }, throwsAssertionError);
});
});

group('TextEditingDeltaDeletion', () {
Expand Down Expand Up @@ -109,6 +122,18 @@ void main() {
expect(delta.selection, expectedSelection);
expect(delta.composing, expectedComposing);
});

test('Verify invalid TextEditingDeltaDeletion fails to apply', () {
const TextEditingDeltaDeletion delta =
TextEditingDeltaDeletion(
oldText: 'hello world',
deletedRange: TextRange(start: 5, end: 12),
selection: TextSelection.collapsed(offset: 5),
composing: TextRange.empty,
);

expect(() { delta.apply(TextEditingValue.empty); }, throwsAssertionError);
});
});

group('TextEditingDeltaReplacement', () {
Expand Down Expand Up @@ -189,6 +214,19 @@ void main() {
expect(delta.selection, expectedSelection);
expect(delta.composing, expectedComposing);
});

test('Verify invalid TextEditingDeltaReplacement fails to apply', () {
const TextEditingDeltaReplacement delta =
TextEditingDeltaReplacement(
oldText: 'hello worl',
replacementText: 'world',
replacedRange: TextRange(start: 5, end: 11),
selection: TextSelection.collapsed(offset: 11),
composing: TextRange.empty,
);

expect(() { delta.apply(TextEditingValue.empty); }, throwsAssertionError);
});
});

group('TextEditingDeltaNonTextUpdate', () {
Expand All @@ -213,5 +251,16 @@ void main() {
expect(delta.selection, expectedSelection);
expect(delta.composing, expectedComposing);
});

test('Verify invalid TextEditingDeltaNonTextUpdate fails to apply', () {
const TextEditingDeltaNonTextUpdate delta =
TextEditingDeltaNonTextUpdate(
oldText: 'hello world',
selection: TextSelection.collapsed(offset: 12),
composing: TextRange.empty,
);

expect(() { delta.apply(TextEditingValue.empty); }, throwsAssertionError);
});
});
}

0 comments on commit 329afbe

Please sign in to comment.