From 39fbc4c29a95510a1c62b6b57723aef496cdfbbc Mon Sep 17 00:00:00 2001
From: qmuntal <quimmuntal@gmail.com>
Date: Thu, 10 Oct 2024 11:01:16 +0200
Subject: [PATCH] syscall,os: move flags validation from os.OpenFile to
 syscall.Open

syscall.Open is the functions that maps Unix/Go flags into Windows
concepts. Part of the flag validation logic was still implemented
in os.OpenFile, move it to syscall.Open for consistency.

A nice side effect is that we don't have to translate the file name
twice in case of an access denied error.

Change-Id: I32c647a9a2a066277c78f53bacb45fb3036f6353
Reviewed-on: https://go-review.googlesource.com/c/go/+/619275
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
---
 src/os/file_windows.go              | 17 +++----------
 src/syscall/syscall_windows.go      |  7 ++++++
 src/syscall/syscall_windows_test.go | 37 ++++++++++++++++-------------
 3 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/src/os/file_windows.go b/src/os/file_windows.go
index cf652ca1bb..f8a6c09bb5 100644
--- a/src/os/file_windows.go
+++ b/src/os/file_windows.go
@@ -103,20 +103,9 @@ func openFileNolog(name string, flag int, perm FileMode) (*File, error) {
 		return nil, &PathError{Op: "open", Path: name, Err: syscall.ENOENT}
 	}
 	path := fixLongPath(name)
-	r, e := syscall.Open(path, flag|syscall.O_CLOEXEC, syscallMode(perm))
-	if e != nil {
-		// We should return EISDIR when we are trying to open a directory with write access.
-		if e == syscall.ERROR_ACCESS_DENIED && (flag&O_WRONLY != 0 || flag&O_RDWR != 0) {
-			pathp, e1 := syscall.UTF16PtrFromString(path)
-			if e1 == nil {
-				var fa syscall.Win32FileAttributeData
-				e1 = syscall.GetFileAttributesEx(pathp, syscall.GetFileExInfoStandard, (*byte)(unsafe.Pointer(&fa)))
-				if e1 == nil && fa.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
-					e = syscall.EISDIR
-				}
-			}
-		}
-		return nil, &PathError{Op: "open", Path: name, Err: e}
+	r, err := syscall.Open(path, flag|syscall.O_CLOEXEC, syscallMode(perm))
+	if err != nil {
+		return nil, &PathError{Op: "open", Path: name, Err: err}
 	}
 	return newFile(r, name, "file"), nil
 }
diff --git a/src/syscall/syscall_windows.go b/src/syscall/syscall_windows.go
index 08120b3f2a..6f6f9e4bde 100644
--- a/src/syscall/syscall_windows.go
+++ b/src/syscall/syscall_windows.go
@@ -398,6 +398,13 @@ func Open(path string, mode int, perm uint32) (fd Handle, err error) {
 	}
 	h, err := CreateFile(pathp, access, sharemode, sa, createmode, attrs, 0)
 	if err != nil {
+		if err == ERROR_ACCESS_DENIED && (mode&O_WRONLY != 0 || mode&O_RDWR != 0) {
+			// We should return EISDIR when we are trying to open a directory with write access.
+			fa, e1 := GetFileAttributes(pathp)
+			if e1 == nil && fa&FILE_ATTRIBUTE_DIRECTORY != 0 {
+				err = EISDIR
+			}
+		}
 		return InvalidHandle, err
 	}
 	if mode&O_TRUNC == O_TRUNC {
diff --git a/src/syscall/syscall_windows_test.go b/src/syscall/syscall_windows_test.go
index c26c8eac10..9773cfbfa2 100644
--- a/src/syscall/syscall_windows_test.go
+++ b/src/syscall/syscall_windows_test.go
@@ -16,24 +16,29 @@ import (
 )
 
 func TestOpen_Dir(t *testing.T) {
-	dir := t.TempDir()
+	t.Parallel()
 
-	h, err := syscall.Open(dir, syscall.O_RDONLY, 0)
-	if err != nil {
-		t.Fatalf("Open failed: %v", err)
+	dir := t.TempDir()
+	tests := []struct {
+		mode int
+		err  error
+	}{
+		{syscall.O_RDONLY, nil},
+		{syscall.O_CREAT, syscall.ERROR_ACCESS_DENIED},                    // TODO(qmuntal): should be allowed.
+		{syscall.O_RDONLY | syscall.O_CREAT, syscall.ERROR_ACCESS_DENIED}, // TODO(qmuntal): should be allowed.
+		{syscall.O_RDONLY | syscall.O_TRUNC, syscall.ERROR_ACCESS_DENIED},
+		{syscall.O_WRONLY | syscall.O_RDWR, syscall.EISDIR},
+		{syscall.O_WRONLY, syscall.EISDIR},
+		{syscall.O_RDWR, syscall.EISDIR},
 	}
-	syscall.CloseHandle(h)
-	h, err = syscall.Open(dir, syscall.O_RDONLY|syscall.O_TRUNC, 0)
-	if err == nil {
-		t.Error("Open should have failed")
-	} else {
-		syscall.CloseHandle(h)
-	}
-	h, err = syscall.Open(dir, syscall.O_RDONLY|syscall.O_CREAT, 0)
-	if err == nil {
-		t.Error("Open should have failed")
-	} else {
-		syscall.CloseHandle(h)
+	for i, tt := range tests {
+		h, err := syscall.Open(dir, tt.mode, 0)
+		if err == nil {
+			syscall.CloseHandle(h)
+		}
+		if err != tt.err {
+			t.Errorf("%d: Open got %v, want %v", i, err, tt.err)
+		}
 	}
 }