[Bug #20438] Disallow "%\n" and "%\0"

spec/ruby/core/string/modulo_spec.rb

@@ -55,33 +55,48 @@ describe "String#%" do
     -> { ("foo%" % [])}.should raise_error(ArgumentError)
   end
 
-  it "formats single % character before a newline as literal %" do
-    ("%\n" % []).should == "%\n"
-    ("foo%\n" % []).should == "foo%\n"
-    ("%\n.3f" % 1.2).should == "%\n.3f"
+  ruby_version_is ""..."3.4" do
+    it "formats single % character before a newline as literal %" do
+      ("%\n" % []).should == "%\n"
+      ("foo%\n" % []).should == "foo%\n"
+      ("%\n.3f" % 1.2).should == "%\n.3f"
+    end
+
+    it "formats single % character before a NUL as literal %" do
+      ("%\0" % []).should == "%\0"
+      ("foo%\0" % []).should == "foo%\0"
+      ("%\0.3f" % 1.2).should == "%\0.3f"
+    end
+
+    it "raises an error if single % appears anywhere else" do
+      -> { (" % " % []) }.should raise_error(ArgumentError)
+      -> { ("foo%quux" % []) }.should raise_error(ArgumentError)
+    end
+
+    it "raises an error if NULL or \\n appear anywhere else in the format string" do
+      begin
+        old_debug, $DEBUG = $DEBUG, false
+
+        -> { "%.\n3f" % 1.2 }.should raise_error(ArgumentError)
+        -> { "%.3\nf" % 1.2 }.should raise_error(ArgumentError)
+        -> { "%.\03f" % 1.2 }.should raise_error(ArgumentError)
+        -> { "%.3\0f" % 1.2 }.should raise_error(ArgumentError)
+      ensure
+        $DEBUG = old_debug
+      end
+    end
   end
 
-  it "formats single % character before a NUL as literal %" do
-    ("%\0" % []).should == "%\0"
-    ("foo%\0" % []).should == "foo%\0"
-    ("%\0.3f" % 1.2).should == "%\0.3f"
-  end
-
-  it "raises an error if single % appears anywhere else" do
-    -> { (" % " % []) }.should raise_error(ArgumentError)
-    -> { ("foo%quux" % []) }.should raise_error(ArgumentError)
-  end
-
-  it "raises an error if NULL or \\n appear anywhere else in the format string" do
-    begin
-      old_debug, $DEBUG = $DEBUG, false
-
+  ruby_version_is "3.4" do
+    it "raises an ArgumentError if % is not followed by a conversion specifier" do
+      -> { "%" % [] }.should raise_error(ArgumentError)
+      -> { "%\n" % [] }.should raise_error(ArgumentError)
+      -> { "%\0" % [] }.should raise_error(ArgumentError)
+      -> { " % " % [] }.should raise_error(ArgumentError)
       -> { "%.\n3f" % 1.2 }.should raise_error(ArgumentError)
       -> { "%.3\nf" % 1.2 }.should raise_error(ArgumentError)
       -> { "%.\03f" % 1.2 }.should raise_error(ArgumentError)
       -> { "%.3\0f" % 1.2 }.should raise_error(ArgumentError)
-    ensure
-      $DEBUG = old_debug
     end
   end
 
@@ -125,8 +140,16 @@ describe "String#%" do
     end
   end
 
-  it "replaces trailing absolute argument specifier without type with percent sign" do
-    ("hello %1$" % "foo").should == "hello %"
+  ruby_version_is ""..."3.4" do
+    it "replaces trailing absolute argument specifier without type with percent sign" do
+      ("hello %1$" % "foo").should == "hello %"
+    end
+  end
+
+  ruby_version_is "3.4" do
+    it "raises an ArgumentError if absolute argument specifier is followed by a conversion specifier" do
+      -> { "hello %1$" % "foo" }.should raise_error(ArgumentError)
+    end
   end
 
   it "raises an ArgumentError when given invalid argument specifiers" do

sprintf.c

@@ -429,10 +429,6 @@ rb_str_format(int argc, const VALUE *argv, VALUE fmt)
             GETNUM(prec, precision);
             goto retry;
 
-          case '\n':
-          case '\0':
-            p--;
-            /* fall through */
           case '%':
             if (flags != FNONE) {
                 rb_raise(rb_eArgError, "invalid format character - %%");

test/ruby/test_sprintf.rb

@@ -266,8 +266,8 @@ class TestSprintf < Test::Unit::TestCase
     # Specifying the precision multiple times with negative star arguments:
     assert_raise(ArgumentError, "[ruby-core:11570]") {sprintf("%.*.*.*.*f", -1, -1, -1, 5, 1)}
 
-    # Null bytes after percent signs are removed:
-    assert_equal("%\0x hello", sprintf("%\0x hello"), "[ruby-core:11571]")
+    assert_raise(ArgumentError) {sprintf("%\0x hello")}
+    assert_raise(ArgumentError) {sprintf("%\nx hello")}
 
     assert_raise(ArgumentError, "[ruby-core:11573]") {sprintf("%.25555555555555555555555555555555555555s", "hello")}
 
@@ -279,10 +279,9 @@ class TestSprintf < Test::Unit::TestCase
     assert_raise_with_message(ArgumentError, /unnumbered\(1\) mixed with numbered/) { sprintf("%1$*d", 3) }
     assert_raise_with_message(ArgumentError, /unnumbered\(1\) mixed with numbered/) { sprintf("%1$.*d", 3) }
 
-    verbose, $VERBOSE = $VERBOSE, nil
-    assert_nothing_raised { sprintf("", 1) }
-  ensure
-    $VERBOSE = verbose
+    assert_warning(/too many arguments/) do
+      sprintf("", 1)
+    end
   end
 
   def test_float