Improve Puma::NullIO consistency with real IO (#3276)

This isn't a big deal, but every small difference has a chance to
let a bug slip as it's likely not every scenario is tested with both
NullIO and a real IO.

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>

lib/puma/null_io.rb

@@ -18,8 +18,22 @@ def each
 
     # Mimics IO#read with no data.
     #
-    def read(count = nil, _buffer = nil)
-      count && count > 0 ? nil : ""
+    def read(length = nil, buffer = nil)
+      if length.to_i < 0
+        raise ArgumentError, "(negative length #{length} given)"
+      end
+
+      buffer = if buffer.nil?
+        "".b
+      else
+        String.try_convert(buffer) or raise TypeError, "no implicit conversion of #{buffer.class} into String"
+      end
+      buffer.clear
+      if length.to_i > 0
+        nil
+      else
+        buffer
+      end
     end
 
     def rewind

test/test_null_io.rb

@@ -47,9 +47,68 @@ def test_read_with_positive_integer_length
     assert_nil nio.read(1)
   end
 
+  def test_read_with_negative_length
+    error = assert_raises ArgumentError do
+      nio.read(-42)
+    end
+    assert_includes error.message, "negative length -42 given"
+  end
+
+  def test_read_with_nil_buffer
+    assert_equal "", nio.read(nil, nil)
+    assert_equal "", nio.read(0, nil)
+    assert_nil nio.read(1, nil)
+  end
+
+  class ImplicitString
+    def to_str
+      "ImplicitString".b
+    end
+  end
+
+  def test_read_with_implicit_string_like_buffer
+    assert_equal "", nio.read(nil, ImplicitString.new)
+  end
+
+  def test_read_with_invalid_buffer
+    error = assert_raises TypeError do
+      nio.read(nil, Object.new)
+    end
+    assert_includes error.message, "no implicit conversion of Object into String"
+
+    error = assert_raises TypeError do
+      nio.read(0, Object.new)
+    end
+
+    error = assert_raises TypeError do
+      nio.read(1, Object.new)
+    end
+    assert_includes error.message, "no implicit conversion of Object into String"
+  end
+
+  def test_read_with_frozen_buffer
+    assert_raises FrozenError do
+      nio.read(nil, "".freeze)
+    end
+
+    assert_raises FrozenError do
+      nio.read(0, "".freeze)
+    end
+
+    assert_raises FrozenError do
+      nio.read(20, "".freeze)
+    end
+  end
+
   def test_read_with_length_and_buffer
-    buf = ""
+    buf = "random_data".b
     assert_nil nio.read(1, buf)
+    assert_equal "".b, buf
+  end
+
+  def test_read_with_buffer
+    buf = "random_data".b
+    assert_same buf, nio.read(nil, buf)
     assert_equal "", buf
   end
 
@@ -69,3 +128,23 @@ def test_closed_returns_false
     assert_equal false, nio.closed?
   end
 end
+
+# Run the same tests but against an empty file to
+# ensure all the test behavior is accurate
+class TestNullIOConformance < TestNullIO
+  def setup
+    self.nio = ::Tempfile.create
+    nio.sync = true
+  end
+
+  def teardown
+    return unless nio.is_a? ::File
+    nio.close
+    File.unlink nio.path
+  end
+
+  def test_string_returns_empty_string
+    self.nio = StringIO.new
+    super
+  end
+end