Ruby Oneof Fields WellKnownType Fields with Default Values Don't Decode Properly

[TedStudley]

What version of protobuf and what language are you using?
Language: Ruby 2.6.2
Version: google-protobuf-3.11.4-x86_64-linux

What did you do?
Steps to reproduce the behavior:

1. Generate ruby code for the following message:

syntax = "proto3";
package sample;

message SampleMessage {
  oneof value {
    google.protobuf.StringValue string = 1;
    google.protobuf.Int32Value int = 2;
  }
}

2. Initialize a message with a default value (empty string or zero) for either of the oneof fields:

msg = Sample::SampleMessage.new(string: Google::Protobuf::StringValue.new(value: ""))
# #<Sample::SampleMessage string: <Google::Protobuf::StringValue value: "">, int: nil>

3. Encode the message to protobuf:

msg.to_proto
# "\x0a\x00"

4. Decode the resulting bytes

Sample::SampleMessage.decode("\x0a\x00")
# #<Sample::FooDto:0x2ad3d4d00b38>

What did you expect to see
The decoded protobuf should be identical to the message before encoding.

What did you see instead?
The decoded protobuf is left in an inconsistent state without the oneof value having been initialized. Attempting to access the string value results in a confusing exception:

Sample::SampleMessage.decode("\x0a\x00").string
# Google::Protobuf::TypeError: Invalid argument for string field 'value' (given FalseClass). 

Anything else we should know about your project / environment
This issue only appears when using well-known types StringWrapper, Int32Wrapper, Int64Wrapper, and FloatWrapper. BoolWrapper is not affected, for whatever reason. Similarly, substituting the Google protobuf wrappers with home-grown wrappers (e.g., a simple message with a single primitive field) does not exhibit this issue.

Attempting to decode the slightly-modified protobuf format "\x0a\x02\x0a\x00" does result in the expected message, which leads me to believe that the issue lies specifically in attempting to populate a default (0-byte) value for a well-known type when decoding a oneof field.

We were able to get around this issue in our project by updating our oneof to use only primitive fields rather than wrappers (the wrappers are pointless in this case because we can't set a oneof to nil), but this still seems like undesirable behavior.

[haberman]

This should be fixed by #8184, which is released in 3.15.0-rc1 and after. Please give it a try and let us know if you're still seeing the problem.

I'm not sure why our tests didn't catch this problem, as we do have some tests for wrappers in a oneof:

protobuf/ruby/tests/common_tests.rb

Lines 1392 to 1427 in aee143a

	def test_oneof_wrappers
	run_test = ->(m) {
	serialized = proto_module::Wrapper::encode(m)
	m2 = proto_module::Wrapper::decode(serialized)
	# Encode directly from lazy form.
	serialized2 = proto_module::Wrapper::encode(m2)
	assert_equal m, m2
	assert_equal serialized, serialized2
	serialized_json = proto_module::Wrapper::encode_json(m)
	m3 = proto_module::Wrapper::decode_json(serialized_json)
	assert_equal m, m3
	}
	m = proto_module::Wrapper.new()
	run_test.call(m)
	m.oneof_double_as_value = 2.0
	run_test.call(m)
	m.oneof_float_as_value = 4.0
	run_test.call(m)
	m.oneof_int32_as_value = 3
	run_test.call(m)
	m.oneof_int64_as_value = 5
	run_test.call(m)
	m.oneof_uint32_as_value = 6
	run_test.call(m)
	m.oneof_uint64_as_value = 7
	run_test.call(m)
	m.oneof_string_as_value = 'str'
	run_test.call(m)
	m.oneof_bytes_as_value = 'fun'
	run_test.call(m)
	end

In any case, the new code as of #8184 is much simpler and better tested, so I expect that this issue is fixed in that PR.