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 <dneil@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>

diff --git a/src/os/file.go b/src/os/file.go
index c46c9030b91b2195567f6f002a6bd92a08121aaa..0a26850c41046e4b656610aeb64c05e0ec0d7489 100644 (file)
--- 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 a1b954c43887ef6f5cc49f530260aa9761e62013..ee030a80a7314caa6a986236de1ae427d514223f 100644 (file)
--- 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 41a066dcbcadffe814f7ef71279778b6547b5d84..55253cdf799dd54d612486aa24a9d26f6c08b9b8 100644 (file)
--- 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 da4c49090e2dce889973143a77ba6507aaddbba0..4116e77170be36cedcc51a4595767facce0ea0ca 100644 (file)
--- 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 5443dfedc8ca2113680b19639f6fd02e08de4994..d444e8b48add27e53a19e858c4ca127ab29702b6 100644 (file)
--- 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 {