Skip to content

Avoid IO operations that are affected by the presence or absence of an encoding converter #1387

Open

Description

@YO4

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.

Activity

  1. eregon commented on Aug 18, 2026

    @eregon
    Member

    This means your branch raises IOError: byte oriented read for character buffered IO in 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

  2. YO4 commented on Aug 19, 2026

    @YO4
    ContributorAuthor

    @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-8 may be outside the scope of ruby/spec, but its existence seems to provide justification for the incompatibility in my branch.

  3. YO4 commented on Aug 23, 2026

    @YO4
    ContributorAuthor

    I posted on b-r-l-o regarding the acceptance of incompatibility.
    https://bugs.ruby-lang.org/issues/21765#note-4

  4. eregon commented on Aug 24, 2026

    @eregon
    Member

    @YO4 Thank you for your reply, I need to find some time to look at it in more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions