Repository navigation
Avoid IO operations that are affected by the presence or absence of an encoding converter #1387
Copy link
Copy link
Open
Description
I think that mixing IO#ungetc with certain operations is not a recommended practice, as it may cause issues depending on the platform or input.
Perhaps it would be better to exclude this from testing as well.
I would like to hear your thoughts.
Background
When I'm working on a patched version of CRuby that uses an encoding converter when opening files with the “r” option, I found the following error occurred on CI.
My working branch is at https://github.com/YO4/ruby/tree/newline_fastpath.
I'm aiming to merge this patch, but I'm not sure if I should just skip the spec.
1)
IO#getbyte reads after ungetc without character conversion ERROR
IOError: byte oriented read for character buffered IO
D:/a/ruby/ruby/src/spec/ruby/core/io/getbyte_spec.rb:33:in 'IO#getbyte'
D:/a/ruby/ruby/src/spec/ruby/core/io/getbyte_spec.rb:33:in 'block (2 levels) in <top (required)>'
D:/a/ruby/ruby/src/spec/ruby/core/io/getbyte_spec.rb:5:in '<top (required)>'
2)
IO#read with internal encoding not specified reads after ungetc ERROR
IOError: byte oriented read for character buffered IO
D:/a/ruby/ruby/src/spec/ruby/core/io/read_spec.rb:694:in 'IO#read'
D:/a/ruby/ruby/src/spec/ruby/core/io/read_spec.rb:694:in 'block (4 levels) in <top (required)>'
D:/a/ruby/ruby/src/spec/ruby/core/io/read_spec.rb:655:in '<top (required)>'
3)
IO#readbyte reads after ungetc without character conversion ERROR
IOError: byte oriented read for character buffered IO
D:/a/ruby/ruby/src/spec/ruby/core/io/readbyte_spec.rb:29:in 'IO#readbyte'
D:/a/ruby/ruby/src/spec/ruby/core/io/readbyte_spec.rb:29:in 'block (2 levels) in <top (required)>'
D:/a/ruby/ruby/src/spec/ruby/core/io/readbyte_spec.rb:3:in '<top (required)>'
4)
IO#ungetc adjusts the stream position FAILED
Expected 1 == 0
to be truthy but was false
D:/a/ruby/ruby/src/spec/ruby/core/io/ungetc_spec.rb:91:in 'block (2 levels) in <top (required)>'
D:/a/ruby/ruby/src/spec/ruby/core/io/ungetc_spec.rb:5:in '<top (required)>'
Also, JRuby already uses an encoding converter for crlf conversion in open(name, “r”).
C:\jruby-10.1.1.0\bin>jruby D:/git/ruby/spec/mspec/bin/mspec-run d:/git/ruby/spec/ruby/core/io/getbyte_spec.rb
WARNING: A restricted method in java.lang.System has been called
WARNING: java.lang.System::load has been called by com.kenai.jffi.internal.StubLoader in an unnamed module
WARNING: Use --enable-native-access=ALL-UNNAMED to avoid a warning for callers in this module
WARNING: Restricted methods will be blocked in a future release unless native access is enabled
jruby 10.1.1.0 (4.0.0) 2026-07-22 aaa95d57e1 OpenJDK 64-Bit Server VM 25.0.1+8-LTS on 25.0.1+8-LTS +indy +jit [x86_64-mswin32]
1)
IO#getbyte reads after ungetc without character conversion ERROR
IOError: byte oriented read for character buffered IO
org/jruby/RubyIO.java:3175:in 'getbyte'
D:/git/ruby/spec/ruby/core/io/getbyte_spec.rb:33:in 'block in <main>'
org/jruby/RubyBasicObject.java:2777:in 'instance_exec'
org/jruby/RubyArrayNative.java:4226:in 'all?'
org/jruby/RubyArrayNative.java:1729:in 'each'
D:/git/ruby/spec/ruby/core/io/getbyte_spec.rb:5:in '<main>'
org/jruby/RubyKernel.java:1413:in 'load'
org/jruby/RubyBasicObject.java:2777:in 'instance_exec'
org/jruby/RubyArrayNative.java:1729:in 'each'
[/ | ==================100%================== | 00:00:00] �[0;32m 0F �[0;33m 1E�[0m
Finished in 0.044000 seconds
1 file, 6 examples, 6 expectations, 0 failures, 1 error, 0 tagged
JRuby's CI appears to be green at first glance, but I am unfamiliar with that situation.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels
Activity
eregon commentedon Aug 18, 2026
This means your branch raises
IOError: byte oriented read for character buffered IOin more cases than before, right?Is that intentional?
Why should that raise when it did not before? That sounds like a potential incompatibility.
Thank you for your reply in https://bugs.ruby-lang.org/issues/22230#note-1 I think that explains the situation well.
My current understanding from https://bugs.ruby-lang.org/issues/22230 is ruby/spec should not spec for
IOError: byte oriented read for character buffered IO. But it's OK to spec for the absence of it (indirectly, by just having specs exercising the behavior and expecting no exception), as long as CRuby does not raise currently for a given case.That said, specs are mutable so with a good enough reason it can be fine to change them.
Ref for the initial discussion about this: #1384
YO4 commentedon Aug 19, 2026
@eregon, thank you for considering this.
The compatibility break in my branch is entirely intentional.
I believe this compatibility issue could potentially arise in other situations as well.
By clarifying the situation for https://bugs.ruby-lang.org/issues/22230#note-1, I’ve become even more convinced that code relying on
ungetc’s behavior without encoding conversion is fragile.For example, it causes problems even when run with
ruby -Eiso-8859-1:utf-8.This means we cannot assume that
open(name, “r”)does not involve encoding conversion.To clarify my intentions, I will also explain the focus of my branch.
Since I was using Ruby 1.8 at the time, this is based on archaeological research.
In Ruby 1.9—until it was changed during the development of Ruby 2.0.0—encoding conversion was used to handle CRLF.
Due to performance issues, the current specification—which utilizes the conversion performed by the C runtime library’s
_read()—was introduced in Feature #5562.It certainly had an effect. https://blade.ruby-lang.org/ruby-core/42686
However, I believe this introduced a different problem. I explain this in detail at Bug #21765.
My branch shares the problem and proposes an alternative solution to the PR mentioned in that issue.
The code in the PR for Bug #21765 is implemented to avoid compatibility issues, but the patches are scattered throughout io.c.
I believe my new branch provides a cleaner patch and achieves better separation for the implementation of Windows-specific features.
At least for now, I believe my branch would be preferable. And just as I was getting ready to prepare the PR, I discovered a failure in the spec.
The issue with
ruby -Eiso-8859-1:utf-8may be outside the scope of ruby/spec, but its existence seems to provide justification for the incompatibility in my branch.YO4 commentedon Aug 23, 2026
I posted on b-r-l-o regarding the acceptance of incompatibility.
https://bugs.ruby-lang.org/issues/21765#note-4
eregon commentedon Aug 24, 2026
@YO4 Thank you for your reply, I need to find some time to look at it in more details.