Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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
Closed

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

thomas- opened this issue Mar 21, 2018 · 5 comments
Assignees
@headius
Labels
windows
Milestone
JRuby 9.1.17.0

Comments

@thomas-
Copy link

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
@enebo enebo added this to the JRuby 9.1.17.0 milestone Mar 21, 2018
@enebo enebo added the windows label Mar 21, 2018
@thomas- thomas- mentioned this issue Mar 21, 2018
Closed
@headius
Copy link
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 mentioned this issue Mar 27, 2018
Closed
15 tasks
@headius
Copy link
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-
Copy link
Author

thomas- commented Apr 16, 2018 •
edited

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
Copy link
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
@headius
Use CRLF newline conversion instead of universal. Fixes #5100.
9549f5d 
See ruby/ruby@f9a6a1dd0c6 for the change in CRuby. The remaining
plumbing in this commit needs to be reviewed.
@headius headius closed this as completed Apr 16, 2018
@headius
Copy link
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
Assignees

@headius headius

Labels
windows
Projects
None yet
Milestone
JRuby 9.1.17.0
Development

No branches or pull requests
3 participants
@headius @enebo @thomas-