Raise JSON::GeneratorError instead of Encoding::UndefinedConversionError

Followup: #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.
ruby · Nov 5, 2024 · 2f76147 · 2f76147
1 parent e3a3695
commit 2f76147
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 12 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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