Skip to content

Commit 14bbd0b

Browse files
authored
Sync a few small CLs: (#1060)
- cl/660721933: moves some proto3 parsing functions to utils to allow reusing internally in a library when parsing an internal format. - cl/646759129: Update `CodedBufferReader` default depth limit to 100, to be consistent with the C++ and Java implementations. - cl/704280814: Update `CodedBufferReader` default size limit to max 32-bit signed integer to be consistent with the C++, Java, Objective-C implementations. - Also a minor change, not a CL: Type casts in `coded_buffer_writer.dart` was updated in this repo probably accidentally, as we never want to type cast with `as` in this repo. Revert it to the internal version which uses implicit casts. With this, about half of the protobuf files become identical with the internal version.
1 parent fb89979 commit 14bbd0b

File tree

9 files changed

+90
-63
lines changed

9 files changed

+90
-63
lines changed

protobuf/CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
## 5.1.0-wip
2+
3+
* Update default size limit of `CodedBufferReader` from 67,108,864 bytes to
4+
2,147,483,647 bytes, and default recursion limit from 64 to 100.
5+
6+
The new limits are consistent with the Java and C++ implementations. ([#1060])
7+
8+
[#1060]: https://github.com/google/protobuf.dart/pull/1060
9+
110
## 5.0.0
211

312
* Improve performance of `GeneratedMessage.deepCopy`. ([#742])

protobuf/lib/src/protobuf/coded_buffer_reader.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ part of 'internal.dart';
88
/// [GeneratedMessage]s.
99
class CodedBufferReader {
1010
// ignore: constant_identifier_names
11-
static const int DEFAULT_RECURSION_LIMIT = 64;
11+
static const int DEFAULT_RECURSION_LIMIT = 100;
12+
// Maximum value of a 32-bit signed integer.
1213
// ignore: constant_identifier_names
13-
static const int DEFAULT_SIZE_LIMIT = 64 << 20;
14+
static const int DEFAULT_SIZE_LIMIT = (1 << 31) - 1;
1415

1516
final Uint8List _buffer;
1617

protobuf/lib/src/protobuf/coded_buffer_writer.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,11 @@ class CodedBufferWriter {
6464
_commitChunk(true);
6565
}
6666

67-
void writeField(int fieldNumber, int fieldType, Object? fieldValue) {
67+
void writeField(int fieldNumber, int fieldType, dynamic fieldValue) {
6868
final valueType = PbFieldType.baseType(fieldType);
6969

7070
if ((fieldType & PbFieldType.PACKED_BIT) != 0) {
71-
final list = fieldValue as List;
71+
final List list = fieldValue;
7272
if (list.isNotEmpty) {
7373
_writeTag(fieldNumber, WIRETYPE_LENGTH_DELIMITED);
7474
final mark = _startLengthDelimited();
@@ -81,7 +81,7 @@ class CodedBufferWriter {
8181
}
8282

8383
if ((fieldType & PbFieldType.MAP_BIT) != 0) {
84-
final map = fieldValue as PbMap;
84+
final PbMap map = fieldValue;
8585
final keyWireFormat = _wireTypes[_valueTypeIndex(map.keyFieldType)];
8686
final valueWireFormat = _wireTypes[_valueTypeIndex(map.valueFieldType)];
8787

@@ -108,7 +108,7 @@ class CodedBufferWriter {
108108
final wireFormat = _wireTypes[_valueTypeIndex(valueType)];
109109

110110
if ((fieldType & PbFieldType.REPEATED_BIT) != 0) {
111-
final list = fieldValue as List;
111+
final List list = fieldValue;
112112
for (var i = 0; i < list.length; i++) {
113113
_writeValue(fieldNumber, valueType, list[i], wireFormat);
114114
}

protobuf/lib/src/protobuf/exceptions.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ class InvalidProtocolBufferException implements Exception {
3535
InvalidProtocolBufferException.recursionLimitExceeded()
3636
: this._('''
3737
Protocol message had too many levels of nesting. May be malicious.
38-
Use CodedBufferReader.setRecursionLimit() to increase the depth limit.
38+
Use a CodedBufferReader with a defined recursion depth limit if you need to
39+
parse deeply nested messages.
3940
''');
4041

4142
InvalidProtocolBufferException.truncatedMessage()

protobuf/lib/src/protobuf/field_type.dart

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,18 @@ part of 'internal.dart';
77

88
/// Defines constants and functions for dealing with fieldType bits.
99
class PbFieldType {
10-
static bool isRepeated(int fieldType) =>
11-
(fieldType & PbFieldType.REPEATED_BIT) != 0;
10+
static bool isRepeated(int fieldType) => (fieldType & REPEATED_BIT) != 0;
1211

13-
static bool isRequired(int fieldType) =>
14-
(fieldType & PbFieldType.REQUIRED_BIT) != 0;
12+
static bool isRequired(int fieldType) => (fieldType & REQUIRED_BIT) != 0;
1513

16-
static bool isEnum(int fieldType) =>
17-
PbFieldType.baseType(fieldType) == PbFieldType.ENUM_BIT;
14+
static bool isEnum(int fieldType) => baseType(fieldType) == ENUM_BIT;
1815

19-
static bool isBytes(int fieldType) =>
20-
PbFieldType.baseType(fieldType) == PbFieldType.BYTES_BIT;
16+
static bool isBytes(int fieldType) => baseType(fieldType) == BYTES_BIT;
2117

2218
static bool isGroupOrMessage(int fieldType) =>
23-
(fieldType & (PbFieldType.GROUP_BIT | PbFieldType.MESSAGE_BIT)) != 0;
19+
(fieldType & (GROUP_BIT | MESSAGE_BIT)) != 0;
2420

25-
static bool isMapField(int fieldType) =>
26-
(fieldType & PbFieldType.MAP_BIT) != 0;
21+
static bool isMapField(int fieldType) => (fieldType & MAP_BIT) != 0;
2722

2823
/// Returns the base field type without any of the required, repeated
2924
/// and packed bits.
@@ -176,6 +171,7 @@ class PbFieldType {
176171
static const int PACKED_SFIXED64 = REPEATED_BIT | PACKED_BIT | SFIXED64_BIT;
177172

178173
static const int MAP = MAP_BIT | MESSAGE_BIT;
174+
179175
// Short names for use in generated code.
180176

181177
// _O_ptional.

protobuf/lib/src/protobuf/proto3_json.dart

Lines changed: 11 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -191,33 +191,6 @@ Object? _writeToProto3Json(FieldSet fs, TypeRegistry typeRegistry) {
191191
return result;
192192
}
193193

194-
int _tryParse32BitProto3(String s, JsonParsingContext context) {
195-
return int.tryParse(s) ??
196-
(throw context.parseException('expected integer', s));
197-
}
198-
199-
int _check32BitSignedProto3(int n, JsonParsingContext context) {
200-
if (n < -2147483648 || n > 2147483647) {
201-
throw context.parseException('expected 32 bit signed integer', n);
202-
}
203-
return n;
204-
}
205-
206-
int _check32BitUnsignedProto3(int n, JsonParsingContext context) {
207-
if (n < 0 || n > 0xFFFFFFFF) {
208-
throw context.parseException('expected 32 bit unsigned integer', n);
209-
}
210-
return n;
211-
}
212-
213-
Int64 _tryParse64BitProto3(Object? json, String s, JsonParsingContext context) {
214-
try {
215-
return Int64.parseInt(s);
216-
} on FormatException {
217-
throw context.parseException('expected integer', json);
218-
}
219-
}
220-
221194
/// TODO(paulberry): find a better home for this?
222195
extension _FindFirst<E> on Iterable<E> {
223196
E? findFirst(bool Function(E) test) {
@@ -384,36 +357,36 @@ void _mergeFromProto3JsonWithContext(
384357
if (value is int) {
385358
result = value;
386359
} else if (value is String) {
387-
result = _tryParse32BitProto3(value, context);
360+
result = Proto3ParseUtils.tryParse32Bit(value, context);
388361
} else {
389362
throw context.parseException(
390363
'Expected int or stringified int',
391364
value,
392365
);
393366
}
394-
return _check32BitUnsignedProto3(result, context);
367+
return Proto3ParseUtils.check32BitUnsigned(result, context);
395368
case PbFieldType.INT32_BIT:
396369
case PbFieldType.SINT32_BIT:
397370
case PbFieldType.SFIXED32_BIT:
398371
int result;
399372
if (value is int) {
400373
result = value;
401374
} else if (value is String) {
402-
result = _tryParse32BitProto3(value, context);
375+
result = Proto3ParseUtils.tryParse32Bit(value, context);
403376
} else {
404377
throw context.parseException(
405378
'Expected int or stringified int',
406379
value,
407380
);
408381
}
409-
_check32BitSignedProto3(result, context);
382+
Proto3ParseUtils.check32BitSigned(result, context);
410383
return result;
411384
case PbFieldType.UINT64_BIT:
412385
Int64 result;
413386
if (value is int) {
414387
result = Int64(value);
415388
} else if (value is String) {
416-
result = _tryParse64BitProto3(json, value, context);
389+
result = Proto3ParseUtils.tryParse64Bit(json, value, context);
417390
} else {
418391
throw context.parseException(
419392
'Expected int or stringified int',
@@ -469,23 +442,23 @@ void _mergeFromProto3JsonWithContext(
469442
case PbFieldType.UINT64_BIT:
470443
// TODO(sigurdm): We do not throw on negative values here.
471444
// That would probably require going via bignum.
472-
return _tryParse64BitProto3(json, key, context);
445+
return Proto3ParseUtils.tryParse64Bit(json, key, context);
473446
case PbFieldType.INT64_BIT:
474447
case PbFieldType.SINT64_BIT:
475448
case PbFieldType.SFIXED64_BIT:
476449
case PbFieldType.FIXED64_BIT:
477-
return _tryParse64BitProto3(json, key, context);
450+
return Proto3ParseUtils.tryParse64Bit(json, key, context);
478451
case PbFieldType.INT32_BIT:
479452
case PbFieldType.SINT32_BIT:
480453
case PbFieldType.FIXED32_BIT:
481454
case PbFieldType.SFIXED32_BIT:
482-
return _check32BitSignedProto3(
483-
_tryParse32BitProto3(key, context),
455+
return Proto3ParseUtils.check32BitSigned(
456+
Proto3ParseUtils.tryParse32Bit(key, context),
484457
context,
485458
);
486459
case PbFieldType.UINT32_BIT:
487-
return _check32BitUnsignedProto3(
488-
_tryParse32BitProto3(key, context),
460+
return Proto3ParseUtils.check32BitUnsigned(
461+
Proto3ParseUtils.tryParse32Bit(key, context),
489462
context,
490463
);
491464
default:

protobuf/lib/src/protobuf/utils.dart

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'package:fixnum/fixnum.dart' show Int64;
6+
57
import 'internal.dart';
8+
import 'json_parsing_context.dart';
69

710
// TODO(antonm): reconsider later if PbList should take care of equality.
811
bool deepEquals(Object lhs, Object rhs) {
@@ -53,3 +56,36 @@ class HashUtils {
5356
static int hash2(dynamic a, dynamic b) =>
5457
_finish(combine(combine(0, a.hashCode), b.hashCode));
5558
}
59+
60+
class Proto3ParseUtils {
61+
static int tryParse32Bit(String s, JsonParsingContext context) {
62+
return int.tryParse(s) ??
63+
(throw context.parseException('expected integer', s));
64+
}
65+
66+
static int check32BitSigned(int n, JsonParsingContext context) {
67+
if (n < -2147483648 || n > 2147483647) {
68+
throw context.parseException('expected 32 bit signed integer', n);
69+
}
70+
return n;
71+
}
72+
73+
static int check32BitUnsigned(int n, JsonParsingContext context) {
74+
if (n < 0 || n > 0xFFFFFFFF) {
75+
throw context.parseException('expected 32 bit unsigned integer', n);
76+
}
77+
return n;
78+
}
79+
80+
static Int64 tryParse64Bit(
81+
Object? json,
82+
String s,
83+
JsonParsingContext context,
84+
) {
85+
try {
86+
return Int64.parseInt(s);
87+
} on FormatException {
88+
throw context.parseException('expected integer', json);
89+
}
90+
}
91+
}

protobuf/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: protobuf
2-
version: 5.0.0
2+
version: 5.1.0-wip
33
description: >-
44
Runtime library for protocol buffers support. Use with package:protoc_plugin
55
to generate Dart code for your '.proto' files.

protoc_plugin/test/generated_message_test.dart

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -268,16 +268,27 @@ void main() {
268268
}
269269
}
270270

271-
final List<int> data64 = makeRecursiveMessage(64).writeToBuffer();
272-
final List<int> data65 = makeRecursiveMessage(65).writeToBuffer();
273-
274-
assertMessageDepth(TestRecursiveMessage.fromBuffer(data64), 64);
271+
// Message with exactly `DEFAULT_RECURSION_LIMIT` levels of nesting.
272+
final List<int> dataShallow =
273+
makeRecursiveMessage(
274+
CodedBufferReader.DEFAULT_RECURSION_LIMIT,
275+
).writeToBuffer();
276+
// Message with more than `DEFAULT_RECURSION_LIMIT` levels of nesting.
277+
final List<int> dataDeep =
278+
makeRecursiveMessage(
279+
CodedBufferReader.DEFAULT_RECURSION_LIMIT + 1,
280+
).writeToBuffer();
281+
282+
assertMessageDepth(
283+
TestRecursiveMessage.fromBuffer(dataShallow),
284+
CodedBufferReader.DEFAULT_RECURSION_LIMIT,
285+
);
275286

276287
expect(() {
277-
TestRecursiveMessage.fromBuffer(data65);
288+
TestRecursiveMessage.fromBuffer(dataDeep);
278289
}, throwsInvalidProtocolBufferException);
279290

280-
final input = CodedBufferReader(data64, recursionLimit: 8);
291+
final input = CodedBufferReader(dataShallow, recursionLimit: 8);
281292
expect(() {
282293
// Uncomfortable alternative to below...
283294
TestRecursiveMessage().mergeFromCodedBufferReader(input);

0 commit comments

Comments
 (0)