Skip to content
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

When parsing store unknown enum values in unknown fields #276

Merged
merged 2 commits into from
Mar 25, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Store unknown enum values into unknown fields
This fix long standing issue of protobuf returning error when parsing
unknown enum values.

New behavior matches proto2 (even is code is generated for proto3).

This is backward-incompatible change.

Issue #233
  • Loading branch information
stepancheg committed Mar 22, 2018
commit bbf67b09d108207b4a70d1a1310d6f54360029a7
21 changes: 21 additions & 0 deletions protobuf-codegen/src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1420,6 +1420,19 @@ impl<'a> FieldGen<'a> {
FieldElem::Primitive(FieldDescriptorProto_Type::TYPE_BYTES, ..) => {
self.write_merge_from_field_message_string_bytes(w);
}
FieldElem::Enum(..) => {
let version = match field.flag {
SingularFieldFlag::WithFlag { .. } => "proto2",
SingularFieldFlag::WithoutFlag => "proto3",
};
w.write_line(&format!(
"::protobuf::rt::read_{}_enum_with_unknown_fields_into({}, is, &mut self.{}, {}, &mut self.unknown_fields)?",
version,
wire_type_var,
self.rust_name,
self.proto_field.number()
));
}
_ => {
let read_proc = format!("{}?", self.proto_type.read("is"));

Expand All @@ -1443,6 +1456,14 @@ impl<'a> FieldGen<'a> {
FieldElem::Primitive(FieldDescriptorProto_Type::TYPE_BYTES, ..) => {
self.write_merge_from_field_message_string_bytes(w);
}
FieldElem::Enum(..) => {
w.write_line(&format!(
"::protobuf::rt::read_repeated_enum_with_unknown_fields_into({}, is, &mut self.{}, {}, &mut self.unknown_fields)?",
wire_type_var,
self.rust_name,
self.proto_field.number()
));
}
_ => {
w.write_line(&format!(
"::protobuf::rt::read_repeated_{}_into({}, is, &mut self.{})?;",
Expand Down
19 changes: 19 additions & 0 deletions protobuf-test/src/common/v2/test_enum_unknown_values_preserved.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
use super::test_enum_unknown_values_preserved_pb::*;

use protobuf::*;
use test::*;

#[test]
fn unknown_values_preserved() {
let mut new = NewMessage::new();
new.set_eee(NewEnum::C);

test_serialize_deserialize("08 1e", &new);

// `OldEnum` doesn't have variant `C = 30`,
// but message still properly serialized and deserialized.

let old: OldMessage = parse_from_bytes(&hex::decode_hex("08 1e")).expect("parse");

test_serialize_deserialize("08 1e", &old);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
syntax = "proto2";

package unknown_values_preserved;

enum OldEnum {
// Add _OLD suffix to field name to avoid collision with NewEnum
UNKNOWN_OLD = 0;
A_OLD = 10;
B_OLD = 20;
}

enum NewEnum {
UNKNOWN = 0;
A = 10;
B = 20;
C = 30;
}

message OldMessage {
optional OldEnum eee = 1;
}

message NewMessage {
optional NewEnum eee = 1;
}
101 changes: 101 additions & 0 deletions protobuf/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ pub fn read_repeated_bool_into(
}

/// Read repeated `enum` field into given vec.
/// This function is no longer called from generated code, remove in 1.5.
pub fn read_repeated_enum_into<E : ProtobufEnum>(
wire_type: WireType,
is: &mut CodedInputStream,
Expand All @@ -487,6 +488,106 @@ pub fn read_repeated_enum_into<E : ProtobufEnum>(
}
}

/// Helper function to read single enum value.
#[inline]
fn read_enum_with_unknown_fields_into<E : ProtobufEnum, C>(
is: &mut CodedInputStream,
target: C,
field_number: u32,
unknown_fields: &mut UnknownFields,
) -> ProtobufResult<()>
where C : FnOnce(E)
{
let i = is.read_int32()?;
match ProtobufEnum::from_i32(i) {
Some(e) => target(e),
None => unknown_fields.add_varint(field_number, i as i64 as u64),
}
Ok(())
}

fn read_repeated_packed_enum_with_unknown_fields_into<E : ProtobufEnum>(
is: &mut CodedInputStream,
target: &mut Vec<E>,
field_number: u32,
unknown_fields: &mut UnknownFields,
) -> ProtobufResult<()> {
let len = is.read_raw_varint64()?;
let old_limit = is.push_limit(len)?;
while !is.eof()? {
read_enum_with_unknown_fields_into(is, |e| target.push(e), field_number, unknown_fields)?;
}
is.pop_limit(old_limit);
Ok(())
}

/// Read repeated `enum` field into given vec,
/// and when value is unknown store it in unknown fields
/// which matches proto2 spec.
///
/// See explanation
/// [here](https://github.com/stepancheg/rust-protobuf/issues/233#issuecomment-375142710)
pub fn read_repeated_enum_with_unknown_fields_into<E : ProtobufEnum>(
wire_type: WireType,
is: &mut CodedInputStream,
target: &mut Vec<E>,
field_number: u32,
unknown_fields: &mut UnknownFields,
) -> ProtobufResult<()> {
match wire_type {
WireTypeLengthDelimited => {
read_repeated_packed_enum_with_unknown_fields_into(
is, target, field_number, unknown_fields)
},
WireTypeVarint => {
read_enum_with_unknown_fields_into(is, |e| target.push(e), field_number, unknown_fields)
}
_ => Err(unexpected_wire_type(wire_type)),
}
}

/// Read repeated `enum` field into given vec,
/// and when value is unknown store it in unknown fields
/// which matches proto2 spec.
///
/// See explanation
/// [here](https://github.com/stepancheg/rust-protobuf/issues/233#issuecomment-375142710)
pub fn read_proto3_enum_with_unknown_fields_into<E : ProtobufEnum>(
wire_type: WireType,
is: &mut CodedInputStream,
target: &mut E,
field_number: u32,
unknown_fields: &mut UnknownFields,
) -> ProtobufResult<()>
{
if wire_type != WireType::WireTypeVarint {
return Err(unexpected_wire_type(wire_type));
}

read_enum_with_unknown_fields_into(is, |e| { *target = e }, field_number, unknown_fields)
}

/// Read repeated `enum` field into given vec,
/// and when value is unknown store it in unknown fields
/// which matches proto2 spec.
///
/// See explanation
/// [here](https://github.com/stepancheg/rust-protobuf/issues/233#issuecomment-375142710)
pub fn read_proto2_enum_with_unknown_fields_into<E : ProtobufEnum>(
wire_type: WireType,
is: &mut CodedInputStream,
target: &mut Option<E>,
field_number: u32,
unknown_fields: &mut UnknownFields,
) -> ProtobufResult<()>
{
if wire_type != WireType::WireTypeVarint {
return Err(unexpected_wire_type(wire_type));
}

read_enum_with_unknown_fields_into(is, |e| { *target = Some(e) }, field_number, unknown_fields)
}

/// Read repeated `string` field into given vec.
pub fn read_repeated_string_into(
wire_type: WireType,
Expand Down