diff --git a/protobuf/CHANGELOG.md b/protobuf/CHANGELOG.md index 2874f129..5b911adb 100644 --- a/protobuf/CHANGELOG.md +++ b/protobuf/CHANGELOG.md @@ -1,3 +1,12 @@ +## 5.1.0-wip + +* Update default size limit of `CodedBufferReader` from 67,108,864 bytes to + 2,147,483,647 bytes, and default recursion limit from 64 to 100. + + The new limits are consistent with the Java and C++ implementations. ([#1060]) + +[#1060]: https://github.com/google/protobuf.dart/pull/1060 + ## 5.0.0 * Improve performance of `GeneratedMessage.deepCopy`. ([#742]) diff --git a/protobuf/lib/src/protobuf/coded_buffer_reader.dart b/protobuf/lib/src/protobuf/coded_buffer_reader.dart index bb1f5855..28d26994 100644 --- a/protobuf/lib/src/protobuf/coded_buffer_reader.dart +++ b/protobuf/lib/src/protobuf/coded_buffer_reader.dart @@ -8,9 +8,10 @@ part of 'internal.dart'; /// [GeneratedMessage]s. class CodedBufferReader { // ignore: constant_identifier_names - static const int DEFAULT_RECURSION_LIMIT = 64; + static const int DEFAULT_RECURSION_LIMIT = 100; + // Maximum value of a 32-bit signed integer. // ignore: constant_identifier_names - static const int DEFAULT_SIZE_LIMIT = 64 << 20; + static const int DEFAULT_SIZE_LIMIT = (1 << 31) - 1; final Uint8List _buffer; diff --git a/protobuf/lib/src/protobuf/coded_buffer_writer.dart b/protobuf/lib/src/protobuf/coded_buffer_writer.dart index a01e633e..8d0e198b 100644 --- a/protobuf/lib/src/protobuf/coded_buffer_writer.dart +++ b/protobuf/lib/src/protobuf/coded_buffer_writer.dart @@ -64,11 +64,11 @@ class CodedBufferWriter { _commitChunk(true); } - void writeField(int fieldNumber, int fieldType, Object? fieldValue) { + void writeField(int fieldNumber, int fieldType, dynamic fieldValue) { final valueType = PbFieldType.baseType(fieldType); if ((fieldType & PbFieldType.PACKED_BIT) != 0) { - final list = fieldValue as List; + final List list = fieldValue; if (list.isNotEmpty) { _writeTag(fieldNumber, WIRETYPE_LENGTH_DELIMITED); final mark = _startLengthDelimited(); @@ -81,7 +81,7 @@ class CodedBufferWriter { } if ((fieldType & PbFieldType.MAP_BIT) != 0) { - final map = fieldValue as PbMap; + final PbMap map = fieldValue; final keyWireFormat = _wireTypes[_valueTypeIndex(map.keyFieldType)]; final valueWireFormat = _wireTypes[_valueTypeIndex(map.valueFieldType)]; @@ -108,7 +108,7 @@ class CodedBufferWriter { final wireFormat = _wireTypes[_valueTypeIndex(valueType)]; if ((fieldType & PbFieldType.REPEATED_BIT) != 0) { - final list = fieldValue as List; + final List list = fieldValue; for (var i = 0; i < list.length; i++) { _writeValue(fieldNumber, valueType, list[i], wireFormat); } diff --git a/protobuf/lib/src/protobuf/exceptions.dart b/protobuf/lib/src/protobuf/exceptions.dart index f55e9be0..cb1852dc 100644 --- a/protobuf/lib/src/protobuf/exceptions.dart +++ b/protobuf/lib/src/protobuf/exceptions.dart @@ -35,7 +35,8 @@ class InvalidProtocolBufferException implements Exception { InvalidProtocolBufferException.recursionLimitExceeded() : this._(''' Protocol message had too many levels of nesting. May be malicious. -Use CodedBufferReader.setRecursionLimit() to increase the depth limit. +Use a CodedBufferReader with a defined recursion depth limit if you need to +parse deeply nested messages. '''); InvalidProtocolBufferException.truncatedMessage() diff --git a/protobuf/lib/src/protobuf/field_type.dart b/protobuf/lib/src/protobuf/field_type.dart index 67b3194e..ea08419d 100644 --- a/protobuf/lib/src/protobuf/field_type.dart +++ b/protobuf/lib/src/protobuf/field_type.dart @@ -7,23 +7,18 @@ part of 'internal.dart'; /// Defines constants and functions for dealing with fieldType bits. class PbFieldType { - static bool isRepeated(int fieldType) => - (fieldType & PbFieldType.REPEATED_BIT) != 0; + static bool isRepeated(int fieldType) => (fieldType & REPEATED_BIT) != 0; - static bool isRequired(int fieldType) => - (fieldType & PbFieldType.REQUIRED_BIT) != 0; + static bool isRequired(int fieldType) => (fieldType & REQUIRED_BIT) != 0; - static bool isEnum(int fieldType) => - PbFieldType.baseType(fieldType) == PbFieldType.ENUM_BIT; + static bool isEnum(int fieldType) => baseType(fieldType) == ENUM_BIT; - static bool isBytes(int fieldType) => - PbFieldType.baseType(fieldType) == PbFieldType.BYTES_BIT; + static bool isBytes(int fieldType) => baseType(fieldType) == BYTES_BIT; static bool isGroupOrMessage(int fieldType) => - (fieldType & (PbFieldType.GROUP_BIT | PbFieldType.MESSAGE_BIT)) != 0; + (fieldType & (GROUP_BIT | MESSAGE_BIT)) != 0; - static bool isMapField(int fieldType) => - (fieldType & PbFieldType.MAP_BIT) != 0; + static bool isMapField(int fieldType) => (fieldType & MAP_BIT) != 0; /// Returns the base field type without any of the required, repeated /// and packed bits. @@ -176,6 +171,7 @@ class PbFieldType { static const int PACKED_SFIXED64 = REPEATED_BIT | PACKED_BIT | SFIXED64_BIT; static const int MAP = MAP_BIT | MESSAGE_BIT; + // Short names for use in generated code. // _O_ptional. diff --git a/protobuf/lib/src/protobuf/proto3_json.dart b/protobuf/lib/src/protobuf/proto3_json.dart index 760f8738..5da05315 100644 --- a/protobuf/lib/src/protobuf/proto3_json.dart +++ b/protobuf/lib/src/protobuf/proto3_json.dart @@ -191,33 +191,6 @@ Object? _writeToProto3Json(FieldSet fs, TypeRegistry typeRegistry) { return result; } -int _tryParse32BitProto3(String s, JsonParsingContext context) { - return int.tryParse(s) ?? - (throw context.parseException('expected integer', s)); -} - -int _check32BitSignedProto3(int n, JsonParsingContext context) { - if (n < -2147483648 || n > 2147483647) { - throw context.parseException('expected 32 bit signed integer', n); - } - return n; -} - -int _check32BitUnsignedProto3(int n, JsonParsingContext context) { - if (n < 0 || n > 0xFFFFFFFF) { - throw context.parseException('expected 32 bit unsigned integer', n); - } - return n; -} - -Int64 _tryParse64BitProto3(Object? json, String s, JsonParsingContext context) { - try { - return Int64.parseInt(s); - } on FormatException { - throw context.parseException('expected integer', json); - } -} - /// TODO(paulberry): find a better home for this? extension _FindFirst on Iterable { E? findFirst(bool Function(E) test) { @@ -384,14 +357,14 @@ void _mergeFromProto3JsonWithContext( if (value is int) { result = value; } else if (value is String) { - result = _tryParse32BitProto3(value, context); + result = Proto3ParseUtils.tryParse32Bit(value, context); } else { throw context.parseException( 'Expected int or stringified int', value, ); } - return _check32BitUnsignedProto3(result, context); + return Proto3ParseUtils.check32BitUnsigned(result, context); case PbFieldType.INT32_BIT: case PbFieldType.SINT32_BIT: case PbFieldType.SFIXED32_BIT: @@ -399,21 +372,21 @@ void _mergeFromProto3JsonWithContext( if (value is int) { result = value; } else if (value is String) { - result = _tryParse32BitProto3(value, context); + result = Proto3ParseUtils.tryParse32Bit(value, context); } else { throw context.parseException( 'Expected int or stringified int', value, ); } - _check32BitSignedProto3(result, context); + Proto3ParseUtils.check32BitSigned(result, context); return result; case PbFieldType.UINT64_BIT: Int64 result; if (value is int) { result = Int64(value); } else if (value is String) { - result = _tryParse64BitProto3(json, value, context); + result = Proto3ParseUtils.tryParse64Bit(json, value, context); } else { throw context.parseException( 'Expected int or stringified int', @@ -469,23 +442,23 @@ void _mergeFromProto3JsonWithContext( case PbFieldType.UINT64_BIT: // TODO(sigurdm): We do not throw on negative values here. // That would probably require going via bignum. - return _tryParse64BitProto3(json, key, context); + return Proto3ParseUtils.tryParse64Bit(json, key, context); case PbFieldType.INT64_BIT: case PbFieldType.SINT64_BIT: case PbFieldType.SFIXED64_BIT: case PbFieldType.FIXED64_BIT: - return _tryParse64BitProto3(json, key, context); + return Proto3ParseUtils.tryParse64Bit(json, key, context); case PbFieldType.INT32_BIT: case PbFieldType.SINT32_BIT: case PbFieldType.FIXED32_BIT: case PbFieldType.SFIXED32_BIT: - return _check32BitSignedProto3( - _tryParse32BitProto3(key, context), + return Proto3ParseUtils.check32BitSigned( + Proto3ParseUtils.tryParse32Bit(key, context), context, ); case PbFieldType.UINT32_BIT: - return _check32BitUnsignedProto3( - _tryParse32BitProto3(key, context), + return Proto3ParseUtils.check32BitUnsigned( + Proto3ParseUtils.tryParse32Bit(key, context), context, ); default: diff --git a/protobuf/lib/src/protobuf/utils.dart b/protobuf/lib/src/protobuf/utils.dart index b4bb8bce..7de5c1e7 100644 --- a/protobuf/lib/src/protobuf/utils.dart +++ b/protobuf/lib/src/protobuf/utils.dart @@ -2,7 +2,10 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. +import 'package:fixnum/fixnum.dart' show Int64; + import 'internal.dart'; +import 'json_parsing_context.dart'; // TODO(antonm): reconsider later if PbList should take care of equality. bool deepEquals(Object lhs, Object rhs) { @@ -53,3 +56,36 @@ class HashUtils { static int hash2(dynamic a, dynamic b) => _finish(combine(combine(0, a.hashCode), b.hashCode)); } + +class Proto3ParseUtils { + static int tryParse32Bit(String s, JsonParsingContext context) { + return int.tryParse(s) ?? + (throw context.parseException('expected integer', s)); + } + + static int check32BitSigned(int n, JsonParsingContext context) { + if (n < -2147483648 || n > 2147483647) { + throw context.parseException('expected 32 bit signed integer', n); + } + return n; + } + + static int check32BitUnsigned(int n, JsonParsingContext context) { + if (n < 0 || n > 0xFFFFFFFF) { + throw context.parseException('expected 32 bit unsigned integer', n); + } + return n; + } + + static Int64 tryParse64Bit( + Object? json, + String s, + JsonParsingContext context, + ) { + try { + return Int64.parseInt(s); + } on FormatException { + throw context.parseException('expected integer', json); + } + } +} diff --git a/protobuf/pubspec.yaml b/protobuf/pubspec.yaml index 162dadcc..84b70d09 100644 --- a/protobuf/pubspec.yaml +++ b/protobuf/pubspec.yaml @@ -1,5 +1,5 @@ name: protobuf -version: 5.0.0 +version: 5.1.0-wip description: >- Runtime library for protocol buffers support. Use with package:protoc_plugin to generate Dart code for your '.proto' files. diff --git a/protoc_plugin/test/generated_message_test.dart b/protoc_plugin/test/generated_message_test.dart index 1a769a32..1f62b9e3 100644 --- a/protoc_plugin/test/generated_message_test.dart +++ b/protoc_plugin/test/generated_message_test.dart @@ -268,16 +268,27 @@ void main() { } } - final List data64 = makeRecursiveMessage(64).writeToBuffer(); - final List data65 = makeRecursiveMessage(65).writeToBuffer(); - - assertMessageDepth(TestRecursiveMessage.fromBuffer(data64), 64); + // Message with exactly `DEFAULT_RECURSION_LIMIT` levels of nesting. + final List dataShallow = + makeRecursiveMessage( + CodedBufferReader.DEFAULT_RECURSION_LIMIT, + ).writeToBuffer(); + // Message with more than `DEFAULT_RECURSION_LIMIT` levels of nesting. + final List dataDeep = + makeRecursiveMessage( + CodedBufferReader.DEFAULT_RECURSION_LIMIT + 1, + ).writeToBuffer(); + + assertMessageDepth( + TestRecursiveMessage.fromBuffer(dataShallow), + CodedBufferReader.DEFAULT_RECURSION_LIMIT, + ); expect(() { - TestRecursiveMessage.fromBuffer(data65); + TestRecursiveMessage.fromBuffer(dataDeep); }, throwsInvalidProtocolBufferException); - final input = CodedBufferReader(data64, recursionLimit: 8); + final input = CodedBufferReader(dataShallow, recursionLimit: 8); expect(() { // Uncomfortable alternative to below... TestRecursiveMessage().mergeFromCodedBufferReader(input);