Project

General

Profile

Actions

Bug #5237

closed

IO.copy_stream calls #read on an object infinitely many times

Added by brixen (Brian Shirai) over 12 years ago. Updated over 12 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
-
Backport:
[ruby-core:39134]

Description

The code for IO.copy_stream appears to support passing any object that implements #readpartial or #read. For example, passing an instance of StringIO works just fine:

sasha:rubinius2.0 brian$ irb
ruby-1.9.2-p290 :001 > puts RUBY_DESCRIPTION
ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0]
=> nil
ruby-1.9.2-p290 :002 > require 'stringio'
=> true
ruby-1.9.2-p290 :003 > s = StringIO.new "this is a test"
=> #StringIO:0x00000100851190
ruby-1.9.2-p290 :004 > IO.copy_stream s, "copy_stream_stringio"
=> 14
ruby-1.9.2-p290 :005 > IO.read "copy_stream_stringio"
=> "this is a test"
ruby-1.9.2-p290 :006 >

However, passing an object that implements eg #read according to the specification of IO#read, causes IO.copy_stream to endlessly call #read on the object:

class A
def initialize
@count = 0
end

def read(bytes, buffer)
print "read: #{@count += 1}\r"
buffer.replace "this is a test"
return nil
end
end

a = A.new
IO.copy_stream a, "copy_stream_test"

ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0]
copy_stream_bug.rb:7:in write': Interrupt from copy_stream_bug.rb:7:in print'
from copy_stream_bug.rb:7:in read' from copy_stream_bug.rb:14:in copy_stream'
from copy_stream_bug.rb:14:in `'

Thanks,
Brian


Files

Updated by normalperson (Eric Wong) over 12 years ago

The class that implements an IO#read-like method without clearing
the destination buffer on EOF is arguably broken, but infinite
looping is bad, I think.

The attached simple patch should fix the issue. Also available via git:
git pull git://bogomips.org/ruby.git copy-stream-infinite-loop

Updated by brixen (Brian Shirai) over 12 years ago

On Sat, Aug 27, 2011 at 3:54 AM, Eric Wong wrote:

Issue #5237 has been updated by Eric Wong.

File 0001-avoid-infinite-loop-of-IO.copy_stream.patch added

The class that implements an IO#read-like method without clearing
the destination buffer on EOF is arguably broken, but infinite
looping is bad, I think.

Ah, you are right, it really should replace buffer with "" (according
to the code) and return nil according to the docs. Seems more than a
little weird, however, to empty the buffer to signal EOF, especially
since the buffer argument is optional.

Cheers,
Brian

The attached simple patch should fix the issue.  Also available via git:
 git pull git://bogomips.org/ruby.git copy-stream-infinite-loop


Bug #5237: IO.copy_stream calls #read on an object infinitely many times
http://redmine.ruby-lang.org/issues/5237

Author: Brian Ford
Status: Open
Priority: Normal
Assignee:
Category: core
Target version:
ruby -v: ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0]

The code for IO.copy_stream appears to support passing any object that implements #readpartial or #read. For example, passing an instance of StringIO works just fine:

sasha:rubinius2.0 brian$ irb
ruby-1.9.2-p290 :001 > puts RUBY_DESCRIPTION
ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0]
 

Updated by kosaki (Motohiro KOSAKI) over 12 years ago

  • ruby -v changed from ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-darwin10.8.0] to -

The class that implements an IO#read-like method without clearing
the destination buffer on EOF is arguably broken, but infinite
looping is bad, I think.

Ah, you are right, it really should replace buffer with "" (according
to the code) and return nil according to the docs. Seems more than a
little weird, however, to empty the buffer to signal EOF, especially
since the buffer argument is optional.

I'm curious. Which doc do you talk about?

Updated by brixen (Brian Shirai) over 12 years ago

On Sat, Aug 27, 2011 at 10:55 PM, KOSAKI Motohiro
wrote:

The class that implements an IO#read-like method without clearing
the destination buffer on EOF is arguably broken, but infinite
looping is bad, I think.

Ah, you are right, it really should replace buffer with "" (according
to the code) and return nil according to the docs. Seems more than a
little weird, however, to empty the buffer to signal EOF, especially
since the buffer argument is optional.

I'm curious. Which doc do you talk about?

The RDoc, which refers to returning nil or "" depending on what
arguments are passed. For the case here, it does not matter what value
is returned. The only way to not loop infinitely is to set the buffer
to "".

Updated by kosaki (Motohiro KOSAKI) over 12 years ago

If you are reading following paragraph,

  • At end of file, it returns nil or ""
  • depend on length.
  • ios.read() and
  • ios.read(nil) returns "".
  • ios.read(positive-integer) returns nil.

you have to read following paragraph too, I think.

  • If length is a positive integer,
  • it try to read length bytes without any conversion (binary mode).
  • It returns nil or a string whose length is 1 to length bytes.
  • nil means it met EOF at beginning.

AT BEGINNING

Thus, positive length destination buffer and returning nil are exclusive. Now, I don't think
we need doc fix.

but I'm not talking about a behavior change. I have no strong opinion. It's another story. I'm waiting akr-san's response.

Updated by brixen (Brian Shirai) over 12 years ago

Hi,

On Tue, Aug 30, 2011 at 8:48 PM, Motohiro KOSAKI
wrote:

Issue #5237 has been updated by Motohiro KOSAKI.

If you are reading following paragraph,

  •  At end of file, it returns nil or ""
  •  depend on length.
  •  ios.read() and
  •  ios.read(nil) returns "".
  •  ios.read(positive-integer) returns nil.

you have to read following paragraph too, I think.

  •  If length is a positive integer,
  •  it try to read length bytes without any conversion (binary mode).
  •  It returns nil or a string whose length is 1 to length bytes.
  •  nil means it met EOF at beginning.

AT BEGINNING

Thus, positive length destination buffer and returning nil are exclusive. Now, I don't think
we need doc fix.

but I'm not talking about a behavior change. I have no strong opinion. It's another story. I'm waiting akr-san's response.

I'm not following your explanation. The return value, whether nil or
anything else, including an empty string or any string (one byte or
not) will NOT cause the copy_stream method to return. It will loop
infinitely unless the buffer that is passed in is set to "". In fact,
I can return anything from #read as long as I set the buffer to "".
This is bizarre.

At the least, the rdoc should note that to participate in this API,
#read must set the buffer to "" to signal EOF. Still, in my opinion,
that is a bad API. The return value should be the sole way to signal
EOF given that the arguments to #read are optional.

sasha:rubinius2.0 brian$ cat copy_stream_bug.rb
class A
def initialize
@count

Actions #7

Updated by akr (Akira Tanaka) over 12 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r33515.
Brian, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • io.c (copy_stream_fallback_body): check nil for EOF of read method.
    patch by Eric Wong. [ruby-core:39134] [Bug #5237]
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0