Skip to content

Commit

Permalink
Fix TypeError when passing an instance of a subclass of String to a s…
Browse files Browse the repository at this point in the history
…tring field (#13818)

## Issue
When an object that is an instance of a string-derived class is passed to a string field in a protobuf message in Ruby, it results in a `Google::Protobuf::TypeError`.

### Steps to reproduce
```rb
~/src/github.com/protocolbuffers/protobuf/ruby/tests ❯❯❯ irb -I .
irb(main):001:0> require 'basic_test_pb'
=> true
irb(main):002:0> myString = Class.new(String)
=> #<Class:0x00000001531540d8>
irb(main):003:0> str = myString.new("foo")
=> "foo"
irb(main):004:0> BasicTest::TestMessage.new(optional_string: "foo")
=> <BasicTest::TestMessage: optional_string: "foo", repeated_int32: [], repeated_int64: [], repeated_uint32: [], repeated_uint64: [], repeated_bool: [], repeated_float: [], repeated_double: [], repeated_string: [], repeated_bytes: [], repeated_msg: [], repeated_enum: []>
irb(main):005:0> BasicTest::TestMessage.new(optional_string: str)
(irb):5:in `initialize': Invalid argument for string field 'optional_string' (given #<Class:0x00000001531540d8>). (Google::Protobuf::TypeError)
irb(main):006:0>
```

## Fix
The issue appears to be caused by the field checking mechanism not properly handling instances of classes that inherit from basic types like String. My proposed solution is to improve the type checking for string fields to consider not just String instances but also instances of subclasses of String.

## Impact
The changes will allow instances of classes derived from String to be passed to string fields without any error.
This is a backwards-compatible change and will not affect the existing behaviour with standard String instances.

Closes #13818

COPYBARA_INTEGRATE_REVIEW=#13818 from NotFounds:support-string-subclass-for-ruby 2d2796c
PiperOrigin-RevId: 590941235
  • Loading branch information
NotFounds authored and Copybara-Service committed Dec 14, 2023
1 parent 2d7376e commit 34908e2
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 1 deletion.
2 changes: 1 addition & 1 deletion ruby/ext/google/protobuf_c/convert.c
Expand Up @@ -141,7 +141,7 @@ upb_MessageValue Convert_RubyToUpb(VALUE value, const char* name,
VALUE utf8 = rb_enc_from_encoding(rb_utf8_encoding());
if (rb_obj_class(value) == rb_cSymbol) {
value = rb_funcall(value, rb_intern("to_s"), 0);
} else if (rb_obj_class(value) != rb_cString) {
} else if (!rb_obj_is_kind_of(value, rb_cString)) {
rb_raise(cTypeError,
"Invalid argument for string field '%s' (given %s).", name,
rb_class2name(CLASS_OF(value)));
Expand Down
11 changes: 11 additions & 0 deletions ruby/tests/basic.rb
Expand Up @@ -787,4 +787,15 @@ def test_oneof_fields_respond_to? # regression test for issue 9202
assert_respond_to msg, :has_d?
refute msg.has_d?
end

def test_string_subclass
str = "hello"
myString = Class.new(String)

m = proto_module::TestMessage.new(
optional_string: myString.new(str),
)

assert_equal str, m.optional_string
end
end

0 comments on commit 34908e2

Please sign in to comment.