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.

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.

@eregon eregon Jul 31, 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.

Maybe we could look at how StringIO behaves, I'm guessing it doesn't raise an error in that case.

Indeed, seems StringIO does not mind: https://github.com/ruby/stringio/blob/master/ext/stringio/stringio.c
So maybe IO should align to StringIO on CRuby then.

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.

If one is gonna put an invalid character, I think might as well use ungetbyte.

Cases of actual encoding conversion on IO are already very rare, I personally don't see a good enough reason to e.g. support broken characters for ungetc, raising there would be more sensible and simplify things significantly (no need for an extra character buffer).

I also imagine it's pretty easy to build your own ungetc-like functionality on top of an IO, and it's something that feels like it doesn't belong to IO, unless it trivially extends the existing buffering, but that isn't the case.

This is of course my opinion, but this area seems messy enough and low value enough for users that I don't think we want to enforce the behavior, at least not until there is actual code relying on this for good reasons / reasonable use case.
Another POV is CRuby devs sometimes are unhappy about specifying too much details including things which might be accidental, this certainly feels like it fits that bill.

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.

Indeed, seems StringIO does not mind

StringIO diverges from IO's behavior in many places. I'm not sure it's a good example of what to do, at least not at this time.

If one is gonna put an invalid character, I think might as well use ungetbyte.

The point is that you can put an invalid character back into the character buffer via ungetc such that there's no way to reverse the conversion to the byte buffer. If you want to eliminate the character buffer, then you either have to reverse the encoded character so that followup byte oriented read operations get the expected bytes or error out when you can't perform the conversion for any reason.

That sounds quite reasonable to me, but I feel like that kind of change will have to wait for Ruby 5.0 because the current CRuby behavior is fully consistent everywhere I've tested and has been so for all versions since 2.7.

I also imagine it's pretty easy to build your own ungetc-like functionality on top of an IO, and it's something that feels like it doesn't belong to IO, unless it trivially extends the existing buffering, but that isn't the case.

I've done that in the io-like gem. It's honestly not that easy when you want all of the read operations to work transparently with it. You have to hack each of the read methods to account for whatever standalone buffer you use. Either that or you have to deal with the reverse conversion problem I mentioned so you can convert the ungetc call to an equivalent ungetbyte call. A limited IO wrapper class could be minted just to expose the necessary behaviors for a specific workload, but I digress.

Anyway, I'm not here to argue the merits of ungetc and ungetbyte or their current behaviors. I just need to figure out if the behavior is something that should be part of the specification or left unspecified. As you said, the CRuby devs may not want this behavior locked down. I'll have to open the ticket and hope it gets an answer.

Thank you for all the discussion on this.

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