From 61c57575cd01940d06a327ce61b8923bf4a7553a Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 8 Nov 2022 16:10:47 -0800 Subject: [PATCH] os: remove special casing of NUL in Windows file operations Some file operations, notably Stat and Mkdir, special cased their behavior when operating on a file named "NUL" (case-insensitive). This check failed to account for the many other names of the NUL device, as well as other non-NUL device files: "./nul", "//./nul", "nul.txt" (on some Windows versions), "con", etc. Remove the special case. os.Mkdir("NUL") now returns no error. This is consonant with the operating system's behavior: CreateDirectory("NUL") succeeds, as does "MKDIR NUL" on the command line. os.Stat("NUL") now follows the existing path for FILE_TYPE_CHAR devices, returning a FileInfo which correctly reports the file as being a character device. os.Stat and os.File.Stat have common elements of their logic unified. For #24482. For #24556. For #56217. Change-Id: I7e70f45901127c9961166dd6dbfe0c4a10b4ab64 Reviewed-on: https://go-review.googlesource.com/c/go/+/448897 Run-TryBot: Damien Neil Reviewed-by: Bryan Mills TryBot-Result: Gopher Robot Reviewed-by: Quim Muntal --- src/os/file.go | 21 ---------------- src/os/os_test.go | 24 +++++++------------ src/os/os_windows_test.go | 44 ++++++++++++++++++---------------- src/os/stat_windows.go | 50 +++++++++++++++++++-------------------- src/os/types_windows.go | 12 ---------- 5 files changed, 57 insertions(+), 94 deletions(-) diff --git a/src/os/file.go b/src/os/file.go index c46c9030b9..0a26850c41 100644 --- a/src/os/file.go +++ b/src/os/file.go @@ -254,9 +254,6 @@ func (f *File) WriteString(s string) (n int, err error) { // bits (before umask). // If there is an error, it will be of type *PathError. func Mkdir(name string, perm FileMode) error { - if runtime.GOOS == "windows" && isWindowsNulName(name) { - return &PathError{Op: "mkdir", Path: name, Err: syscall.ENOTDIR} - } longName := fixLongPath(name) e := ignoringEINTR(func() error { return syscall.Mkdir(longName, syscallMode(perm)) @@ -591,24 +588,6 @@ func (f *File) SyscallConn() (syscall.RawConn, error) { return newRawConn(f) } -// isWindowsNulName reports whether name is os.DevNull ('NUL') on Windows. -// True is returned if name is 'NUL' whatever the case. -func isWindowsNulName(name string) bool { - if len(name) != 3 { - return false - } - if name[0] != 'n' && name[0] != 'N' { - return false - } - if name[1] != 'u' && name[1] != 'U' { - return false - } - if name[2] != 'l' && name[2] != 'L' { - return false - } - return true -} - // DirFS returns a file system (an fs.FS) for the tree of files rooted at the directory dir. // // Note that DirFS("/prefix") only guarantees that the Open calls it makes to the diff --git a/src/os/os_test.go b/src/os/os_test.go index a1b954c438..ee030a80a7 100644 --- a/src/os/os_test.go +++ b/src/os/os_test.go @@ -2012,18 +2012,8 @@ func TestSameFile(t *testing.T) { } } -func testDevNullFileInfo(t *testing.T, statname, devNullName string, fi FileInfo, ignoreCase bool) { +func testDevNullFileInfo(t *testing.T, statname, devNullName string, fi FileInfo) { pre := fmt.Sprintf("%s(%q): ", statname, devNullName) - name := filepath.Base(devNullName) - if ignoreCase { - if strings.ToUpper(fi.Name()) != strings.ToUpper(name) { - t.Errorf(pre+"wrong file name have %v want %v", fi.Name(), name) - } - } else { - if fi.Name() != name { - t.Errorf(pre+"wrong file name have %v want %v", fi.Name(), name) - } - } if fi.Size() != 0 { t.Errorf(pre+"wrong file size have %d want 0", fi.Size()) } @@ -2038,7 +2028,7 @@ func testDevNullFileInfo(t *testing.T, statname, devNullName string, fi FileInfo } } -func testDevNullFile(t *testing.T, devNullName string, ignoreCase bool) { +func testDevNullFile(t *testing.T, devNullName string) { f, err := Open(devNullName) if err != nil { t.Fatalf("Open(%s): %v", devNullName, err) @@ -2049,17 +2039,21 @@ func testDevNullFile(t *testing.T, devNullName string, ignoreCase bool) { if err != nil { t.Fatalf("Stat(%s): %v", devNullName, err) } - testDevNullFileInfo(t, "f.Stat", devNullName, fi, ignoreCase) + testDevNullFileInfo(t, "f.Stat", devNullName, fi) fi, err = Stat(devNullName) if err != nil { t.Fatalf("Stat(%s): %v", devNullName, err) } - testDevNullFileInfo(t, "Stat", devNullName, fi, ignoreCase) + testDevNullFileInfo(t, "Stat", devNullName, fi) } func TestDevNullFile(t *testing.T) { - testDevNullFile(t, DevNull, false) + testDevNullFile(t, DevNull) + if runtime.GOOS == "windows" { + testDevNullFile(t, "./nul") + testDevNullFile(t, "//./nul") + } } var testLargeWrite = flag.Bool("large_write", false, "run TestLargeWriteToConsole test that floods console with output") diff --git a/src/os/os_windows_test.go b/src/os/os_windows_test.go index 41a066dcbc..55253cdf79 100644 --- a/src/os/os_windows_test.go +++ b/src/os/os_windows_test.go @@ -891,10 +891,6 @@ func TestOneDrive(t *testing.T) { } func TestWindowsDevNullFile(t *testing.T) { - testDevNullFile(t, "NUL", true) - testDevNullFile(t, "nul", true) - testDevNullFile(t, "Nul", true) - f1, err := os.Open("NUL") if err != nil { t.Fatal(err) @@ -922,6 +918,30 @@ func TestWindowsDevNullFile(t *testing.T) { } } +func TestFileStatNUL(t *testing.T) { + f, err := os.Open("NUL") + if err != nil { + t.Fatal(err) + } + fi, err := f.Stat() + if err != nil { + t.Fatal(err) + } + if got, want := fi.Mode(), os.ModeDevice|os.ModeCharDevice|0666; got != want { + t.Errorf("Open(%q).Stat().Mode() = %v, want %v", "NUL", got, want) + } +} + +func TestStatNUL(t *testing.T) { + fi, err := os.Stat("NUL") + if err != nil { + t.Fatal(err) + } + if got, want := fi.Mode(), os.ModeDevice|os.ModeCharDevice|0666; got != want { + t.Errorf("Stat(%q).Mode() = %v, want %v", "NUL", got, want) + } +} + // TestSymlinkCreation verifies that creating a symbolic link // works on Windows when developer mode is active. // This is supported starting Windows 10 (1703, v10.0.14972). @@ -1232,19 +1252,3 @@ func TestWindowsReadlink(t *testing.T) { mklink(t, "relfilelink", "file") testReadlink(t, "relfilelink", "file") } - -// os.Mkdir(os.DevNull) fails. -func TestMkdirDevNull(t *testing.T) { - err := os.Mkdir(os.DevNull, 777) - oserr, ok := err.(*fs.PathError) - if !ok { - t.Fatalf("error (%T) is not *fs.PathError", err) - } - errno, ok := oserr.Err.(syscall.Errno) - if !ok { - t.Fatalf("error (%T) is not syscall.Errno", oserr) - } - if errno != syscall.ENOTDIR { - t.Fatalf("error %d is not syscall.ENOTDIR", errno) - } -} diff --git a/src/os/stat_windows.go b/src/os/stat_windows.go index da4c49090e..4116e77170 100644 --- a/src/os/stat_windows.go +++ b/src/os/stat_windows.go @@ -16,30 +16,11 @@ func (file *File) Stat() (FileInfo, error) { if file == nil { return nil, ErrInvalid } - if file.isdir() { // I don't know any better way to do that for directory return Stat(file.dirinfo.path) } - if isWindowsNulName(file.name) { - return &devNullStat, nil - } - - ft, err := file.pfd.GetFileType() - if err != nil { - return nil, &PathError{Op: "GetFileType", Path: file.name, Err: err} - } - switch ft { - case syscall.FILE_TYPE_PIPE, syscall.FILE_TYPE_CHAR: - return &fileStat{name: basename(file.name), filetype: ft}, nil - } - - fs, err := newFileStatFromGetFileInformationByHandle(file.name, file.pfd.Sysfd) - if err != nil { - return nil, err - } - fs.filetype = ft - return fs, err + return statHandle(file.name, file.pfd.Sysfd) } // stat implements both Stat and Lstat of a file. @@ -47,9 +28,6 @@ func stat(funcname, name string, createFileAttrs uint32) (FileInfo, error) { if len(name) == 0 { return nil, &PathError{Op: funcname, Path: name, Err: syscall.Errno(syscall.ERROR_PATH_NOT_FOUND)} } - if isWindowsNulName(name) { - return &devNullStat, nil - } namep, err := syscall.UTF16PtrFromString(fixLongPath(name)) if err != nil { return nil, &PathError{Op: funcname, Path: name, Err: err} @@ -91,14 +69,34 @@ func stat(funcname, name string, createFileAttrs uint32) (FileInfo, error) { } // Finally use CreateFile. - h, err := syscall.CreateFile(namep, 0, 0, nil, - syscall.OPEN_EXISTING, createFileAttrs, 0) + h, err := syscall.CreateFile(namep, + syscall.GENERIC_READ, + syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE, + nil, + syscall.OPEN_EXISTING, + createFileAttrs, 0) if err != nil { return nil, &PathError{Op: "CreateFile", Path: name, Err: err} } defer syscall.CloseHandle(h) + return statHandle(name, h) +} - return newFileStatFromGetFileInformationByHandle(name, h) +func statHandle(name string, h syscall.Handle) (FileInfo, error) { + ft, err := syscall.GetFileType(h) + if err != nil { + return nil, &PathError{Op: "GetFileType", Path: name, Err: err} + } + switch ft { + case syscall.FILE_TYPE_PIPE, syscall.FILE_TYPE_CHAR: + return &fileStat{name: basename(name), filetype: ft}, nil + } + fs, err := newFileStatFromGetFileInformationByHandle(name, h) + if err != nil { + return nil, err + } + fs.filetype = ft + return fs, err } // statNolog implements Stat for Windows. diff --git a/src/os/types_windows.go b/src/os/types_windows.go index 5443dfedc8..d444e8b48a 100644 --- a/src/os/types_windows.go +++ b/src/os/types_windows.go @@ -110,9 +110,6 @@ func (fs *fileStat) Size() int64 { } func (fs *fileStat) Mode() (m FileMode) { - if fs == &devNullStat { - return ModeDevice | ModeCharDevice | 0666 - } if fs.FileAttributes&syscall.FILE_ATTRIBUTE_READONLY != 0 { m |= 0444 } else { @@ -204,15 +201,6 @@ func (fs *fileStat) saveInfoFromPath(path string) error { return nil } -// devNullStat is fileStat structure describing DevNull file ("NUL"). -var devNullStat = fileStat{ - name: DevNull, - // hopefully this will work for SameFile - vol: 0, - idxhi: 0, - idxlo: 0, -} - func sameFile(fs1, fs2 *fileStat) bool { e := fs1.loadFileId() if e != nil { -- 2.50.0