New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

File.open() on Windows without binary flag reading 0x0A (newline) instead of 0x0D (return) #5100

Closed
thomas- opened this Issue Mar 21, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@thomas-

thomas- commented Mar 21, 2018

I ran into this bug using a ruby gem that does file uploads, when some gifs became very mangled, seems to be affecting only Windows JRuby from what I can tell.

My reason for reporting is mostly that it is a different outcome than what happens in other envs and in cruby.

Environment

  • jruby 9.1.16.0 (2.3.3) 2018-02-21 8f3f95a OpenJDK 64-Bit Server VM 9.0.4.1-ojdkbuild+11 on 9.0.4.1-ojdkbuild+11 +jit [mswin32-x86_64]
  • Windows 10

Expected Behavior

Script:

#!/usr/bin/ruby

f1 = File.open("0d.bin") { |f| f.read }
f2 = File.open("0d.bin", "rb") { |f| f.read }

puts "file.open (no rb):"
f1hex = f1.each_byte.map { |b| b.to_s(16) }.join(' ')
puts f1hex
puts "file.open (with 'rb'):"
f2hex = f2.each_byte.map { |b| b.to_s(16) }.join(' ')
puts f2hex

puts "equal?"
puts f1hex == f2hex

Where 0d.bin is a simple file with following hex contents:

0D 01 02 03 04 0D 01 02  03 04 0D 01 02 03 04 0D

I tested behavior with JRuby under linux, CRuby under linux, and CRuby under windows, all of which output:

file.open (no rb):
d 1 2 3 4 d 1 2 3 4 d 1 2 3 4 d
file.open (with 'rb'):
d 1 2 3 4 d 1 2 3 4 d 1 2 3 4 d
equal?
true

Gist with bin file at https://gist.github.com/thomas-/3aa435855b10c7bc728c17e65d0ee4d1

Actual Behavior

JRuby under windows instead outputs:

file.open (no rb):
a 1 2 3 4 a 1 2 3 4 a 1 2 3 4 a
file.open (with 'rb'):
d 1 2 3 4 d 1 2 3 4 d 1 2 3 4 d
equal?
false
@headius

This comment has been minimized.

Member

headius commented Mar 26, 2018

I'm sure this is a carriage-return/line-feed translation bug. "b" mode, which does no such translation, is obviously working correctly here. "t" mode, the default, wants to normalize line terminators to line-feed, but obviously it should not be doing so for a bare carriage-return.

@headius headius self-assigned this Mar 26, 2018

@headius headius referenced this issue Mar 27, 2018

Open

JRuby 9.2 Projects #5119

6 of 15 tasks complete
@headius

This comment has been minimized.

Member

headius commented Apr 12, 2018

Ok, so first off a bit of pushback: if there's a library reading files from disk and it's not specifying rb, that's a bug. I can fix it so it won't translate bare CR, but if it encounters CRLF it's still going to mangle the result. If it specified rb you wouldn't run into this problem.

That in my mind downgrades the severity of this bug.

We will, of course, fix the text-mode mangling of CR.

@thomas-

This comment has been minimized.

thomas- commented Apr 16, 2018

Yeah that makes sense. Just wanted to give a bit of background as to how I came across the issue, agree it should be rb - felt worthy of reporting as the bare CR translation seems to be different from from cruby is all.

Thanks for looking into this! :)

@headius

This comment has been minimized.

Member

headius commented Apr 16, 2018

I believe this is the commit in CRuby that changed from universal newline conversion (which does CR) to CRLF newline conversion (which only does CRLF): f9a6a1dd0c6

I believe most of the remaining plumbing exists in JRuby but this change was missed.

headius added a commit that referenced this issue Apr 16, 2018

Use CRLF newline conversion instead of universal. Fixes #5100.
See ruby/ruby@f9a6a1d for the change in CRuby. The remaining
plumbing in this commit needs to be reviewed.

@headius headius closed this Apr 16, 2018

@headius

This comment has been minimized.

Member

headius commented Apr 16, 2018

@thomas- It should be fixed. Unfortunately nightly builds aren't running at the moment, but once they are (or you build yourself) you should be able to confirm it.

Perhaps you could also add a quick spec to https://github.com/spec/ruby?

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