Skip to content

Fix unknown enum handling #853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions protobuf/lib/src/protobuf/proto3_json.dart
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ void _mergeFromProto3Json(
);

void recursionHelper(Object? json, FieldSet fieldSet) {
// Convert a JSON object to proto object. Returns `null` on unknown enum
// values when [ignoreUnknownFields] is [true].
Object? convertProto3JsonValue(Object value, FieldInfo fieldInfo) {
final fieldType = fieldInfo.type;
switch (PbFieldType.baseType(fieldType)) {
Expand All @@ -184,16 +186,14 @@ void _mergeFromProto3Json(
throw context.parseException('Expected bool value', json);
case PbFieldType.BYTES_BIT:
if (value is String) {
Uint8List result;
try {
result = base64Decode(value);
return base64Decode(value);
} on FormatException {
throw context.parseException(
'Expected bytes encoded as base64 String',
json,
);
}
return result;
}
throw context.parseException(
'Expected bytes encoded as base64 String',
Expand Down Expand Up @@ -294,16 +294,14 @@ void _mergeFromProto3Json(
case PbFieldType.SFIXED64_BIT:
if (value is int) return Int64(value);
if (value is String) {
Int64 result;
try {
result = Int64.parseInt(value);
return Int64.parseInt(value);
} on FormatException {
throw context.parseException(
'Expected int or stringified int',
value,
);
}
return result;
}
throw context.parseException(
'Expected int or stringified int',
Expand Down Expand Up @@ -409,13 +407,14 @@ void _mergeFromProto3Json(
throw context.parseException('Expected a String key', subKey);
}
context.addMapIndex(subKey);
fieldValues[decodeMapKey(
subKey,
mapFieldInfo.keyFieldType,
)] = convertProto3JsonValue(
final key = decodeMapKey(subKey, mapFieldInfo.keyFieldType);
final value = convertProto3JsonValue(
subValue,
mapFieldInfo.valueFieldInfo,
);
if (value != null) {
fieldValues[key] = value;
}
context.popIndex();
});
} else {
Expand All @@ -427,7 +426,10 @@ void _mergeFromProto3Json(
for (var i = 0; i < value.length; i++) {
final entry = value[i];
context.addListIndex(i);
values.add(convertProto3JsonValue(entry, fieldInfo));
final parsedValue = convertProto3JsonValue(entry, fieldInfo);
if (parsedValue != null) {
values.add(parsedValue);
}
context.popIndex();
}
} else {
Expand All @@ -450,11 +452,15 @@ void _mergeFromProto3Json(
original.mergeFromMessage(parsedSubMessage);
}
} else {
fieldSet._setFieldUnchecked(
meta,
fieldInfo,
convertProto3JsonValue(value, fieldInfo),
);
final parsedValue = convertProto3JsonValue(value, fieldInfo);
if (parsedValue == null) {
// Unknown enum
if (!ignoreUnknownFields) {
throw context.parseException('Unknown enum value', value);
}
} else {
fieldSet._setFieldUnchecked(meta, fieldInfo, parsedValue);
}
}
context.popIndex();
});
Expand Down
47 changes: 29 additions & 18 deletions protobuf/test/json_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,21 @@ import 'package:test/test.dart';
import 'mock_util.dart' show T, mockEnumValues;

void main() {
final example =
T()
..val = 123
..str = 'hello'
..int32s.addAll(<int>[1, 2, 3]);
test('mergeFromProto3Json unknown enum fields with names', () {
final example = T();

test('testProto3JsonEnum', () {
// No enum value specified.
expect(example.hasEnm, isFalse);

// Defaults to first when it doesn't exist.
expect(example.enm, equals(mockEnumValues.first));
expect((example..mergeFromProto3Json({'enm': 'a'})).enm.name, equals('a'));

// Now it's explicitly set after merging.
expect(example.hasEnm, isTrue);

expect((example..mergeFromProto3Json({'enm': 'b'})).enm.name, equals('b'));

// "c" is not a legal enum value.
expect(
() => example..mergeFromProto3Json({'enm': 'c'}),
Expand All @@ -42,21 +41,26 @@ void main() {
),
),
);

// `example` hasn't changed.
expect(example.hasEnm, isTrue);
expect(example.enm.name, equals('b'));

// "c" is not a legal enum value, but we are ignoring unknown fields, so
// default behavior is to unset `enm`, returning the default value "a"
// `enm` value shouldn't change.
expect(
(example..mergeFromProto3Json({'enm': 'c'}, ignoreUnknownFields: true))
.enm
.name,
equals('a'),
equals('b'),
);
expect(example.hasEnm, isFalse);
expect(example.hasEnm, isTrue);
});

test('mergeFromProto3Json unknown enum fields with indices', () {
// Similar to above, but with indices.
final example = T();

// Same for index values...
expect((example..mergeFromProto3Json({'enm': 2})).enm.name, 'b');
expect(
() => example..mergeFromProto3Json({'enm': 3}),
Expand All @@ -69,34 +73,35 @@ void main() {
),
),
);

// `example` hasn't changed.
expect(example.hasEnm, isTrue);
expect(example.enm.name, equals('b'));

// "c" is not a legal enum value, but we are ignoring unknown fields, so
// default behavior is to unset `enm`, returning the default value "a"
// "c" is not a legal enum value, but we are ignoring unknown fields, so the
// value shouldn't change.
expect(
(example..mergeFromProto3Json({'enm': 3}, ignoreUnknownFields: true))
.enm
.name,
equals('a'),
equals('b'),
);
expect(example.hasEnm, isFalse);
expect(example.hasEnm, isTrue);
});

test('testWriteToJson', () {
final json = example.writeToJson();
final json = makeTestJson().writeToJson();
checkJsonMap(jsonDecode(json));
});

test('testWriteFrozenToJson', () {
final frozen = example.clone()..freeze();
final frozen = makeTestJson().clone()..freeze();
final json = frozen.writeToJson();
checkJsonMap(jsonDecode(json));
});

test('writeToJsonMap', () {
final Map m = example.writeToJsonMap();
final Map m = makeTestJson().writeToJsonMap();
checkJsonMap(m);
});

Expand Down Expand Up @@ -134,13 +139,19 @@ void main() {
});

test('testJsonMapWithUnknown', () {
final m = example.writeToJsonMap();
final m = makeTestJson().writeToJsonMap();
m['9999'] = 'world';
final t = T()..mergeFromJsonMap(m);
checkJsonMap(t.writeToJsonMap(), unknownFields: {'9999': 'world'});
});
}

T makeTestJson() =>
T()
..val = 123
..str = 'hello'
..int32s.addAll(<int>[1, 2, 3]);

void checkJsonMap(Map m, {Map<String, dynamic>? unknownFields}) {
expect(m.length, 3 + (unknownFields?.length ?? 0));
expect(m['1'], 123);
Expand Down
1 change: 1 addition & 0 deletions protoc_plugin/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ TEST_PROTO_LIST = \
entity \
enum_extension \
enum_name \
enum_test \
enums \
extend_unittest \
ExtensionEnumNameConflict \
Expand Down
16 changes: 16 additions & 0 deletions protoc_plugin/test/protos/enum_test.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// 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.

syntax = "proto3";

enum A {
X = 0;
Y = 1;
}

message Message {
A enum_field = 1;
map<int32, A> map_value_field = 2;
repeated A repeated_enum_field = 3;
}
41 changes: 41 additions & 0 deletions protoc_plugin/test/unknown_enums_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// 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:test/test.dart';

import 'gen/enum_test.pb.dart';

void main() {
// 'Z' below is an unknown enum value. Known values are 'X' and 'Y'.
group('Enum parsing in maps, lists, messages', () {
test('Parse known fields', () {
final json = {
'enumField': 'Y',
'mapValueField': {'1': 'Y'},
'repeatedEnumField': ['Y'],
};

final msg = Message();
msg.mergeFromProto3Json(json);
expect(msg.enumField, A.Y);
expect(msg.mapValueField.values.toList(), [A.Y]);
expect(msg.repeatedEnumField, [A.Y]);
});

test('Skip unknown fields', () {
final json = {
'enumField': 'Z',
'mapValueField': {'1': 'X', '2': 'Z', '3': 'Y'},
'repeatedEnumField': ['X', 'Z', 'Y'],
};

final msg = Message();
msg.enumField = A.Y;
msg.mergeFromProto3Json(json, ignoreUnknownFields: true);
expect(msg.enumField, A.Y);
expect(msg.mapValueField.values.toList(), [A.X, A.Y]);
expect(msg.repeatedEnumField, [A.X, A.Y]);
});
});
}
Loading