protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
65.65k stars 15.49k forks source link

C++: FieldMask causes OneOf invalidation #3115

Closed danshaffer closed 6 years ago

danshaffer commented 7 years ago

I'm observing what looks like FieldMask is calling mutable_foo on messages underneath oneofs, causing the oneof_case to switch and leading to data loss.

We are using protobuf v3.3.1. Here's a simple unit test that repros this issue:

oneof_mask.proto:

syntax = "proto3";

message OneOfDemo {
  int64 id = 1;

  oneof demo {
    string foo = 2;
    SubMsg bar = 3;
  }
}

message SubMsg {
  string baz = 1;
}

oneof_mask_test.cc:

TEST(OneofMaskTest, OneofMaskValueTest) {
  OneOfDemo demo;
  demo.set_id(123);
  demo.set_foo("foo");

  google::protobuf::FieldMask mask;
  mask.add_paths("foo");
  mask.add_paths("bar.baz");

  google::protobuf::util::FieldMaskUtil::TrimMessage(mask, &demo);

  EXPECT_EQ(demo.foo(), "foo");
  EXPECT_FALSE(demo.has_bar());
}

GTEST_MAIN()

And test output:

oneof_mask_test.cc:16: Failure
      Expected: demo.foo()
      Which is: ""
To be equal to: "foo"
oneof_mask_test.cc:17: Failure
Value of: demo.has_bar()
  Actual: true
Expected: false
[  FAILED  ] OneofMaskTest.OneofMaskValueTest (0 ms)
[----------] 1 test from OneofMaskTest (0 ms total)
acozzette commented 7 years ago

@danshaffer Thank you for reporting this bug. I'll take a look and try to send out a fix.

acozzette commented 6 years ago

I'll close this issue because we actually ended up fixing this in 09354db1434859a31a3c81abebcc4018d42f2715 for the 3.4 release.