Skip to content

Commit

Permalink
Fix File#truncate and #lock for Win32 append-mode files (#14706)
Browse files Browse the repository at this point in the history
  • Loading branch information
HertzDevil authored Aug 8, 2024
1 parent 74f0093 commit 4c2eaf0
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 7 deletions.
35 changes: 35 additions & 0 deletions spec/std/file_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,41 @@ describe "File" do
end
end

it "does not overwrite existing content in append mode" do
with_tempfile("append-override.txt") do |filename|
File.write(filename, "0123456789")

File.open(filename, "a") do |file|
file.seek(5)
file.write "abcd".to_slice
end

File.read(filename).should eq "0123456789abcd"
end
end

it "truncates file opened in append mode (#14702)" do
with_tempfile("truncate-append.txt") do |path|
File.write(path, "0123456789")

File.open(path, "a") do |file|
file.truncate(4)
end

File.read(path).should eq "0123"
end
end

it "locks file opened in append mode (#14702)" do
with_tempfile("truncate-append.txt") do |path|
File.write(path, "0123456789")

File.open(path, "a") do |file|
file.flock_exclusive { }
end
end
end

it "can navigate with pos" do
File.open(datapath("test_file.txt")) do |file|
file.pos = 3
Expand Down
3 changes: 3 additions & 0 deletions src/crystal/system/unix/file.cr
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ module Crystal::System::File
{fd, fd < 0 ? Errno.value : Errno::NONE}
end

protected def system_set_mode(mode : String)
end

def self.info?(path : String, follow_symlinks : Bool) : ::File::Info?
stat = uninitialized LibC::Stat
if follow_symlinks
Expand Down
3 changes: 3 additions & 0 deletions src/crystal/system/wasi/file.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ require "../unix/file"

# :nodoc:
module Crystal::System::File
protected def system_set_mode(mode : String)
end

def self.chmod(path, mode)
raise NotImplementedError.new "Crystal::System::File.chmod"
end
Expand Down
20 changes: 16 additions & 4 deletions src/crystal/system/win32/file.cr
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ require "c/ntifs"
require "c/winioctl"

module Crystal::System::File
# On Windows we cannot rely on the system mode `FILE_APPEND_DATA` and
# keep track of append mode explicitly. When writing data, this ensures to only
# write at the end of the file.
@system_append = false

def self.open(filename : String, mode : String, perm : Int32 | ::File::Permissions) : FileDescriptor::Handle
perm = ::File::Permissions.new(perm) if perm.is_a? Int32
# Only the owner writable bit is used, since windows only supports
Expand Down Expand Up @@ -52,10 +57,9 @@ module Crystal::System::File
LibC::FILE_GENERIC_READ
end

if flags.bits_set? LibC::O_APPEND
access |= LibC::FILE_APPEND_DATA
access &= ~LibC::FILE_WRITE_DATA
end
# do not handle `O_APPEND`, because Win32 append mode relies on removing
# `FILE_WRITE_DATA` which breaks file truncation and locking; instead,
# simply set the end of the file as the write offset in `#write_blocking`

if flags.bits_set? LibC::O_TRUNC
if flags.bits_set? LibC::O_CREAT
Expand Down Expand Up @@ -96,6 +100,14 @@ module Crystal::System::File
{access, disposition, attributes}
end

protected def system_set_mode(mode : String)
@system_append = true if mode.starts_with?('a')
end

private def write_blocking(handle, slice)
write_blocking(handle, slice, pos: @system_append ? UInt64::MAX : nil)
end

NOT_FOUND_ERRORS = {
WinError::ERROR_FILE_NOT_FOUND,
WinError::ERROR_PATH_NOT_FOUND,
Expand Down
13 changes: 11 additions & 2 deletions src/crystal/system/win32/file_descriptor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,17 @@ module Crystal::System::FileDescriptor
end
end

private def write_blocking(handle, slice)
ret = LibC.WriteFile(handle, slice, slice.size, out bytes_written, nil)
private def write_blocking(handle, slice, pos = nil)
overlapped = LibC::OVERLAPPED.new
if pos
overlapped.union.offset.offset = LibC::DWORD.new!(pos)
overlapped.union.offset.offsetHigh = LibC::DWORD.new!(pos >> 32)
overlapped_ptr = pointerof(overlapped)
else
overlapped_ptr = Pointer(LibC::OVERLAPPED).null
end

ret = LibC.WriteFile(handle, slice, slice.size, out bytes_written, overlapped_ptr)
if ret.zero?
case error = WinError.value
when .error_access_denied?
Expand Down
2 changes: 1 addition & 1 deletion src/file.cr
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class File < IO::FileDescriptor
def self.new(filename : Path | String, mode = "r", perm = DEFAULT_CREATE_PERMISSIONS, encoding = nil, invalid = nil, blocking = true)
filename = filename.to_s
fd = Crystal::System::File.open(filename, mode, perm: perm)
new(filename, fd, blocking: blocking, encoding: encoding, invalid: invalid)
new(filename, fd, blocking: blocking, encoding: encoding, invalid: invalid).tap { |f| f.system_set_mode(mode) }
end

getter path : String
Expand Down

0 comments on commit 4c2eaf0

Please sign in to comment.