From 045e58d8310890c43bf48a3b9ed6062b781b52c2 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 31 Oct 2024 08:52:19 +0100 Subject: [PATCH] JSON.generate: call to_json on String subclasses Fix: https://github.com/ruby/json/issues/667 This is yet another behavior on which the various implementations differed, but the C implementation used to call `to_json` on String subclasses used as keys. This was optimized out in e125072130229e54a651f7b11d7d5a782ae7fb65 but there is an Active Support test case for it, so it's best to make all 3 implementation respect this behavior. --- CHANGES.md | 2 ++ ext/json/ext/fbuffer/fbuffer.h | 4 +++ ext/json/ext/generator/generator.c | 6 +++- java/src/json/ext/Generator.java | 9 +++++- lib/json/pure/generator.rb | 33 +++++++++++++++----- test/json/json_generator_test.rb | 50 ++++++++++++++++++++++++++++++ 6 files changed, 95 insertions(+), 9 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 667958ea..1cab01ee 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,7 @@ # Changes +* Fix a regression in JSON.generate when dealing with Hash keys that are string subclasses, call `to_json` on them. + ### 2024-10-25 (2.7.5) * Fix a memory leak when `#to_json` methods raise an exception. diff --git a/ext/json/ext/fbuffer/fbuffer.h b/ext/json/ext/fbuffer/fbuffer.h index 76bd6ce1..4c62aa36 100644 --- a/ext/json/ext/fbuffer/fbuffer.h +++ b/ext/json/ext/fbuffer/fbuffer.h @@ -36,6 +36,10 @@ static VALUE fbuffer_to_s(FBuffer *fb); #define RB_UNLIKELY(expr) expr #endif +#ifndef RB_LIKELY +#define RB_LIKELY(expr) expr +#endif + static FBuffer *fbuffer_alloc(unsigned long initial_length) { FBuffer *fb; diff --git a/ext/json/ext/generator/generator.c b/ext/json/ext/generator/generator.c index ca48e0f1..ca9f1a9c 100644 --- a/ext/json/ext/generator/generator.c +++ b/ext/json/ext/generator/generator.c @@ -678,7 +678,11 @@ json_object_i(VALUE key, VALUE val, VALUE _arg) break; } - generate_json_string(buffer, Vstate, state, key_to_s); + if (RB_LIKELY(RBASIC_CLASS(key_to_s) == rb_cString)) { + generate_json_string(buffer, Vstate, state, key_to_s); + } else { + generate_json(buffer, Vstate, state, key_to_s); + } if (RB_UNLIKELY(state->space_before)) fbuffer_append(buffer, state->space_before, state->space_before_len); fbuffer_append_char(buffer, ':'); if (RB_UNLIKELY(state->space)) fbuffer_append(buffer, state->space, state->space_len); diff --git a/lib/json/pure/generator.rb b/lib/json/pure/generator.rb index 516ee287..0a563813 100644 --- a/lib/json/pure/generator.rb +++ b/lib/json/pure/generator.rb @@ -301,19 +301,30 @@ def generate(obj) # Handles @allow_nan, @buffer_initial_length, other ivars must be the default value (see above) private def generate_json(obj, buf) - case obj - when Hash + klass = obj.class + if klass == Hash buf << '{' first = true obj.each_pair do |k,v| buf << ',' unless first - fast_serialize_string(k.to_s, buf) + + key_str = k.to_s + if key_str.is_a?(::String) + if key_str.class == ::String + fast_serialize_string(key_str, buf) + else + generate_json(key_str, buf) + end + else + raise TypeError, "#{k.class}#to_s returns an instance of #{key_str.class}, expected a String" + end + buf << ':' generate_json(v, buf) first = false end buf << '}' - when Array + elsif klass == Array buf << '[' first = true obj.each do |e| @@ -322,9 +333,9 @@ def generate(obj) first = false end buf << ']' - when String + elsif klass == String fast_serialize_string(obj, buf) - when Integer + elsif klass == Integer buf << obj.to_s else # Note: Float is handled this way since Float#to_s is slow anyway @@ -414,7 +425,15 @@ def json_transform(state) each { |key, value| result << delim unless first result << state.indent * depth if indent - result = +"#{result}#{key.to_s.to_json(state)}#{state.space_before}:#{state.space}" + + key_str = key.to_s + key_json = if key_str.is_a?(::String) + key_str = key_str.to_json(state) + else + raise TypeError, "#{key.class}#to_s returns an instance of #{key_str.class}, expected a String" + end + + result = +"#{result}#{key_json}#{state.space_before}:#{state.space}" if state.strict? && !(false == value || true == value || nil == value || String === value || Array === value || Hash === value || Integer === value || Float === value) raise GeneratorError, "#{value.class} not allowed in JSON" elsif value.respond_to?(:to_json) diff --git a/test/json/json_generator_test.rb b/test/json/json_generator_test.rb index a83d730d..8e76bec3 100755 --- a/test/json/json_generator_test.rb +++ b/test/json/json_generator_test.rb @@ -479,6 +479,56 @@ def test_invalid_encoding_string end end + class MyCustomString < String + def to_json(_state = nil) + '"my_custom_key"' + end + + def to_s + self + end + end + + def test_string_subclass_as_keys + # Ref: https://github.com/ruby/json/issues/667 + # if key.to_s doesn't return a bare string, we call `to_json` on it. + key = MyCustomString.new("won't be used") + assert_equal '{"my_custom_key":1}', JSON.generate(key => 1) + end + + class FakeString + def to_json(_state = nil) + raise "Shouldn't be called" + end + + def to_s + self + end + end + + def test_custom_object_as_keys + key = FakeString.new + error = assert_raise(TypeError) do + JSON.generate(key => 1) + end + assert_match "FakeString", error.message + end + + def test_to_json_called_with_state_object + object = Object.new + called = false + argument = nil + object.singleton_class.define_method(:to_json) do |state| + called = true + argument = state + "" + end + + assert_equal "", JSON.dump(object) + assert called, "#to_json wasn't called" + assert_instance_of JSON::State, argument + end + if defined?(JSON::Ext::Generator) and RUBY_PLATFORM != "java" def test_valid_utf8_in_different_encoding utf8_string = "€™"