]> Cypherpunks repositories - gostls13.git/commitdiff
Revert "os: check for valid Windows path when creating files"
authorGeorge Adams <georgeadams1995@gmail.com>
Tue, 19 Nov 2024 10:05:27 +0000 (10:05 +0000)
committerGopher Robot <gobot@golang.org>
Tue, 19 Nov 2024 17:26:40 +0000 (17:26 +0000)
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>
src/os/export_windows_test.go
src/os/file.go
src/os/file_nonwindows.go [deleted file]
src/os/file_windows.go
src/os/os_windows_test.go
src/os/path_windows.go
src/os/path_windows_test.go

index faab5470f75da0f2d02746728f5d84797a44c255..aefbe4033e3738d14a47c8db5e5afd20305113b6 100644 (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
 )
index c75606d749746a5fdf9f6ee740ecc4240996650a..0341469e2d84a2c3e8129df7cde4259a88da10b6 100644 (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
 }
 
diff --git a/src/os/file_nonwindows.go b/src/os/file_nonwindows.go
deleted file mode 100644 (file)
index d0e7843..0000000
+++ /dev/null
@@ -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))
-       })
-}
index 0fd6c99f9433690386b600ffe7da8b63d91cdacc..465cf5d1862beada941c3d295f8e50d2e0658987 100644 (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)
 
index 9208fe3b16e3099adfda9378e25af4caa6818261..31c379011c29526711f73832d6d0f5781af78065 100644 (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)
-               }
-       }
-}
index d72649be8605a708a190087cdd4e26abc85fb89f..f585aa5ee6ded43794cab5e7590f1f9eb5a3fd0f 100644 (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
-       }
-}
index e1f0697979958d615e4f810770708c60976daea2..3fa02e2a65b083d6454f8d3aae728d89b710078f 100644 (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)
-               }
-       }
-}