Bug #8767

IO.copy_stream should write in binary mode.

Added by Lin Jen-Shin about 2 years ago. Updated about 2 years ago.

[ruby-core:56518]
Status:Closed
Priority:Normal
Assignee:-
ruby -v:ruby 2.1.0dev (2013-08-11 trunk 42495) [x86_64-darwin12.2.1] Backport:1.9.3: UNKNOWN, 2.0.0: UNKNOWN

Description

This patch makes `IO.copy_stream' always copy in binary mode,
fixing the following scenario:

require 'tempfile'
require 'stringio'
Encoding.default_internal = 'UTF-8'
out = Tempfile.new('out')
out.binmode
# before this patch it raises:
# in `write': "\xDE" from ASCII-8BIT to UTF-8 (Encoding::UndefinedConversionError)
IO.copy_stream(StringIO.new("\xDE\xAD\xBE\xEF"), out)

The other way to fix this would be trying to preserve the file mode
from Tempfile instead of always writing in binary mode. However,
this won't be the case for other objects responding to `to_path'.

I think as we're treating destination as streams, we would always
want writing in binary. So I guess this is ok.

Thank you for reviewing.
This is discovered by using rubyzip, which is using Tempfile for
buffering input/output stream.

Commit on Github:
https://github.com/godfat/ruby/commit/94d9f3dd733fd19f5ade7b6e6f5bdf0c904e06c1

0001-io.c-IO.copy_stream-should-write-in-binary-mode.patch Magnifier (1.36 KB) Lin Jen-Shin, 08/11/2013 03:02 AM

0001-io.c-IO.copy_stream-should-write-in-binary-mode.patch Magnifier - added a test case (2.18 KB) Lin Jen-Shin, 08/12/2013 03:07 AM

Associated revisions

Revision 42709
Added by Nobuyoshi Nakada about 2 years ago

io.c: copy in binary mode

  • io.c (copy_stream_body): should write in binary mode. based on a patch by godfat (Lin Jen-Shin) at . [Bug #8767]

Revision 42709
Added by Nobuyoshi Nakada about 2 years ago

io.c: copy in binary mode

  • io.c (copy_stream_body): should write in binary mode. based on a patch by godfat (Lin Jen-Shin) at . [Bug #8767]

Revision 48631
Added by Usaku NAKAMURA 9 months ago

merge revision(s) 42709: [Backport #10529]

* io.c (copy_stream_body): should write in binary mode.  based on a
  patch by godfat (Lin Jen-Shin) at .
   [Bug #8767]

* io.c (copy_stream_body): move common open flags.

History

#1 Updated by Eric Wong about 2 years ago

"godfat (Lin Jen-Shin)" godfat@godfat.org wrote:

This patch makes `IO.copy_stream' always copy in binary mode,

I think your patch makes sense.

fixing the following scenario:

require 'tempfile'
require 'stringio'
Encoding.default_internal = 'UTF-8'
out = Tempfile.new('out')
out.binmode
# before this patch it raises:
# in `write': "\xDE" from ASCII-8BIT to UTF-8 (Encoding::UndefinedConversionError)
IO.copy_stream(StringIO.new("\xDE\xAD\xBE\xEF"), out)

Can make the above a testcase with your patch?

The other way to fix this would be trying to preserve the file mode
from Tempfile instead of always writing in binary mode. However,
this won't be the case for other objects responding to `to_path'.

Please no :)

#2 Updated by Lin Jen-Shin about 2 years ago

normalperson (Eric Wong) wrote:

"godfat (Lin Jen-Shin)" godfat@godfat.org wrote:

This patch makes `IO.copy_stream' always copy in binary mode,

I think your patch makes sense.

fixing the following scenario:

require 'tempfile'
require 'stringio'
Encoding.default_internal = 'UTF-8'
out = Tempfile.new('out')
out.binmode
# before this patch it raises:
# in `write': "\xDE" from ASCII-8BIT to UTF-8 (Encoding::UndefinedConversionError)
IO.copy_stream(StringIO.new("\xDE\xAD\xBE\xEF"), out)

Can make the above a testcase with your patch?

Yes, thank you for your support. I've added a test for this
and updated the patch in the attachment.

The updated commit on Github is located at:
https://github.com/godfat/ruby/commit/1ad5547335fd2caf31dfa6b5846d01699e693b5f

P.S. To my surprise, it's extremely easy to run the test suites.

#3 Updated by Nobuyoshi Nakada about 2 years ago

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

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


io.c: copy in binary mode

  • io.c (copy_stream_body): should write in binary mode. based on a patch by godfat (Lin Jen-Shin) at . [Bug #8767]

Also available in: Atom PDF