From f141cb99a177004d6cdbeee9dc46369837acea6a Mon Sep 17 00:00:00 2001 From: Manju Karikatti Date: Thu, 30 Apr 2026 13:57:29 +0100 Subject: [PATCH 1/2] use NtSetInformationFile if os.Rename fails Signed-off-by: Manju Karikatti --- atomicwriter/atomicwriter.go | 2 +- atomicwriter/atomicwriter_unix.go | 9 +++ atomicwriter/atomicwriter_windows.go | 107 +++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 atomicwriter/atomicwriter_unix.go create mode 100644 atomicwriter/atomicwriter_windows.go diff --git a/atomicwriter/atomicwriter.go b/atomicwriter/atomicwriter.go index d0d3be88..c29ebdb5 100644 --- a/atomicwriter/atomicwriter.go +++ b/atomicwriter/atomicwriter.go @@ -152,7 +152,7 @@ func (w *atomicFileWriter) Close() (retErr error) { return err } if w.writeErr == nil && w.written { - return os.Rename(w.f.Name(), w.fn) + return atomicwriterRenameAt(w.f.Name(), w.fn) } return nil } diff --git a/atomicwriter/atomicwriter_unix.go b/atomicwriter/atomicwriter_unix.go new file mode 100644 index 00000000..55d49096 --- /dev/null +++ b/atomicwriter/atomicwriter_unix.go @@ -0,0 +1,9 @@ +//go:build !windows + +package atomicwriter + +import "os" + +func atomicwriterRenameAt(oldpath, newpath string) error { + return os.Rename(oldpath, newpath) +} diff --git a/atomicwriter/atomicwriter_windows.go b/atomicwriter/atomicwriter_windows.go new file mode 100644 index 00000000..a77064b5 --- /dev/null +++ b/atomicwriter/atomicwriter_windows.go @@ -0,0 +1,107 @@ +package atomicwriter + +import ( + "os" + "unsafe" + + "golang.org/x/sys/windows" +) + +// fileRenameInformation is the FILE_RENAME_INFORMATION structure used by +// NtSetInformationFile to rename a file. FileName is a variable-length +// field; callers must allocate a buffer large enough to hold the full name. +type fileRenameInformation struct { + ReplaceIfExists uint32 + RootDirectory windows.Handle + FileNameLength uint32 + FileName [1]uint16 +} + +// fileRenameInformationEx is the FILE_RENAME_INFORMATION_EX structure used by +// NtSetInformationFile to rename a file. +type fileRenameInformationEx struct { + Flags uint32 + RootDirectory windows.Handle + FileNameLength uint32 + FileName [1]uint16 +} + +// atomicwriterRenameAt renames oldpath to newpath using os.Rename. If it fails, +// it attempts to use NtSetInformationFile with FILE_RENAME_POSIX_SEMANTICS, +// which allows atomic replacement of a file even when the destination +// is open by another process. +func atomicwriterRenameAt(oldpath, newpath string) error { + + err := os.Rename(oldpath, newpath) + if err == nil { + return nil + } + // Open the source file requesting DELETE access so we can rename it. + srcPtr, err := windows.UTF16PtrFromString(oldpath) + if err != nil { + return &os.PathError{Op: "rename", Path: oldpath, Err: err} + } + handle, err := windows.CreateFile( + srcPtr, + windows.DELETE, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_ATTRIBUTE_NORMAL, + 0, + ) + if err != nil { + return &os.PathError{Op: "rename", Path: oldpath, Err: err} + } + defer windows.CloseHandle(handle) + + // NtSetInformationFile requires an absolute NT path (\??\C:\...) when + // RootDirectory is NULL. + ntNewPath := `\??\` + newpath + newPathUTF16, err := windows.UTF16FromString(ntNewPath) + if err != nil { + return &os.PathError{Op: "rename", Path: newpath, Err: err} + } + + fileNameLen := len(newPathUTF16)*2 - 2 // byte length, excluding null terminator + renameInfoEx := fileRenameInformationEx{ + Flags: windows.FILE_RENAME_REPLACE_IF_EXISTS | + windows.FILE_RENAME_POSIX_SEMANTICS, + } + var dummyEx fileRenameInformationEx + bufferSizeEx := int(unsafe.Offsetof(dummyEx.FileName)) + fileNameLen + bufferEx := make([]byte, bufferSizeEx) + infoEx := (*fileRenameInformationEx)(unsafe.Pointer(&bufferEx[0])) + infoEx.Flags = renameInfoEx.Flags + infoEx.FileNameLength = uint32(fileNameLen) + copy((*[windows.MAX_LONG_PATH]uint16)(unsafe.Pointer(&infoEx.FileName[0]))[:fileNameLen/2:fileNameLen/2], newPathUTF16) + + const ( + FileRenameInformation = 10 + FileRenameInformationEx = 65 + ) + var iosbEx windows.IO_STATUS_BLOCK + + err = windows.NtSetInformationFile(handle, &iosbEx, &bufferEx[0], uint32(bufferSizeEx), FileRenameInformationEx) + if err == nil { + return nil + } + + // If the extended rename fails, fall back to the original FILE_RENAME_INFORMATION + // which is supported on older versions of Windows. This may fail if the destination + // file is open by another process, but there's no way to detect that beforehand. + + var dummy fileRenameInformation + bufferSize := int(unsafe.Offsetof(dummy.FileName)) + fileNameLen + buffer := make([]byte, bufferSize) + info := (*fileRenameInformation)(unsafe.Pointer(&buffer[0])) + info.ReplaceIfExists = windows.FILE_RENAME_REPLACE_IF_EXISTS | windows.FILE_RENAME_POSIX_SEMANTICS + info.FileNameLength = uint32(fileNameLen) + copy((*[windows.MAX_LONG_PATH]uint16)(unsafe.Pointer(&info.FileName[0]))[:fileNameLen/2:fileNameLen/2], newPathUTF16) + + var iosb windows.IO_STATUS_BLOCK + if err := windows.NtSetInformationFile(handle, &iosb, &buffer[0], uint32(bufferSize), FileRenameInformation); err != nil { + return &os.PathError{Op: "rename", Path: newpath, Err: err} + } + return nil +} From 14bc07463c2c44eeed5b61a0ceffecb191fc7308 Mon Sep 17 00:00:00 2001 From: Manju Karikatti Date: Sun, 14 Jun 2026 14:52:31 +0100 Subject: [PATCH 2/2] Added unit tests --- atomicwriter/atomicwriter.go | 2 +- atomicwriter/atomicwriter_test.go | 57 ++++++++++++++++++++++++++++ atomicwriter/atomicwriter_unix.go | 2 +- atomicwriter/atomicwriter_windows.go | 4 +- 4 files changed, 61 insertions(+), 4 deletions(-) diff --git a/atomicwriter/atomicwriter.go b/atomicwriter/atomicwriter.go index c29ebdb5..38b80b17 100644 --- a/atomicwriter/atomicwriter.go +++ b/atomicwriter/atomicwriter.go @@ -152,7 +152,7 @@ func (w *atomicFileWriter) Close() (retErr error) { return err } if w.writeErr == nil && w.written { - return atomicwriterRenameAt(w.f.Name(), w.fn) + return atomicwriterRename(w.f.Name(), w.fn) } return nil } diff --git a/atomicwriter/atomicwriter_test.go b/atomicwriter/atomicwriter_test.go index e98f7f33..98a385d2 100644 --- a/atomicwriter/atomicwriter_test.go +++ b/atomicwriter/atomicwriter_test.go @@ -262,6 +262,63 @@ func TestWriteFile(t *testing.T) { }) } +func TestAtomicWriterRename(t *testing.T) { + t.Run("rename to new path", func(t *testing.T) { + tmpDir := t.TempDir() + srcPath := filepath.Join(tmpDir, "src.txt") + dstPath := filepath.Join(tmpDir, "dst.txt") + srcContent := []byte("source content") + fileMode := testMode() + if err := os.WriteFile(srcPath, srcContent, fileMode); err != nil { + t.Fatalf("Error writing source file: %v", err) + } + + if err := atomicwriterRename(srcPath, dstPath); err != nil { + t.Fatalf("Error renaming file: %v", err) + } + + if _, err := os.Stat(srcPath); !errors.Is(err, os.ErrNotExist) { + t.Errorf("Source file should no longer exist, got err: %v", err) + } + assertFile(t, dstPath, srcContent, fileMode) + }) + t.Run("replace existing destination", func(t *testing.T) { + tmpDir := t.TempDir() + srcPath := filepath.Join(tmpDir, "src.txt") + dstPath := filepath.Join(tmpDir, "dst.txt") + srcContent := []byte("new content") + fileMode := testMode() + if err := os.WriteFile(srcPath, srcContent, fileMode); err != nil { + t.Fatalf("Error writing source file: %v", err) + } + if err := os.WriteFile(dstPath, []byte("original content"), fileMode); err != nil { + t.Fatalf("Error writing destination file: %v", err) + } + + if err := atomicwriterRename(srcPath, dstPath); err != nil { + t.Fatalf("Error renaming file: %v", err) + } + + if _, err := os.Stat(srcPath); !errors.Is(err, os.ErrNotExist) { + t.Errorf("Source file should no longer exist, got err: %v", err) + } + assertFile(t, dstPath, srcContent, fileMode) + }) + t.Run("missing source", func(t *testing.T) { + tmpDir := t.TempDir() + srcPath := filepath.Join(tmpDir, "missing.txt") + dstPath := filepath.Join(tmpDir, "dst.txt") + + err := atomicwriterRename(srcPath, dstPath) + if !errors.Is(err, os.ErrNotExist) { + t.Errorf("Should produce a 'not found' error, but got %[1]T (%[1]v)", err) + } + if _, err := os.Stat(dstPath); !errors.Is(err, os.ErrNotExist) { + t.Errorf("Destination should not exist, got err: %v", err) + } + }) +} + func TestWriteSetCommit(t *testing.T) { tmpDir := t.TempDir() diff --git a/atomicwriter/atomicwriter_unix.go b/atomicwriter/atomicwriter_unix.go index 55d49096..5001ef93 100644 --- a/atomicwriter/atomicwriter_unix.go +++ b/atomicwriter/atomicwriter_unix.go @@ -4,6 +4,6 @@ package atomicwriter import "os" -func atomicwriterRenameAt(oldpath, newpath string) error { +func atomicwriterRename(oldpath, newpath string) error { return os.Rename(oldpath, newpath) } diff --git a/atomicwriter/atomicwriter_windows.go b/atomicwriter/atomicwriter_windows.go index a77064b5..d3a5554f 100644 --- a/atomicwriter/atomicwriter_windows.go +++ b/atomicwriter/atomicwriter_windows.go @@ -26,11 +26,11 @@ type fileRenameInformationEx struct { FileName [1]uint16 } -// atomicwriterRenameAt renames oldpath to newpath using os.Rename. If it fails, +// atomicwriterRename renames oldpath to newpath using os.Rename. If it fails, // it attempts to use NtSetInformationFile with FILE_RENAME_POSIX_SEMANTICS, // which allows atomic replacement of a file even when the destination // is open by another process. -func atomicwriterRenameAt(oldpath, newpath string) error { +func atomicwriterRename(oldpath, newpath string) error { err := os.Rename(oldpath, newpath) if err == nil {