Project

General

Profile

Actions

Bug #21151

closed

IO and StringIO raise FrozenError even for read-only methods

Added by headius (Charles Nutter) 6 months ago. Updated 1 day ago.

Status:
Feedback
Assignee:
-
Target version:
-
[ruby-core:121136]

Description

Problem

While fixing a small regression from my recent StringIO fixes I discovered that a large number of read-only methods will fail if the StringIO is frozen. A few examples:

$ ruby -rstringio -e 's = StringIO.new; p s.lineno; s.freeze; p s.lineno' 
0
-e:1:in 'StringIO#lineno': can't modify frozen StringIO: #<StringIO:0x0000000103711ea8> (FrozenError)
	from -e:1:in '<main>'
$ ruby -rstringio -e 's = StringIO.new; p s.closed?; s.freeze; p s.closed?'
false
-e:1:in 'StringIO#closed?': can't modify frozen StringIO: #<StringIO:0x00000001008c1e68> (FrozenError)
	from -e:1:in '<main>'
$ ruby -rstringio -e 's = StringIO.new; p s.eof?; s.freeze; p s.eof?'
true
-e:1:in 'StringIO#eof?': can't modify frozen StringIO: #<StringIO:0x000000011c171e80> (FrozenError)
	from -e:1:in '<main>'
$ ruby -rstringio -e 's = StringIO.new; p s.pos; s.freeze; p s.pos'
0
-e:1:in 'StringIO#pos': can't modify frozen StringIO: #<StringIO:0x0000000105551df0> (FrozenError)
	from -e:1:in '<main>'

@kou (Kouhei Sutou) pointed out that IO also raises for at least lineno and suggested I re-open the issue here. I still believe these read-only operations should be allowed on frozen IO or StringIO.

Cause in StringIO

This is because the StringIO macro calls get_strio which calls rb_io_taint_check which calls rb_check_frozen. The StringIO macro is used in almost every method to access the rb_stringio_t data structure.

A list of methods I believe should be operable when the StringIO is frozen (in stringio.c definition order):

  • string (returns underlying String but does not mutate anything)
  • lineno
  • pos
  • closed?/closed_read?/closed_write?
  • eof/eof?
  • sync
  • pid (a dummy method but it writes nothing)
  • fileno (dummy)
  • pread (by definition does not modify state)
  • isatty/tty?
  • size/length
  • external_encoding
  • internal_encoding

In addition, initialize_copy probably should not require the original StringIO to be writable:

$ ruby -rstringio -e 's = StringIO.new("foo"); s.freeze; p s.dup'                                                                                           
-e:1:in 'StringIO#initialize_copy': can't modify frozen StringIO: #<StringIO:0x0000000102eb1df8> (FrozenError)
	from -e:1:in 'Kernel#initialize_dup'
	from -e:1:in 'Kernel#dup'
	from -e:1:in '<main>'

The data from the original StringIO is unmodified by initialize_copy, other than the reference-counting ptr->count (which should not be subject to frozen checks).

Cause in IO

The GetOpenFile macro calls RB_IO_POINTER macro which calls rb_io_taint_check which calls rb_check_frozen.

Methods in IO that should work on a frozen IO include those from the StringIO list above that currently raise. For example lineno uses GetOpenFile and raises, but fileno does not and does not raise.

There's clearly some inconsistency here we can clean up.

Origin

I believe most of the StringIO cases are caused by this change from 2011 that added frozen checks to StringIO (the class and the macro): https://github.com/ruby/ruby/commit/d8d9bac5c8b071135e50ad3f21c8a9b6a9c06e54

In IO, this behavior dates way back to 2000 by @matz (Yukihiro Matsumoto) himself: https://github.com/ruby/ruby/commit/087c83d7ceed6893afff93066937fb570ae4a115

Notes

Originally filed as https://github.com/ruby/stringio/issues/120.

I have started to fix the StringIO cases for JRuby in https://github.com/ruby/stringio/pull/122. I could probably fix the C version of StringIO as well, but I'm a little more unsure about how to fix IO.

Actions #1

Updated by headius (Charles Nutter) 6 months ago

  • Description updated (diff)
Actions #2

Updated by headius (Charles Nutter) 6 months ago

  • Description updated (diff)

Updated by shyouhei (Shyouhei Urabe) 6 months ago

StringIO situation is different, but for an IO to be read it has to be writable; thus can not be frozen. See what OpenSSL people say aobut this: https://github.com/openssl/openssl/commit/b1d6e3f551ce7e081ed3e30d525253042ebb10a4

Updated by matz (Yukihiro Matsumoto) 5 months ago

  • Status changed from Open to Feedback

First, I would like to know why you want to freeze IO and StringIO. I can't think of a use case.
If there is no valid reason, it would not help if all operations on a frozen IO were prohibited. If there could be a good reason, I would consider allowing the possible operations, as @headious said.

Matz.

Updated by headius (Charles Nutter) 1 day ago

for an IO to be read it has to be writable

In order to read data out of an IO, sure. But the methods I mentioned do not require write access to the IO; they just return non-writable state.

I would like to know why you want to freeze IO and StringIO

The non-writeable state could be passed on to another library for querying without risking that they would further modify that state.

For example, passing a frozen StringIO to a library that needs to know lineno or pos.

Basically, the use case is that if I'm done with a StringIO or IO and want to prevent it from being modified further while sharing its non-writable state with another library.

There's also no logical reason why reading non-writable state from a StringIO or IO should require that it be mutable.

Updated by headius (Charles Nutter) 1 day ago

Edit: previously I referred to non-writable state, but in actuality any state of StringIO or IO should be safe to read even if the object itself is frozen. There's no need for either type to be mutable just to read sync or external_encoding for example.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0