Skip to content

Commit 329afbe

Browse files
Renzo-OlivaresRenzo Olivares
Renzo-Olivares
and
Renzo Olivares
authored
updateEditingValueWithDeltas should fail loudly when TextRange is invalid (flutter#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]>
1 parent 2600b2d commit 329afbe

File tree

3 files changed

+245
-11
lines changed

3 files changed

+245
-11
lines changed

‎packages/flutter/lib/src/services/text_editing_delta.dart

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,21 @@ TextAffinity? _toTextAffinity(String? affinity) {
2424
return null;
2525
}
2626

27-
/// Replaces a range of text in the original string with the text given in the
28-
/// replacement string.
29-
String _replace(String originalText, String replacementText, int start, int end) {
30-
final String textStart = originalText.substring(0, start);
31-
final String textEnd = originalText.substring(end, originalText.length);
32-
final String newText = textStart + replacementText + textEnd;
33-
return newText;
27+
// Replaces a range of text in the original string with the text given in the
28+
// replacement string.
29+
String _replace(String originalText, String replacementText, TextRange replacementRange) {
30+
assert(replacementRange.isValid);
31+
return originalText.replaceRange(replacementRange.start, replacementRange.end, replacementText);
32+
}
33+
34+
// Verify that the given range is within the text.
35+
bool _debugTextRangeIsValid(TextRange range, String text) {
36+
if (!range.isValid) {
37+
return true;
38+
}
39+
40+
return (range.start >= 0 && range.start <= text.length)
41+
&& (range.end >= 0 && range.end <= text.length);
3442
}
3543

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

128136
if (isNonTextUpdate) {
137+
assert(_debugTextRangeIsValid(newSelection, oldText), 'The selection range: $newSelection is not within the bounds of text: $oldText of length: ${oldText.length}');
138+
assert(_debugTextRangeIsValid(newComposing, oldText), 'The composing range: $newComposing is not within the bounds of text: $oldText of length: ${oldText.length}');
139+
129140
return TextEditingDeltaNonTextUpdate(
130141
oldText: oldText,
131142
selection: newSelection,
132143
composing: newComposing,
133144
);
134145
}
135146

136-
final String newText = _replace(oldText, replacementSource, replacementDestinationStart, replacementDestinationEnd);
147+
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}');
148+
149+
final String newText = _replace(oldText, replacementSource, TextRange(start: replacementDestinationStart, end: replacementDestinationEnd));
150+
151+
assert(_debugTextRangeIsValid(newSelection, newText), 'The selection range: $newSelection is not within the bounds of text: $newText of length: ${newText.length}');
152+
assert(_debugTextRangeIsValid(newComposing, newText), 'The composing range: $newComposing is not within the bounds of text: $newText of length: ${newText.length}');
153+
137154
final bool isEqual = oldText == newText;
138155

139156
final bool isDeletionGreaterThanOne = (replacementDestinationEnd - replacementDestinationStart) - (replacementSourceEnd - replacementSourceStart) > 1;
@@ -265,7 +282,10 @@ class TextEditingDeltaInsertion extends TextEditingDelta {
265282
// policy and apply the delta to the oldText. This is due to the asyncronous
266283
// nature of the connection between the framework and platform text input plugins.
267284
String newText = oldText;
268-
newText = _replace(newText, textInserted, insertionOffset, insertionOffset);
285+
assert(_debugTextRangeIsValid(TextRange.collapsed(insertionOffset), newText), 'Applying TextEditingDeltaInsertion failed, the insertionOffset: $insertionOffset is not within the bounds of $newText of length: ${newText.length}');
286+
newText = _replace(newText, textInserted, TextRange.collapsed(insertionOffset));
287+
assert(_debugTextRangeIsValid(selection, newText), 'Applying TextEditingDeltaInsertion failed, the selection range: $selection is not within the bounds of $newText of length: ${newText.length}');
288+
assert(_debugTextRangeIsValid(composing, newText), 'Applying TextEditingDeltaInsertion failed, the composing range: $composing is not within the bounds of $newText of length: ${newText.length}');
269289
return value.copyWith(text: newText, selection: selection, composing: composing);
270290
}
271291
}
@@ -298,7 +318,10 @@ class TextEditingDeltaDeletion extends TextEditingDelta {
298318
// policy and apply the delta to the oldText. This is due to the asyncronous
299319
// nature of the connection between the framework and platform text input plugins.
300320
String newText = oldText;
301-
newText = _replace(newText, '', deletedRange.start, deletedRange.end);
321+
assert(_debugTextRangeIsValid(deletedRange, newText), 'Applying TextEditingDeltaDeletion failed, the deletedRange: $deletedRange is not within the bounds of $newText of length: ${newText.length}');
322+
newText = _replace(newText, '', deletedRange);
323+
assert(_debugTextRangeIsValid(selection, newText), 'Applying TextEditingDeltaDeletion failed, the selection range: $selection is not within the bounds of $newText of length: ${newText.length}');
324+
assert(_debugTextRangeIsValid(composing, newText), 'Applying TextEditingDeltaDeletion failed, the composing range: $composing is not within the bounds of $newText of length: ${newText.length}');
302325
return value.copyWith(text: newText, selection: selection, composing: composing);
303326
}
304327
}
@@ -341,7 +364,10 @@ class TextEditingDeltaReplacement extends TextEditingDelta {
341364
// policy and apply the delta to the oldText. This is due to the asyncronous
342365
// nature of the connection between the framework and platform text input plugins.
343366
String newText = oldText;
344-
newText = _replace(newText, replacementText, replacedRange.start, replacedRange.end);
367+
assert(_debugTextRangeIsValid(replacedRange, newText), 'Applying TextEditingDeltaReplacement failed, the replacedRange: $replacedRange is not within the bounds of $newText of length: ${newText.length}');
368+
newText = _replace(newText, replacementText, replacedRange);
369+
assert(_debugTextRangeIsValid(selection, newText), 'Applying TextEditingDeltaReplacement failed, the selection range: $selection is not within the bounds of $newText of length: ${newText.length}');
370+
assert(_debugTextRangeIsValid(composing, newText), 'Applying TextEditingDeltaReplacement failed, the composing range: $composing is not within the bounds of $newText of length: ${newText.length}');
345371
return value.copyWith(text: newText, selection: selection, composing: composing);
346372
}
347373
}
@@ -372,6 +398,8 @@ class TextEditingDeltaNonTextUpdate extends TextEditingDelta {
372398
// To stay inline with the plain text model we should follow a last write wins
373399
// policy and apply the delta to the oldText. This is due to the asyncronous
374400
// nature of the connection between the framework and platform text input plugins.
401+
assert(_debugTextRangeIsValid(selection, oldText), 'Applying TextEditingDeltaNonTextUpdate failed, the selection range: $selection is not within the bounds of $oldText of length: ${oldText.length}');
402+
assert(_debugTextRangeIsValid(composing, oldText), 'Applying TextEditingDeltaNonTextUpdate failed, the composing region: $composing is not within the bounds of $oldText of length: ${oldText.length}');
375403
return TextEditingValue(text: oldText, selection: selection, composing: composing);
376404
}
377405
}

‎packages/flutter/test/services/delta_text_input_test.dart

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'dart:convert' show jsonDecode;
66

7+
import 'package:flutter/foundation.dart';
78
import 'package:flutter/services.dart';
89
import 'package:flutter_test/flutter_test.dart';
910

@@ -65,6 +66,162 @@ void main() {
6566
expect(client.latestMethodCall, 'updateEditingValueWithDeltas');
6667
},
6768
);
69+
70+
test('Invalid TextRange fails loudly when being converted to JSON - NonTextUpdate', () async {
71+
final List<FlutterErrorDetails> record = <FlutterErrorDetails>[];
72+
FlutterError.onError = (FlutterErrorDetails details) {
73+
record.add(details);
74+
};
75+
76+
final FakeDeltaTextInputClient client = FakeDeltaTextInputClient(const TextEditingValue(text: '1'));
77+
const TextInputConfiguration configuration = TextInputConfiguration(enableDeltaModel: true);
78+
TextInput.attach(client, configuration);
79+
80+
const String jsonDelta = '{'
81+
'"oldText": "1",'
82+
' "deltaText": "",'
83+
' "deltaStart": -1,'
84+
' "deltaEnd": -1,'
85+
' "selectionBase": 3,'
86+
' "selectionExtent": 3,'
87+
' "selectionAffinity" : "TextAffinity.downstream" ,'
88+
' "selectionIsDirectional": false,'
89+
' "composingBase": -1,'
90+
' "composingExtent": -1}';
91+
92+
final ByteData? messageBytes = const JSONMessageCodec().encodeMessage(<String, dynamic>{
93+
'method': 'TextInputClient.updateEditingStateWithDeltas',
94+
'args': <dynamic>[-1, jsonDecode('{"deltas": [$jsonDelta]}')],
95+
});
96+
97+
await ServicesBinding.instance.defaultBinaryMessenger.handlePlatformMessage(
98+
'flutter/textinput',
99+
messageBytes,
100+
(ByteData? _) {},
101+
);
102+
expect(record.length, 1);
103+
// Verify the error message in parts because Web formats the message
104+
// differently from others.
105+
expect(record[0].exception.toString(), matches(RegExp(r'\bThe selection range: TextSelection.collapsed\(offset: 3, affinity: TextAffinity.downstream, isDirectional: false\)(?!\w)')));
106+
expect(record[0].exception.toString(), matches(RegExp(r'\bis not within the bounds of text: 1 of length: 1\b')));
107+
});
108+
109+
test('Invalid TextRange fails loudly when being converted to JSON - Faulty deltaStart and deltaEnd', () async {
110+
final List<FlutterErrorDetails> record = <FlutterErrorDetails>[];
111+
FlutterError.onError = (FlutterErrorDetails details) {
112+
record.add(details);
113+
};
114+
115+
final FakeDeltaTextInputClient client = FakeDeltaTextInputClient(TextEditingValue.empty);
116+
const TextInputConfiguration configuration = TextInputConfiguration(enableDeltaModel: true);
117+
TextInput.attach(client, configuration);
118+
119+
const String jsonDelta = '{'
120+
'"oldText": "",'
121+
' "deltaText": "hello",'
122+
' "deltaStart": 0,'
123+
' "deltaEnd": 1,'
124+
' "selectionBase": 5,'
125+
' "selectionExtent": 5,'
126+
' "selectionAffinity" : "TextAffinity.downstream" ,'
127+
' "selectionIsDirectional": false,'
128+
' "composingBase": -1,'
129+
' "composingExtent": -1}';
130+
131+
final ByteData? messageBytes = const JSONMessageCodec().encodeMessage(<String, dynamic>{
132+
'method': 'TextInputClient.updateEditingStateWithDeltas',
133+
'args': <dynamic>[-1, jsonDecode('{"deltas": [$jsonDelta]}')],
134+
});
135+
136+
await ServicesBinding.instance.defaultBinaryMessenger.handlePlatformMessage(
137+
'flutter/textinput',
138+
messageBytes,
139+
(ByteData? _) {},
140+
);
141+
expect(record.length, 1);
142+
// Verify the error message in parts because Web formats the message
143+
// differently from others.
144+
expect(record[0].exception.toString(), matches(RegExp(r'\bThe delta range: TextRange\(start: 0, end: 5\)(?!\w)')));
145+
expect(record[0].exception.toString(), matches(RegExp(r'\bis not within the bounds of text: of length: 0\b')));
146+
});
147+
148+
test('Invalid TextRange fails loudly when being converted to JSON - Faulty Selection', () async {
149+
final List<FlutterErrorDetails> record = <FlutterErrorDetails>[];
150+
FlutterError.onError = (FlutterErrorDetails details) {
151+
record.add(details);
152+
};
153+
154+
final FakeDeltaTextInputClient client = FakeDeltaTextInputClient(TextEditingValue.empty);
155+
const TextInputConfiguration configuration = TextInputConfiguration(enableDeltaModel: true);
156+
TextInput.attach(client, configuration);
157+
158+
const String jsonDelta = '{'
159+
'"oldText": "",'
160+
' "deltaText": "hello",'
161+
' "deltaStart": 0,'
162+
' "deltaEnd": 0,'
163+
' "selectionBase": 6,'
164+
' "selectionExtent": 6,'
165+
' "selectionAffinity" : "TextAffinity.downstream" ,'
166+
' "selectionIsDirectional": false,'
167+
' "composingBase": -1,'
168+
' "composingExtent": -1}';
169+
170+
final ByteData? messageBytes = const JSONMessageCodec().encodeMessage(<String, dynamic>{
171+
'method': 'TextInputClient.updateEditingStateWithDeltas',
172+
'args': <dynamic>[-1, jsonDecode('{"deltas": [$jsonDelta]}')],
173+
});
174+
175+
await ServicesBinding.instance.defaultBinaryMessenger.handlePlatformMessage(
176+
'flutter/textinput',
177+
messageBytes,
178+
(ByteData? _) {},
179+
);
180+
expect(record.length, 1);
181+
// Verify the error message in parts because Web formats the message
182+
// differently from others.
183+
expect(record[0].exception.toString(), matches(RegExp(r'\bThe selection range: TextSelection.collapsed\(offset: 6, affinity: TextAffinity.downstream, isDirectional: false\)(?!\w)')));
184+
expect(record[0].exception.toString(), matches(RegExp(r'\bis not within the bounds of text: hello of length: 5\b')));
185+
});
186+
187+
test('Invalid TextRange fails loudly when being converted to JSON - Faulty Composing Region', () async {
188+
final List<FlutterErrorDetails> record = <FlutterErrorDetails>[];
189+
FlutterError.onError = (FlutterErrorDetails details) {
190+
record.add(details);
191+
};
192+
193+
final FakeDeltaTextInputClient client = FakeDeltaTextInputClient(const TextEditingValue(text: 'worl'));
194+
const TextInputConfiguration configuration = TextInputConfiguration(enableDeltaModel: true);
195+
TextInput.attach(client, configuration);
196+
197+
const String jsonDelta = '{'
198+
'"oldText": "worl",'
199+
' "deltaText": "world",'
200+
' "deltaStart": 0,'
201+
' "deltaEnd": 4,'
202+
' "selectionBase": 5,'
203+
' "selectionExtent": 5,'
204+
' "selectionAffinity" : "TextAffinity.downstream" ,'
205+
' "selectionIsDirectional": false,'
206+
' "composingBase": 0,'
207+
' "composingExtent": 6}';
208+
209+
final ByteData? messageBytes = const JSONMessageCodec().encodeMessage(<String, dynamic>{
210+
'method': 'TextInputClient.updateEditingStateWithDeltas',
211+
'args': <dynamic>[-1, jsonDecode('{"deltas": [$jsonDelta]}')],
212+
});
213+
214+
await ServicesBinding.instance.defaultBinaryMessenger.handlePlatformMessage(
215+
'flutter/textinput',
216+
messageBytes,
217+
(ByteData? _) {},
218+
);
219+
expect(record.length, 1);
220+
// Verify the error message in parts because Web formats the message
221+
// differently from others.
222+
expect(record[0].exception.toString(), matches(RegExp(r'\bThe composing range: TextRange\(start: 0, end: 6\)(?!\w)')));
223+
expect(record[0].exception.toString(), matches(RegExp(r'\bis not within the bounds of text: world of length: 5\b')));
224+
});
68225
});
69226
}
70227

