Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4a7d459
Add spec for IO#binmode disabling newline conversion when reading
javanthropus Jul 22, 2026
976d046
Add specs for IO#getbyte after ungetc
javanthropus Jul 22, 2026
6f9fba9
Add specs for IO#internal_encoding with BINARY external encoding
javanthropus Jul 22, 2026
a8dd789
Add spec for IO#pwrite raising Errno::EINVAL on invalid offset
javanthropus Jul 22, 2026
d91bdf7
Update and add specs for IO#read_nonblock after ungetc and with 0 length
javanthropus Jul 22, 2026
40e859f
Add specs for IO#read after ungetc
javanthropus Jul 22, 2026
3a69b22
Add specs for IO#readbyte after ungetc
javanthropus Jul 22, 2026
f0a3b5f
Add spec for IO#readpartial raising IOError after ungetc with charact…
javanthropus Jul 22, 2026
46f9838
Add specs for IO#reopen argument checking and open modes
javanthropus Jul 22, 2026
538111f
Add spec for IO#seek clearing character buffer after ungetc
javanthropus Jul 22, 2026
950b6cc
Refactor IO#set_encoding specs and add missing edge cases
javanthropus Jul 22, 2026
09eb335
Add and update specs for IO#sysread with buffered IO
javanthropus Jul 22, 2026
18069f9
Add and update specs for IO#sysseek with buffered IO
javanthropus Jul 22, 2026
e1b1972
Add and refactor specs for IO#syswrite on pipes
javanthropus Jul 22, 2026
c9449cb
Add specs for IO#write with ASCII-8BIT encoding and newline conversion
javanthropus Jul 22, 2026
5694592
Omit IO#pwrite test for EINVAL result on Windows
javanthropus Jul 23, 2026
7008f3b
Remove guards and specs for pre-3.3 behaviors
javanthropus Jul 24, 2026
0749bb7
Add additional specs for binmode to verify behavior of other reader m…
javanthropus Jul 25, 2026
40f33d2
Move spec for #set_encoding and #internal_encoding behavior to set_en…
javanthropus Jul 25, 2026
b9fa9e5
Use a test value that should trigger an error for pwrite on all platf…
javanthropus Jul 25, 2026
5309d09
Remove erroneous, semi-duplicate code from set_encoding specs
javanthropus Jul 25, 2026
68eaaf0
Use shorthand helper method to force a string to binary encoding (#b)
javanthropus Jul 30, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions core/io/binmode_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,142 @@
@io.binmode
@io.internal_encoding.should == nil
end

it "disables newline conversion for #read" do
data = "line1\r\nline2\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
@io.read.should == data
end
Comment thread
andrykonchin marked this conversation as resolved.

it "disables newline conversion for #gets" do
data = "line1\r\nline2\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
@io.gets.should == "line1\r\n"
@io.gets.should == "line2\r\n"
end

it "disables newline conversion for #readline" do
data = "line1\r\nline2\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
@io.readline.should == "line1\r\n"
@io.readline.should == "line2\r\n"
end

it "disables newline conversion for #readlines" do
data = "line1\r\nline2\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
@io.readlines.should == ["line1\r\n", "line2\r\n"]
end

it "disables newline conversion for #each" do
data = "line1\r\nline2\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
@io.each.to_a.should == ["line1\r\n", "line2\r\n"]
end

it "disables newline conversion for #each_line" do
data = "line1\r\nline2\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
@io.each_line.to_a.should == ["line1\r\n", "line2\r\n"]
end

it "disables newline conversion for #getc" do
data = "line1\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
5.times { @io.getc }
@io.getc.should == "\r"
@io.getc.should == "\n"
end

it "disables newline conversion for #readchar" do
data = "line1\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
5.times { @io.readchar }
@io.readchar.should == "\r"
@io.readchar.should == "\n"
end

it "disables newline conversion for #each_char" do
data = "line1\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
@io.each_char.to_a.should == ["l", "i", "n", "e", "1", "\r", "\n"]
end

it "disables newline conversion for #each_codepoint" do
data = "line1\r\n"

@io = new_io(@name, "wb")
@io.write(data)
@io.close

@io = new_io(@name, "rt")
@io.set_encoding("utf-8:ISO-8859-1", newline: :universal)
@io.binmode
@io.each_codepoint.to_a.should == [108, 105, 110, 101, 49, 13, 10]
end
end

describe "IO#binmode?" do
Expand Down
16 changes: 16 additions & 0 deletions core/io/getbyte_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,22 @@
it "raises an IOError on closed stream" do
-> { IOSpecs.closed_io.getbyte }.should.raise(IOError)
end

it "reads after ungetc without character conversion" do
@io.set_encoding("utf-8")
c = @io.getc
@io.ungetc(c)
@io.getbyte.should == 86
end

it "raises an exception after ungetc with character conversion" do
@io.set_encoding("utf-8:utf-16be")
c = @io.getc
@io.ungetc(c)
-> do
@io.getbyte
end.should.raise(IOError, "byte oriented read for character buffered IO")
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: this appears to be a CRuby-specific implementation detail (or an intentional design constraint) that other Ruby implementations, such as JRuby, TruffleRuby, or Natalie, might handle differently (e.g., by
permitting byte-oriented reading under these conditions).

CRuby already allows mixing byte-oriented and character-oriented read operations on streams with multibyte encodings (such as UTF-16BE). Semantically, calling ungetc does not make this behavior any worse or introduce additional issues.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that this project documents the behavior of CRuby, and this is indeed a CRuby behavior. I've not tried it on other implementations. Is that a requirement now?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that this project documents the behavior of CRuby

Not exactly, as the README says:

The Ruby Spec Suite, abbreviated ruby/spec, is a test suite for the behavior of the Ruby programming language.

Which notably means avoiding to add specs for things that are bugs in CRuby (and instead report the bug/inconsistency), ask clarification on the CRuby bug tracker for unclear cases and otherwise yes mostly assume CRuby behavior is intended if it looks reasonable.

I think this is a case of "unclear, should ask clarification on the CRuby bug tracker".
Would you mind opening a Misc issue at https://bugs.ruby-lang.org/ to ask the reason for this error and detail there how it's inconsistent?

I think for now it's better to not add these specs in ruby/spec, because the effect would be probably unclear semantics and extra overhead for other Ruby implementations, when that behavior might be undesirable anyway (could probably be even seen as a bug by someone trying to do legitimately).
Could you remove them for this PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Playing a bit in IRB with this and yeah it's weird, I could maybe understand if byte operations are not allowed with UTF-16/32 (which are the only encodings with min-length > 1 byte), although that would be inconsistent with String which doesn't mind.
But it's not even that, that's allowed, it's indeed after ungetc it's not allowed.
That starts to feel like a bug.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah BTW there was already 1 spec about this (it "raises an exception after ungetc with data in the buffer and character conversion enabled" do) let's remove it too

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will open tickets as requested, but I've had mixed results getting attention on some of them. This ticket is one such which is being ignored. Is there something else I should do to avoid these new tickets also being ignored?

Yes sometimes people answer and sometimes they don't, maybe because nobody knows or remember.
One possibility is to add a ticket to a DevMeeting, then usually someone will take a look at it.
The next dev meeting isn't scheduled yet though.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at READ_CHAR_PENDING/READ_DATA_PENDING in CRuby and them having different buffers seems very much like accidental complexity to me, not something we'd want to spec, but some implementation artifact from old Ruby versions. The Ruby IO API is also known as one of the most messy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unpopular opinion but maybe ungetc/ungetbyte should be deprecated/removed, it adds significant complexity for little value and little need. I'm not hopeful though.

Read buffering itself is fine and needed e.g. for IO#readline, but being able to mutate the buffer though ungetc/ungetbyte that seems quite brittle.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That part I don't know so well, when one does ungetc is there any encoding conversion?

No, the bytes passed to ungetc are dropped directly into the internal character buffer, which is separate from the internal byte buffer. The idea is that you get those bytes back when calling getc or any other character oriented read operation. You can even intentionally put an invalid character back via ungetc, so it's not even possible to always reverse the character conversion of the stream in order to drop the bytes into the byte buffer.

In order to attempt a reverse conversion, the behavior of ungetc would have to change to raise a conversion error when the conversion fails. It never does that now, so that would risk breaking existing code in surprising ways. Then there's the cost of creating a new character convertor for the reverse operation and risk of state loss when resetting the state of the forward character convertor after pushing converted bytes back into the byte buffer. This quickly becomes a can of worms.

I think we're approaching the point of identifying these specs as undefined behavior unless the Ruby community sees value in aligning on an implementation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at READ_CHAR_PENDING/READ_DATA_PENDING in CRuby and them having different buffers seems very much like accidental complexity to me, not something we'd want to spec, but some implementation artifact from old Ruby versions.

I think it's deliberate complexity given my earlier message. It provides the least surprise in the more common case where people don't mix character and byte oriented operations on the same IO object.

The Ruby IO API is also known as one of the most messy.

I'm far too aware of that mess. I've spent years of effort attempting to make a faithful, pure Ruby implementation of IO operations in the io-like gem. I've been successful, which is why I have all these new specs for weird corner cases.

Unpopular opinion but maybe ungetc/ungetbyte should be deprecated/removed, it adds significant complexity for little value and little need.

People implementing parsers at least would probably disagree with you. It's common to leverage the read buffer as a form of lookahead by putting back bytes or characters that you just peeked at in order to make a decision.

being able to mutate the buffer though ungetc/ungetbyte that seems quite brittle.

Whether brittle or not, I used ungetc in these tests as a way to ensure that the character buffer has predictable content. There may be other ways, but this was the easiest to prove and use. The point is that the character buffer may have content in it. That content is derived from the byte buffer, so bytes were consumed to provide the character buffer content.

The "right" solution for a byte oriented read operation in that condition is debatable, but it's hard to imagine the CRuby maintainers changing their decision at this point. Nothing less than a complete overhaul of the IO class would justify that sort of change.

end

describe "IO#getbyte" do
Expand Down
5 changes: 5 additions & 0 deletions core/io/internal_encoding_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@
@io = new_io @name, "#{@object}:binary"
@io.internal_encoding.should == nil
end

it "returns nil when the external encoding is BINARY and internal encoding is set" do
@io = new_io @name, "#{@object}:binary:ibm437"
@io.internal_encoding.should == nil
end
end
end

Expand Down
6 changes: 6 additions & 0 deletions core/io/pwrite_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@
}.should.raise(NoMethodError, /undefined method [`']to_s'/)
end

it "raises a Errno::EINVAL if the offset is invalid" do
-> {
@file.pwrite("foo", -3)
}.should.raise(Errno::EINVAL)
end

it "raises a TypeError if the offset cannot be converted to an Integer" do
-> {
@file.pwrite("foo", Object.new)
Expand Down
14 changes: 12 additions & 2 deletions core/io/read_nonblock_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,16 @@
@read.read_nonblock(3).should == "bar"
end

it "raises an exception after ungetc with data in the buffer and character conversion enabled" do
it "raises an exception after ungetc with character conversion enabled" do
@write.write("foobar")
@read.set_encoding(
'utf-8', universal_newline: true
)
c = @read.getc
@read.ungetc(c)
-> { @read.read_nonblock(3).should == "foo" }.should.raise(IOError)
-> do
@read.read_nonblock(3)
end.should.raise(IOError, "byte oriented read for character buffered IO")
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eregon, while removing the tests as you requested, this one caught my eye because it's a pre-existing test that does exactly the same thing. All I did here was clean it up and add a check for the error message in addition to the exception type.

In light of this test having already been accepted back in 2022, maybe it's safe to say that other Ruby implementations already have this logic correctly implemented. If you still think that this class of test should be removed, then I think it best to remove this one as well. What do you think?

@eregon eregon Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove this one too, like in #1384 (comment).
If it's many of them then yeah let's reconsider but if a handful let's just remove those.
If you miss some it's also not a big deal.

maybe it's safe to say that other Ruby implementations already have this logic correctly implemented

TruffleRuby doesn't have it, Rubinius doesn't have it.
JRuby seems to have some of it due to copying that code from CRuby, but I don't think they have given it much thought.


it "returns less data if that is all that is available" do
Expand Down Expand Up @@ -137,6 +139,14 @@
-> { @read.read_nonblock(5) }.should.raise(EOFError)
end

ruby_bug "#18421", ""..."3.0.4" do
it "clears and returns the given buffer if the length argument is 0" do
buffer = String.new("existing content")
@read.read_nonblock(0, buffer).should == buffer
buffer.should == ""
end
end

it "preserves the encoding of the given buffer" do
buffer = ''.encode(Encoding::ISO_8859_1)
@write.write("abc")
Expand Down
38 changes: 38 additions & 0 deletions core/io/read_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,12 @@
@io.read.encoding.should.equal?(Encoding::EUC_JP)
end

it "reads after ungetc" do
c = @io.getc
@io.ungetc(c)
@io.read(2).should == [164, 162].pack('C*').force_encoding(Encoding::BINARY)
end

it_behaves_like :io_read_size_internal_encoding, nil
end

Expand All @@ -680,6 +686,14 @@
@io = IOSpecs.io_fixture "read_euc_jp.txt", "r:euc-jp:utf-8"
end

it "raises an exception after ungetc" do
c = @io.getc
@io.ungetc(c)
-> do
@io.read(2)
end.should.raise(IOError, "byte oriented read for character buffered IO")
end

it_behaves_like :io_read_internal_encoding, nil
it_behaves_like :io_read_size_internal_encoding, nil
end
Expand All @@ -689,6 +703,14 @@
@io = IOSpecs.io_fixture "read_euc_jp.txt", mode: "r:euc-jp:utf-8"
end

it "raises an exception after ungetc" do
c = @io.getc
@io.ungetc(c)
-> do
@io.read(2)
end.should.raise(IOError, "byte oriented read for character buffered IO")
end

it_behaves_like :io_read_internal_encoding, nil
it_behaves_like :io_read_size_internal_encoding, nil
end
Expand All @@ -701,6 +723,14 @@
@io = IOSpecs.io_fixture "read_euc_jp.txt", options
end

it "raises an exception after ungetc" do
c = @io.getc
@io.ungetc(c)
-> do
@io.read(2)
end.should.raise(IOError, "byte oriented read for character buffered IO")
end

it_behaves_like :io_read_internal_encoding, nil
it_behaves_like :io_read_size_internal_encoding, nil
end
Expand All @@ -711,6 +741,14 @@
@io = IOSpecs.io_fixture "read_euc_jp.txt", options
end

it "raises an exception after ungetc" do
c = @io.getc
@io.ungetc(c)
-> do
@io.read(2)
end.should.raise(IOError, "byte oriented read for character buffered IO")
end

it_behaves_like :io_read_internal_encoding, nil
it_behaves_like :io_read_size_internal_encoding, nil
end
Expand Down
16 changes: 16 additions & 0 deletions core/io/readbyte_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,20 @@
@io.readbyte
end.should.raise EOFError
end

it "reads after ungetc without character conversion" do
@io.set_encoding("utf-8")
c = @io.getc
@io.ungetc(c)
@io.readbyte.should == ?r.getbyte(0)
end

it "raises an exception after ungetc with character conversion" do
@io.set_encoding("utf-8:utf-16be")
c = @io.getc
@io.ungetc(c)
-> do
@io.readbyte
end.should.raise(IOError, "byte oriented read for character buffered IO")
end
end
10 changes: 10 additions & 0 deletions core/io/readpartial_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@
@rd.readpartial(2).should == "b"
end

it "raises an exception after ungetc with character conversion enabled" do
@wr.write("foobar")
@rd.set_encoding('utf-8', 'ascii')
c = @rd.getc
@rd.ungetc(c)
-> do
@rd.readpartial(3)
end.should.raise(IOError, "byte oriented read for character buffered IO")
end

it "discards the existing buffer content upon successful read" do
buffer = +"existing content"
@wr.write("hello world")
Expand Down
Loading