From 2f7614744b7d027688b371dbff650270e56a88c9 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 5 Nov 2024 14:13:42 +0100 Subject: [PATCH] Raise JSON::GeneratorError instead of Encoding::UndefinedConversionError Followup: https://github.com/ruby/json/pull/633 That's what was raised historically. You could argue that this new exception is more precise, but I've encountered some real production code that expected the old behavior and that was broken by this change. --- ext/json/ext/generator/generator.c | 4 ++++ java/src/json/ext/Generator.java | 20 +++++++++++++------- lib/json/pure/generator.rb | 12 ++++++++++-- test/json/json_generator_test.rb | 14 +++++++++++--- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/ext/json/ext/generator/generator.c b/ext/json/ext/generator/generator.c index 09fdd3a1..a04df36d 100644 --- a/ext/json/ext/generator/generator.c +++ b/ext/json/ext/generator/generator.c @@ -954,6 +954,10 @@ static VALUE generate_json_rescue(VALUE d, VALUE exc) struct generate_json_data *data = (struct generate_json_data *)d; fbuffer_free(data->buffer); + if (RBASIC_CLASS(exc) == rb_path2class("Encoding::UndefinedConversionError")) { + exc = rb_exc_new_str(eGeneratorError, rb_funcall(exc, rb_intern("message"), 0)); + } + rb_exc_raise(exc); return Qundef; diff --git a/java/src/json/ext/Generator.java b/java/src/json/ext/Generator.java index d818ce49..3c8247b9 100644 --- a/java/src/json/ext/Generator.java +++ b/java/src/json/ext/Generator.java @@ -17,6 +17,7 @@ import org.jruby.runtime.ThreadContext; import org.jruby.runtime.builtin.IRubyObject; import org.jruby.util.ByteList; +import org.jruby.exceptions.RaiseException; public final class Generator { private Generator() { @@ -390,14 +391,19 @@ void generate(Session session, RubyString object, ByteList buffer) { RuntimeInfo info = session.getInfo(); RubyString src; - if (object.encoding(session.getContext()) != info.utf8.get()) { - src = (RubyString)object.encode(session.getContext(), - info.utf8.get()); - } else { - src = object; - } + try { + if (object.encoding(session.getContext()) != info.utf8.get()) { + src = (RubyString)object.encode(session.getContext(), + info.utf8.get()); + } else { + src = object; + } - session.getStringEncoder().encode(src.getByteList(), buffer); + session.getStringEncoder().encode(src.getByteList(), buffer); + } catch (RaiseException re) { + throw Utils.newException(session.getContext(), Utils.M_GENERATOR_ERROR, + re.getMessage()); + } } }; diff --git a/lib/json/pure/generator.rb b/lib/json/pure/generator.rb index 0a563813..0a5c7856 100644 --- a/lib/json/pure/generator.rb +++ b/lib/json/pure/generator.rb @@ -347,7 +347,11 @@ def generate(obj) if Regexp.method_defined?(:match?) private def fast_serialize_string(string, buf) # :nodoc: buf << '"' - string = string.encode(::Encoding::UTF_8) unless string.encoding == ::Encoding::UTF_8 + begin + string = string.encode(::Encoding::UTF_8) unless string.encoding == ::Encoding::UTF_8 + rescue Encoding::UndefinedConversionError => error + raise GeneratorError, error.message + end raise GeneratorError, "source sequence is illegal/malformed utf-8" unless string.valid_encoding? if /["\\\x0-\x1f]/n.match?(string) @@ -536,7 +540,11 @@ def to_json(state = nil, *args) end string = self else - string = encode(::Encoding::UTF_8) + begin + string = encode(::Encoding::UTF_8) + rescue Encoding::UndefinedConversionError => error + raise GeneratorError, error.message + end end if state.ascii_only? %("#{JSON.utf8_to_json_ascii(string, state.script_safe)}") diff --git a/test/json/json_generator_test.rb b/test/json/json_generator_test.rb index f30f9cf4..c829e44c 100755 --- a/test/json/json_generator_test.rb +++ b/test/json/json_generator_test.rb @@ -470,12 +470,20 @@ def test_invalid_encoding_string end assert_includes error.message, "source sequence is illegal/malformed utf-8" - assert_raise(Encoding::UndefinedConversionError) do + assert_raise(JSON::GeneratorError) do + JSON.dump("\x82\xAC\xEF".b) + end + + assert_raise(JSON::GeneratorError) do "\x82\xAC\xEF".b.to_json end - assert_raise(Encoding::UndefinedConversionError) do - JSON.dump("\x82\xAC\xEF".b) + assert_raise(JSON::GeneratorError) do + ["\x82\xAC\xEF".b].to_json + end + + assert_raise(JSON::GeneratorError) do + { foo: "\x82\xAC\xEF".b }.to_json end end