‎packages/flutter/test/services/text_editing_delta_test.dart

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,19 @@ void main() {
5757
expect(delta.selection, expectedSelection);
5858
expect(delta.composing, expectedComposing);
5959
});
60+
61+
test('Verify invalid TextEditingDeltaInsertion fails to apply', () {
62+
const TextEditingDeltaInsertion delta =
63+
TextEditingDeltaInsertion(
64+
oldText: 'hello worl',
65+
textInserted: 'd',
66+
insertionOffset: 11,
67+
selection: TextSelection.collapsed(offset: 11),
68+
composing: TextRange.empty,
69+
);
70+
71+
expect(() { delta.apply(TextEditingValue.empty); }, throwsAssertionError);
72+
});
6073
});
6174

6275
group('TextEditingDeltaDeletion', () {
@@ -109,6 +122,18 @@ void main() {
109122
expect(delta.selection, expectedSelection);
110123
expect(delta.composing, expectedComposing);
111124
});
125+
126+
test('Verify invalid TextEditingDeltaDeletion fails to apply', () {
127+
const TextEditingDeltaDeletion delta =
128+
TextEditingDeltaDeletion(
129+
oldText: 'hello world',
130+
deletedRange: TextRange(start: 5, end: 12),
131+
selection: TextSelection.collapsed(offset: 5),
132+
composing: TextRange.empty,
133+
);
134+
135+
expect(() { delta.apply(TextEditingValue.empty); }, throwsAssertionError);
136+
});
112137
});
113138

