mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert "os: check for valid Windows path when creating files" 

This reverts commit CL 618496.

Reason for revert: https://github.com/golang/go/issues/54040#issuecomment-2485151973

Change-Id: I3bf27f7fdd475a005cb6aa190994153504e96fb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/629595
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
This commit is contained in:
George Adams 2024-11-19 10:05:27 +00:00 committed by Gopher Robot
parent 405a0c4ae8
commit 5deea4c242
7 changed files with 17 additions and 203 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/os/export_windows_test.go
View File

@ -7,9 +7,8 @@ package os
// Export for testing.
var (
AddExtendedPrefix = addExtendedPrefix
NewConsoleFile = newConsoleFile
CommandLineToArgv = commandLineToArgv
AllowReadDirFileID = &allowReadDirFileID
ValidatePathForCreate = validatePathForCreate
AddExtendedPrefix = addExtendedPrefix
NewConsoleFile = newConsoleFile
CommandLineToArgv = commandLineToArgv
AllowReadDirFileID = &allowReadDirFileID
)

19
src/os/file.go
View File

@ -305,18 +305,25 @@ 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 {
err := mkdir(name, perm)
if err != nil {
return &PathError{Op: "mkdir", Path: name, Err: err}
longName := fixLongPath(name)
e := ignoringEINTR(func() error {
return syscall.Mkdir(longName, syscallMode(perm))
})
if e != nil {
return &PathError{Op: "mkdir", Path: name, Err: e}
}
// mkdir(2) itself won't handle the sticky bit on *BSD and Solaris
if !supportsCreateWithStickyBit && perm&ModeSticky != 0 {
err = setStickyBit(name)
if err != nil {
e = setStickyBit(name)
if e != nil {
Remove(name)
return err
return e
}
}
return nil
}

15
src/os/file_nonwindows.go
View File

@ -1,15 +0,0 @@
// Copyright 2024 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
//go:build !windows
package os
import "syscall"
func mkdir(name string, perm FileMode) error {
return ignoringEINTR(func() error {
return syscall.Mkdir(name, syscallMode(perm))
})
}

22
src/os/file_windows.go
View File

@ -17,8 +17,6 @@ import (
"unsafe"
)
var errInvalidPath = errors.New("invalid path: cannot end with a space or period")
// This matches the value in syscall/syscall_windows.go.
const _UTIME_OMIT = -1
@ -104,9 +102,6 @@ func openFileNolog(name string, flag int, perm FileMode) (*File, error) {
if name == "" {
return nil, &PathError{Op: "open", Path: name, Err: syscall.ENOENT}
}
if flag&O_CREATE != 0 && !validatePathForCreate(name) {
return nil, &PathError{Op: "open", Path: name, Err: errInvalidPath}
}
path := fixLongPath(name)
r, err := syscall.Open(path, flag|syscall.O_CLOEXEC, syscallMode(perm))
if err != nil {
@ -119,14 +114,6 @@ func openDirNolog(name string) (*File, error) {
return openFileNolog(name, O_RDONLY, 0)
}
func mkdir(name string, perm FileMode) error {
if !validatePathForCreate(name) {
return errInvalidPath
}
longName := fixLongPath(name)
return syscall.Mkdir(longName, syscallMode(perm))
}
func (file *file) close() error {
if file == nil {
return syscall.EINVAL
@ -217,9 +204,6 @@ func Remove(name string) error {
}
func rename(oldname, newname string) error {
if !validatePathForCreate(newname) {
return &LinkError{"rename", oldname, newname, errInvalidPath}
}
e := windows.Rename(fixLongPath(oldname), fixLongPath(newname))
if e != nil {
return &LinkError{"rename", oldname, newname, e}
@ -268,9 +252,6 @@ func tempDir() string {
// Link creates newname as a hard link to the oldname file.
// If there is an error, it will be of type *LinkError.
func Link(oldname, newname string) error {
if !validatePathForCreate(newname) {
return &LinkError{"link", oldname, newname, errInvalidPath}
}
n, err := syscall.UTF16PtrFromString(fixLongPath(newname))
if err != nil {
return &LinkError{"link", oldname, newname, err}
@ -291,9 +272,6 @@ func Link(oldname, newname string) error {
// if oldname is later created as a directory the symlink will not work.
// If there is an error, it will be of type *LinkError.
func Symlink(oldname, newname string) error {
if !validatePathForCreate(newname) {
return &LinkError{"symlink", oldname, newname, errInvalidPath}
}
// '/' does not work in link's content
oldname = filepathlite.FromSlash(oldname)

73
src/os/os_windows_test.go
View File

@ -1563,76 +1563,3 @@ func TestReadDirNoFileID(t *testing.T) {
t.Errorf("SameFile(%v, %v) = false; want true", f2, f2s)
}
}
func TestOpen_InvalidPath(t *testing.T) {
dir := t.TempDir()
file, err := os.Open(dir + ".")
if err != nil {
t.Errorf("Open(%q) should have succeeded, got %v", dir+".", err)
} else {
file.Close()
}
file, err = os.Open(dir + " ")
if err != nil {
t.Errorf("Open(%q) should have succeeded, got %v", dir+" ", err)
} else {
file.Close()
}
}
func TestMkdirAll_InvalidPath(t *testing.T) {
// Parent folder contains traling spaces
path := `C:\temp\folder \this one fails`
err := os.MkdirAll(path, 0644)
if err == nil {
t.Errorf("MkdirAll(%q) should have failed", path)
} else if !strings.Contains(err.Error(), "invalid path: cannot end with a space or period") {
t.Errorf("expected errInvalidPath for path %q, got %v", path, err)
}
}
func TestCreate_InvalidPath(t *testing.T) {
testInvalidPath(t, func(_, path string) error {
_, err := os.Create(path)
return err
})
}
func TestMkdir_InvalidPath(t *testing.T) {
testInvalidPath(t, func(_, path string) error {
return os.Mkdir(path, 0644)
})
}
func TestRename_InvalidPath(t *testing.T) {
testInvalidPath(t, os.Rename)
}
func TestLink_InvalidPath(t *testing.T) {
testInvalidPath(t, os.Link)
}
func TestSymlink_InvalidPath(t *testing.T) {
testInvalidPath(t, os.Symlink)
}
func testInvalidPath(t *testing.T, fn func(src, dest string) error) {
dir := t.TempDir()
// Test invalid paths (with trailing space and period)
invalidPaths := []string{
filepath.Join(dir, "invalid_dir "), // path ending in space
filepath.Join(dir, "invalid_dir."), // path ending in period
}
for _, path := range invalidPaths {
err := fn(dir, path)
if err == nil {
t.Errorf("(%q, %q) should have failed", dir, path)
} else if !strings.Contains(err.Error(), "invalid path: cannot end with a space or period") {
t.Errorf("expected errInvalidPath for path %q, got %v", path, err)
}
}
}

29
src/os/path_windows.go
View File

@ -6,7 +6,6 @@ package os
import (
"internal/filepathlite"
"internal/stringslite"
"internal/syscall/windows"
"syscall"
)
@ -151,31 +150,3 @@ func addExtendedPrefix(path string) string {
copy(buf, prefix)
return syscall.UTF16ToString(buf)
}
// validatePathForCreate checks if a given path is valid for creation on a Windows system.
// It returns true if the path is considered valid, and false otherwise.
// The function performs the following checks:
// 1. If the path is empty, it is considered valid.
// 2. If the path starts with `\\?\` or \??\, it is considered valid without further checks.
// 3. Otherwise, a path ending with a space or . is invalid.
func validatePathForCreate(path string) bool {
// Check if the path is empty.
if len(path) == 0 {
return true
}
// Paths starting with \\?\ should be considered valid without further checks.
if stringslite.HasPrefix(path, `\\?\`) || stringslite.HasPrefix(path, `\??\`) {
return true
}
// Get the base name of the path to check only the last component.
base := filepathlite.Base(path)
// Check if the last character of the base name is a space or period, which is invalid.
switch base[len(base)-1] {
case ' ':
return false
case '.':
return base == "." || base == ".."
default:
return true
}
}

53
src/os/path_windows_test.go
View File

@ -278,56 +278,3 @@ func BenchmarkAddExtendedPrefix(b *testing.B) {
os.AddExtendedPrefix(veryLong)
}
}
func TestValidatePathForCreate(t *testing.T) {
tests := []struct {
path string
pass bool
}{
{`C:\foo`, true},
{`C:\foo\ `, false},
{`C:\foo\.`, true},
{`C:\foo.`, false},
{`C:\foo\ .`, false},
{`C:\foo \`, false},
{"C:/foo/", true},
{"C:/foo", true},
{"C:/foo/.", true},
{"C:/foo/ .", false},
{".", true},
{"..", true},
{"...", false},
{`\\?\C:\foo`, true},
{`\\?\C:\foo\ `, true},
{`\\?\C:\foo\.`, true},
{`\??\C:\foo`, true},
{`\??\C:\foo\ `, true},
{`\??\C:\foo\.`, true},
{`\\server\share\path`, true},
{"", true},
{" ", false},
{"C:.", true},
{"C: ", false},
{"C:..", true},
{"C:...", false},
{"foo:.", false},
{"C:bar:..", false},
{`\..`, true},
{`\...`, false},
{"/.", true},
{"/..", true},
{"a..", false},
{"aa..", false},
{"a ", false},
{`a\ `, false},
{`a \`, false},
{`.\`, true},
{`..\`, true},
}
for _, tt := range tests {
if os.ValidatePathForCreate(tt.path) != tt.pass {
t.Errorf("validatePathForCreate(%q) = %v, want %v", tt.path, !tt.pass, tt.pass)
}
}
}