Actions
Patch #35720
closed
Defect: Binmode specified twice
Start date:
Due date:
% Done:
0%
Estimated time:
Description
after #35539 I'm getting failures
ArgumentError: binmode specified twice
app/models/attachment.rb:572:in `initialize'
app/models/attachment.rb:572:in `open'
app/models/attachment.rb:572:in `create_diskfile'
app/models/attachment.rb:142:in `files_to_final_location'
File::BINARY flag should be enough to setup a binary mode
diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index c65c56722..b0b0a02ba 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -572,7 +572,6 @@ class Attachment < ActiveRecord::Base
File.open(
File.join(path, name),
flags: File::CREAT | File::EXCL | File::BINARY | File::WRONLY,
- binmode: true,
&block
)
rescue Errno::EEXIST
Related issues
Updated by Go MAEDA over 3 years ago
I think what you pointed out is right but I cannot reproduce the ArgumentError by uploading files or running the test. The following code also doesn't cause errors.
File.open('/tmp/test.txt', flags: File::CREAT | File::EXCL | File::BINARY | File::WRONLY, binmode: true) {|io| io.puts 'hello'}
Could you let me know how I can reproduce the error?
Updated by Pavel Rosický over 3 years ago
your example fails as-well, but it depends on the platform
on Windows
File::BINARY => 32768
on Linux
File::BINARY => 0
which means it actually has no effect on Linux and it won't break if you specify the binmode argument twice.
this is because *nix operating systems don't do automatic linefeed conversion for I/O on "text" files so O_TEXT and O_BINARY flags wouldn't make sense.
Updated by Pavel Rosický over 3 years ago
I removed the flag instead, because binmode also affects encoding on Linux. This passes on both platforms.
diff --git a/app/models/attachment.rb b/app/models/attachment.rb index c65c56722..8fba26eff 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -571,7 +571,7 @@ class Attachment < ActiveRecord::Base name = "#{timestamp}_#{ascii}" File.open( File.join(path, name), - flags: File::CREAT | File::EXCL | File::BINARY | File::WRONLY, + flags: File::CREAT | File::EXCL | File::WRONLY, binmode: true, &block )
Updated by Go MAEDA over 3 years ago
- Related to Defect #35539: Race condition (possible filename collision) in Attachment.disk_filename added
Updated by
Updated by Go MAEDA over 3 years ago
- Status changed from New to Closed
- Assignee set to Go MAEDA
Committed the fix as a part of #35539. Thank you.
Actions