114139
group('TextEditingDeltaReplacement', () {
@@ -189,6 +214,19 @@ void main() {
189214
expect(delta.selection, expectedSelection);
190215
expect(delta.composing, expectedComposing);
191216
});
217+
218+
test('Verify invalid TextEditingDeltaReplacement fails to apply', () {
219+
const TextEditingDeltaReplacement delta =
220+
TextEditingDeltaReplacement(
221+
oldText: 'hello worl',
222+
replacementText: 'world',
223+
replacedRange: TextRange(start: 5, end: 11),
224+
selection: TextSelection.collapsed(offset: 11),
225+
composing: TextRange.empty,
226+
);
227+
228+
expect(() { delta.apply(TextEditingValue.empty); }, throwsAssertionError);
229+
});
192230
});
193231

194232
group('TextEditingDeltaNonTextUpdate', () {
@@ -213,5 +251,16 @@ void main() {
213251
expect(delta.selection, expectedSelection);
214252
expect(delta.composing, expectedComposing);
215253
});
254+
255+
test('Verify invalid TextEditingDeltaNonTextUpdate fails to apply', () {
256+
const TextEditingDeltaNonTextUpdate delta =
257+
TextEditingDeltaNonTextUpdate(
258+
oldText: 'hello world',
259+
selection: TextSelection.collapsed(offset: 12),
260+
composing: TextRange.empty,
261+
);
262+
263+
expect(() { delta.apply(TextEditingValue.empty); }, throwsAssertionError);
264+
});
216265
});
217266
}

0 commit comments

Comments
 (0)