Fix a potential issue in CodedBufferWriter (#594)
Currently `CodedBufferWriter`, when serializing a field, manually masks
the "repeated", "required", and "packed" bits to get the "base type" of
the field.
However it also needs to mask the "map" bit as the function to map the
"base type" to an array index (`_valueTypeIndex`) only works for values
that are a power of 2, i.e. there needs to be at most one bit set.
This code still works today because of the special case when the map bit
set, after getting the base type incorrectly.
In other words, when the map bit is set we compute the wire type
incorrectly, but we don't use the incorrect value.
Changed in this PR:
1. `_valueTypeIndex` now has an assertion checking that the argument is
really a power of 2.
2. `valueType` is now computed using `PbFieldType._baseType`, which
correctly masks the "map" bit.
3. `wireFormat` is moved after the special case that checks for the map
bit.
With (1) tests in protoc_compiler start to fail. Either one of (2) or
(3) fixes the issue, but I think both are improvements.
JSON serializers correctly use the `PbFieldType._baseType` function to
get the base type so no changes needed.
diff --git a/protobuf/lib/src/protobuf/coded_buffer_writer.dart b/protobuf/lib/src/protobuf/coded_buffer_writer.dart
index b1821d8..2c8f37f 100644
--- a/protobuf/lib/src/protobuf/coded_buffer_writer.dart
+++ b/protobuf/lib/src/protobuf/coded_buffer_writer.dart
@@ -65,7 +65,7 @@
}
void writeField(int fieldNumber, int fieldType, fieldValue) {
- final valueType = fieldType & ~0x07;
+ final valueType = PbFieldType._baseType(fieldType);
if ((fieldType & PbFieldType._PACKED_BIT) != 0) {
if (!fieldValue.isEmpty) {
@@ -79,8 +79,6 @@
return;
}
- final wireFormat = _wireTypes[_valueTypeIndex(valueType)];
-
if ((fieldType & PbFieldType._MAP_BIT) != 0) {
final keyWireFormat =
_wireTypes[_valueTypeIndex(fieldValue.keyFieldType)];
@@ -99,6 +97,8 @@
return;
}
+ final wireFormat = _wireTypes[_valueTypeIndex(valueType)];
+
if ((fieldType & PbFieldType._REPEATED_BIT) != 0) {
for (var i = 0; i < fieldValue.length; i++) {
_writeValue(fieldNumber, valueType, fieldValue[i], wireFormat);
@@ -445,8 +445,10 @@
/// where multiplication becomes a floating point multiplication.
///
/// [1] http://supertech.csail.mit.edu/papers/debruijn.pdf
- static int _valueTypeIndex(int powerOf2) =>
- ((0x077CB531 * powerOf2) >> 27) & 31;
+ int _valueTypeIndex(int powerOf2) {
+ assert(powerOf2 & (powerOf2 - 1) == 0, '$powerOf2 is not a power of 2');
+ return ((0x077CB531 * powerOf2) >> 27) & 31;
+ }
/// Precomputed indices for all FbFieldType._XYZ_BIT values:
///
