Skip to content

Handle non-string keys returning immediate values via to_s #792

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 27, 2025
Merged

Handle non-string keys returning immediate values via to_s #792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Changes

* Handle non-string hash keys with broken `to_s` implementations.

### 2025-04-25 (2.11.3)

* Fix a regression in `JSON.pretty_generate` that could cause indentation to be off once some `#to_json` has been called.
Expand Down
17 changes: 16 additions & 1 deletion ext/json/ext/generator/generator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,21 @@ struct hash_foreach_arg {
int iter;
};

static VALUE
convert_string_subclass(VALUE key)
{
VALUE key_to_s = rb_funcall(key, i_to_s, 0);

if (RB_UNLIKELY(rb_type(key_to_s) != T_STRING)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RB_TYPE_P() might be faster: https://github.com/ruby/ruby/blob/3ec7bfff2e7ac4f6f69d26676edcfd2e73ea3b05/include/ruby/internal/value_type.h#L376

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah it doesn't handle T_STRING explicitly currently, so I guess it's the same, but maybe in the future it might?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an unlucky path, so I don't think it matter too much, but it's nicer to read, so I changed it.

VALUE cname = rb_obj_class(key);
rb_raise(rb_eTypeError,
"can't convert %"PRIsVALUE" to %s (%"PRIsVALUE"#%s gives %"PRIsVALUE")",
cname, "String", cname, "to_s", rb_obj_class(key_to_s));
}

return key_to_s;
}

static int
json_object_i(VALUE key, VALUE val, VALUE _arg)
{
Expand Down Expand Up @@ -817,7 +832,7 @@ json_object_i(VALUE key, VALUE val, VALUE _arg)
if (RB_LIKELY(RBASIC_CLASS(key) == rb_cString)) {
key_to_s = key;
} else {
key_to_s = rb_funcall(key, i_to_s, 0);
key_to_s = convert_string_subclass(key);
}
break;
case T_SYMBOL:
Expand Down
16 changes: 16 additions & 0 deletions test/json/json_generator_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,22 @@ def test_string_subclass_with_to_s
assert_equal '{"JSONGeneratorTest::StringWithToS#to_s":1}', JSON.generate(StringWithToS.new => 1)
end

def test_string_subclass_with_broken_to_s
klass = Class.new(String) do
def to_s
false
end
end
s = klass.new("test")
assert_equal '["test"]', JSON.generate([s])

omit("Can't figure out how to match behavior in java code") if RUBY_PLATFORM == "java"

assert_raise TypeError do
JSON.generate(s => 1)
end
end

if defined?(JSON::Ext::Generator) and RUBY_PLATFORM != "java"
def test_valid_utf8_in_different_encoding
utf8_string = "€™"
Expand Down