diff --git a/.github/workflows/functional-sidecar.yml b/.github/workflows/functional-sidecar.yml index 5cf0a896..3c4573a6 100644 --- a/.github/workflows/functional-sidecar.yml +++ b/.github/workflows/functional-sidecar.yml @@ -24,7 +24,7 @@ jobs: - name: Build and Run run: | make testbin - ./runtests.sh --sidecar + ./runtests.sh --sidecar --skip-racey - name: Coverage Report run: | diff --git a/backend/posix/dir_other.go b/backend/posix/dir_other.go new file mode 100644 index 00000000..e8017434 --- /dev/null +++ b/backend/posix/dir_other.go @@ -0,0 +1,54 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//go:build !windows + +package posix + +import ( + "errors" + "os" + "syscall" + + "github.com/versity/versitygw/s3err" +) + +func handleParentDirError(_ string) error { + return s3err.GetAPIError(s3err.ErrObjectParentIsFile) +} + +// isErrNotDir reports whether err indicates that a path component is a file, +// not a directory (POSIX ENOTDIR). +func isErrNotDir(err error) bool { + return errors.Is(err, syscall.ENOTDIR) +} + +// isErrNameTooLong reports whether err indicates that a filename or path +// component is too long (POSIX ENAMETOOLONG). +func isErrNameTooLong(err error) bool { + return errors.Is(err, syscall.ENAMETOOLONG) +} + +// isErrDirNotEmpty reports whether err indicates that a directory is not empty +// (POSIX ENOTEMPTY). +func isErrDirNotEmpty(err error) bool { + return errors.Is(err, syscall.ENOTEMPTY) +} + +// openForRead opens a file for reading. On non-Windows systems, os.Open is +// sufficient because POSIX allows removing (unlinking) a file that is still +// open by another process. +func openForRead(name string) (*os.File, error) { + return os.Open(name) +} diff --git a/backend/posix/dir_windows.go b/backend/posix/dir_windows.go new file mode 100644 index 00000000..3538620c --- /dev/null +++ b/backend/posix/dir_windows.go @@ -0,0 +1,131 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//go:build windows + +package posix + +import ( + "errors" + "os" + "path/filepath" + "syscall" + + "github.com/versity/versitygw/s3err" +) + +func handleParentDirError(name string) error { + dir := filepath.Dir(name) + + // Walk up the directory hierarchy + for dir != "." && dir != "/" { + d, statErr := os.Stat(dir) + if statErr == nil { + // Path component exists + if !d.IsDir() { + // Found a file in the ancestor path + return s3err.GetAPIError(s3err.ErrObjectParentIsFile) + } + // Found a valid directory ancestor, parent truly doesn't exist + break + } + // Continue checking parent directories + dir = filepath.Dir(dir) + } + // Parent doesn't exist or is a directory, treat as ENOENT + return nil +} + +// errDirectory is Windows ERROR_DIRECTORY (267): "The directory name is invalid." +// Windows returns this when opening a path like "file/" where "file" is a regular +// file rather than a directory — the POSIX equivalent is ENOTDIR. +const errDirectory = syscall.Errno(267) + +// errInvalidName is Windows ERROR_INVALID_NAME (123): "The filename, directory +// name, or volume label syntax is incorrect." Windows returns this when a path +// component exceeds the filesystem name-length limit — the POSIX equivalent is +// ENAMETOOLONG. +const errInvalidName = syscall.Errno(123) + +// errDirNotEmpty is Windows ERROR_DIR_NOT_EMPTY (145): "The directory is not +// empty." — the POSIX equivalent is ENOTEMPTY. +const errDirNotEmpty = syscall.Errno(145) + +// isErrNameTooLong reports whether err indicates that a filename or path +// component is too long. On Windows this covers both ENAMETOOLONG +// (ERROR_FILENAME_EXCED_RANGE, 206) and ERROR_INVALID_NAME (123), which is +// what the Windows kernel returns for a 300-character filename that exceeds +// MAX_PATH. +func isErrNameTooLong(err error) bool { + if errors.Is(err, syscall.ENAMETOOLONG) { + return true + } + var sysErr syscall.Errno + if errors.As(err, &sysErr) { + return sysErr == errInvalidName + } + return false +} + +// isErrDirNotEmpty reports whether err indicates that a directory is not empty. +// On Windows this covers both ENOTEMPTY and ERROR_DIR_NOT_EMPTY (145). +func isErrDirNotEmpty(err error) bool { + if errors.Is(err, syscall.ENOTEMPTY) { + return true + } + var sysErr syscall.Errno + if errors.As(err, &sysErr) { + return sysErr == errDirNotEmpty + } + return false +} + +// isErrNotDir reports whether err indicates that a path component is a file, +// not a directory. On Windows this covers both ENOTDIR and ERROR_DIRECTORY +// because os.Open / os.Stat do not map ERROR_DIRECTORY to ENOTDIR. +func isErrNotDir(err error) bool { + if errors.Is(err, syscall.ENOTDIR) { + return true + } + var sysErr syscall.Errno + if errors.As(err, &sysErr) { + return sysErr == errDirectory + } + return false +} + +// openForRead opens a file for reading with FILE_SHARE_DELETE so that a +// concurrent DeleteObject (os.Remove) can succeed even while the file handle +// is held open for streaming the GET response body. Without this flag, +// Windows returns "The process cannot access the file because it is being +// used by another process" on the Remove call. +func openForRead(name string) (*os.File, error) { + ptr, err := syscall.UTF16PtrFromString(name) + if err != nil { + return nil, &os.PathError{Op: "open", Path: name, Err: err} + } + h, err := syscall.CreateFile( + ptr, + syscall.GENERIC_READ, + syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE|syscall.FILE_SHARE_DELETE, + nil, + syscall.OPEN_EXISTING, + syscall.FILE_ATTRIBUTE_NORMAL, + 0, + ) + if err != nil { + return nil, &os.PathError{Op: "open", Path: name, Err: err} + } + return os.NewFile(uintptr(h), name), nil +} diff --git a/backend/posix/parentdir_other.go b/backend/posix/parentdir_other.go deleted file mode 100644 index c5b67954..00000000 --- a/backend/posix/parentdir_other.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2026 Versity Software -// This file is licensed under the Apache License, Version 2.0 -// (the "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -//go:build !windows - -package posix - -import ( - "github.com/versity/versitygw/s3err" -) - -func handleParentDirError(_ string) error { - return s3err.GetAPIError(s3err.ErrObjectParentIsFile) -} diff --git a/backend/posix/parentdir_windows.go b/backend/posix/parentdir_windows.go deleted file mode 100644 index d9d96c2a..00000000 --- a/backend/posix/parentdir_windows.go +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2026 Versity Software -// This file is licensed under the Apache License, Version 2.0 -// (the "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -//go:build windows - -package posix - -import ( - "os" - "path/filepath" - - "github.com/versity/versitygw/s3err" -) - -func handleParentDirError(name string) error { - dir := filepath.Dir(name) - - // Walk up the directory hierarchy - for dir != "." && dir != "/" { - d, statErr := os.Stat(dir) - if statErr == nil { - // Path component exists - if !d.IsDir() { - // Found a file in the ancestor path - return s3err.GetAPIError(s3err.ErrObjectParentIsFile) - } - // Found a valid directory ancestor, parent truly doesn't exist - break - } - // Continue checking parent directories - dir = filepath.Dir(dir) - } - // Parent doesn't exist or is a directory, treat as ENOENT - return nil -} diff --git a/backend/posix/posix.go b/backend/posix/posix.go index d8324e29..401e102a 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -370,7 +370,7 @@ func (p *Posix) doesBucketAndObjectExist(bucket, object string) error { } _, err = os.Stat(filepath.Join(bucket, object)) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil { @@ -1058,7 +1058,7 @@ func (p *Posix) ensureNotDeleteMarker(bucket, object, versionId string) error { // data file simply doesn't exist — the two cases are indistinguishable // from metadata alone. Verify the data file directly so callers // receive the correct NoSuchVersion / NoSuchKey error. - if _, statErr := os.Stat(filepath.Join(bucket, object)); errors.Is(statErr, fs.ErrNotExist) || errors.Is(statErr, syscall.ENOTDIR) { + if _, statErr := os.Stat(filepath.Join(bucket, object)); errors.Is(statErr, fs.ErrNotExist) || isErrNotDir(statErr) { if versionId != "" { return s3err.GetAPIError(s3err.ErrNoSuchVersion) } @@ -1066,7 +1066,7 @@ func (p *Posix) ensureNotDeleteMarker(bucket, object, versionId string) error { } _, err := p.meta.RetrieveAttribute(nil, bucket, object, deleteMarkerKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return s3err.GetAPIError(s3err.ErrNoSuchVersion) } @@ -1085,7 +1085,7 @@ func (p *Posix) ensureNotDeleteMarker(bucket, object, versionId string) error { // Check if the given object is a delete marker func (p *Posix) isObjDeleteMarker(bucket, object string) (bool, error) { _, err := p.meta.RetrieveAttribute(nil, bucket, object, deleteMarkerKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return false, s3err.GetAPIError(s3err.ErrNoSuchKey) } if errors.Is(err, meta.ErrNoSuchKey) { @@ -3269,7 +3269,7 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput) if errors.Is(err, fs.ErrNotExist) { return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrNoSuchUpload) } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrKeyTooLong) } if err != nil { @@ -3340,7 +3340,7 @@ func (p *Posix) UploadPartCopy(ctx context.Context, upi *s3.UploadPartCopyInput) } return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrNoSuchKey) } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return s3response.CopyPartResult{}, s3err.GetAPIError(s3err.ErrKeyTooLong) } if err != nil { @@ -3748,10 +3748,10 @@ func (p *Posix) PutObjectWithPostFunc(ctx context.Context, po s3response.PutObje _ = p.meta.DeleteAttribute(*po.Bucket, *po.Key, objectRetentionKey) } } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return s3response.PutObjectOutput{}, s3err.GetAPIError(s3err.ErrKeyTooLong) } - if errors.Is(err, syscall.ENOTDIR) { + if isErrNotDir(err) { parentErr := handleParentDirError(name) if parentErr != nil { return s3response.PutObjectOutput{}, parentErr @@ -4038,11 +4038,11 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( if getString(input.VersionId) == "" { // if the versionId is not specified, make the current version a delete marker fi, err := os.Stat(objpath) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { // AWS returns success if the object does not exist return &s3.DeleteObjectOutput{}, nil } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return nil, s3err.GetAPIError(s3err.ErrKeyTooLong) } if err != nil { @@ -4105,7 +4105,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( versionPath := p.genObjVersionPath(bucket, object) vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { // AWS returns success if the object does not exist return &s3.DeleteObjectOutput{ VersionId: input.VersionId, @@ -4122,7 +4122,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( // but "foo" is a regular file (not a directory), the path cannot // contain any object. _, statErr := os.Stat(filepath.Join(bucket, object)) - if errors.Is(statErr, fs.ErrNotExist) || errors.Is(statErr, syscall.ENOTDIR) { + if errors.Is(statErr, fs.ErrNotExist) || isErrNotDir(statErr) { return &s3.DeleteObjectOutput{VersionId: input.VersionId}, nil } vId = []byte(nullVersionId) @@ -4246,10 +4246,10 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( isDelMarker, _ := p.isObjDeleteMarker(versionPath, *input.VersionId) err = os.Remove(filepath.Join(versionPath, *input.VersionId)) - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return nil, s3err.GetAPIError(s3err.ErrKeyTooLong) } - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId) } if err != nil { @@ -4267,10 +4267,10 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( } fi, err := os.Stat(objpath) - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return nil, s3err.GetAPIError(s3err.ErrKeyTooLong) } - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { // AWS returns success if the object does not exist return &s3.DeleteObjectOutput{}, nil } @@ -4300,7 +4300,7 @@ func (p *Posix) DeleteObject(ctx context.Context, input *s3.DeleteObjectInput) ( if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } - if errors.Is(err, syscall.ENOTEMPTY) { + if isErrDirNotEmpty(err) { // If the directory object has been uploaded explicitly // remove the directory object (remove the ETag) _, err = p.meta.RetrieveAttribute(nil, objpath, "", etagkey) @@ -4458,7 +4458,7 @@ func (p *Posix) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.Ge object := *input.Key if versionId != "" { vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -4477,13 +4477,13 @@ func (p *Posix) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.Ge objPath := filepath.Join(bucket, object) fid, err := os.Stat(objPath) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return nil, s3err.GetAPIError(s3err.ErrNoSuchVersion) } return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return nil, s3err.GetAPIError(s3err.ErrKeyTooLong) } if err != nil { @@ -4616,7 +4616,10 @@ func (p *Posix) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.Ge versionId = string(vId) } - f, err := os.Open(objPath) + // openForRead opens with FILE_SHARE_DELETE on Windows so that a concurrent + // DeleteObject can call os.Remove on this file while the GET response body + // is still being streamed. On POSIX, os.Open is sufficient. + f, err := openForRead(objPath) if errors.Is(err, fs.ErrNotExist) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } @@ -4625,13 +4628,13 @@ func (p *Posix) GetObject(ctx context.Context, input *s3.GetObjectInput) (*s3.Ge } fi, err := f.Stat() - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId) } return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return nil, s3err.GetAPIError(s3err.ErrKeyTooLong) } if err != nil { @@ -4796,7 +4799,7 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. if versionId != "" { vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -4816,13 +4819,13 @@ func (p *Posix) HeadObject(ctx context.Context, input *s3.HeadObjectInput) (*s3. objPath := filepath.Join(bucket, object) fi, err := os.Stat(objPath) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return nil, s3err.GetAPIError(s3err.ErrNoSuchVersion) } return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return nil, s3err.GetAPIError(s3err.ErrKeyTooLong) } if err != nil { @@ -5142,7 +5145,7 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrInvalidVersionId) } vId, err := p.meta.RetrieveAttribute(nil, srcBucket, srcObject, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -5165,13 +5168,13 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput objPath := joinPathWithTrailer(srcBucket, srcObject) f, err := os.Open(objPath) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if p.versioningEnabled() && vEnabled { return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchVersion) } return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchKey) } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrKeyTooLong) } if err != nil { @@ -5312,7 +5315,7 @@ func (p *Posix) CopyObject(ctx context.Context, input s3response.CopyObjectInput b, _ := p.meta.RetrieveAttribute(nil, dstBucket, dstObject, etagkey) etag = string(b) vId, _ := p.meta.RetrieveAttribute(nil, dstBucket, dstObject, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return s3response.CopyObjectOutput{}, s3err.GetAPIError(s3err.ErrNoSuchKey) } version = backend.GetPtrFromString(string(vId)) @@ -5846,10 +5849,10 @@ func (p *Posix) GetObjectTagging(ctx context.Context, bucket, object, versionId if versionId == "" { _, err = os.Stat(filepath.Join(bucket, object)) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return nil, s3err.GetAPIError(s3err.ErrKeyTooLong) } if err != nil { @@ -5863,7 +5866,7 @@ func (p *Posix) GetObjectTagging(ctx context.Context, bucket, object, versionId return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId) } vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -5887,7 +5890,7 @@ func (p *Posix) GetObjectTagging(ctx context.Context, bucket, object, versionId func (p *Posix) getAttrTags(bucket, object, versionId string) (map[string]string, error) { tags := make(map[string]string) b, err := p.meta.RetrieveAttribute(nil, bucket, object, tagHdr) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return nil, s3err.GetAPIError(s3err.ErrNoSuchVersion) } @@ -5936,10 +5939,10 @@ func (p *Posix) PutObjectTagging(ctx context.Context, bucket, object, versionId if versionId == "" { _, err = os.Stat(filepath.Join(bucket, object)) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return s3err.GetAPIError(s3err.ErrNoSuchKey) } - if errors.Is(err, syscall.ENAMETOOLONG) { + if isErrNameTooLong(err) { return s3err.GetAPIError(s3err.ErrKeyTooLong) } if err != nil { @@ -5953,7 +5956,7 @@ func (p *Posix) PutObjectTagging(ctx context.Context, bucket, object, versionId return s3err.GetAPIError(s3err.ErrInvalidVersionId) } vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -5973,7 +5976,7 @@ func (p *Posix) PutObjectTagging(ctx context.Context, bucket, object, versionId if tags == nil { err = p.meta.DeleteAttribute(bucket, object, tagHdr) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return s3err.GetAPIError(s3err.ErrNoSuchVersion) } @@ -5994,7 +5997,7 @@ func (p *Posix) PutObjectTagging(ctx context.Context, bucket, object, versionId } err = p.meta.StoreAttribute(nil, bucket, object, tagHdr, b) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return s3err.GetAPIError(s3err.ErrNoSuchVersion) } @@ -6295,7 +6298,7 @@ func (p *Posix) PutObjectLegalHold(ctx context.Context, bucket, object, versionI return s3err.GetAPIError(s3err.ErrInvalidVersionId) } vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -6314,7 +6317,7 @@ func (p *Posix) PutObjectLegalHold(ctx context.Context, bucket, object, versionI } err = p.meta.StoreAttribute(nil, bucket, object, objectLegalHoldKey, statusData) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return s3err.GetAPIError(s3err.ErrNoSuchVersion) } @@ -6356,7 +6359,7 @@ func (p *Posix) GetObjectLegalHold(ctx context.Context, bucket, object, versionI return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId) } vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -6375,7 +6378,7 @@ func (p *Posix) GetObjectLegalHold(ctx context.Context, bucket, object, versionI } data, err := p.meta.RetrieveAttribute(nil, bucket, object, objectLegalHoldKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return nil, s3err.GetAPIError(s3err.ErrNoSuchVersion) } @@ -6422,7 +6425,7 @@ func (p *Posix) PutObjectRetention(ctx context.Context, bucket, object, versionI return s3err.GetAPIError(s3err.ErrInvalidVersionId) } vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -6477,7 +6480,7 @@ func (p *Posix) GetObjectRetention(ctx context.Context, bucket, object, versionI return nil, s3err.GetAPIError(s3err.ErrInvalidVersionId) } vId, err := p.meta.RetrieveAttribute(nil, bucket, object, versionIdKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { return nil, s3err.GetAPIError(s3err.ErrNoSuchKey) } if err != nil && !errors.Is(err, meta.ErrNoSuchKey) { @@ -6496,7 +6499,7 @@ func (p *Posix) GetObjectRetention(ctx context.Context, bucket, object, versionI } data, err := p.meta.RetrieveAttribute(nil, bucket, object, objectRetentionKey) - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + if errors.Is(err, fs.ErrNotExist) || isErrNotDir(err) { if versionId != "" { return nil, s3err.GetAPIError(s3err.ErrNoSuchVersion) } diff --git a/runtests.ps1 b/runtests.ps1 index 42cf700f..6d08b81e 100644 --- a/runtests.ps1 +++ b/runtests.ps1 @@ -67,9 +67,9 @@ if ($gwProc.HasExited) { } Invoke-GwTest -Description "full flow tests" -GatewayProc $gwProc ` - -TestArgs @("-a", "user", "-s", "pass", "-e", "http://127.0.0.1:7070", "full-flow", "--parallel") + -TestArgs @("-a", "user", "-s", "pass", "-e", "http://127.0.0.1:7070", "full-flow", "--parallel", "--windows-test-mode") Invoke-GwTest -Description "posix tests" -GatewayProc $gwProc ` - -TestArgs @("-a", "user", "-s", "pass", "-e", "http://127.0.0.1:7070", "posix") + -TestArgs @("-a", "user", "-s", "pass", "-e", "http://127.0.0.1:7070", "posix", "--windows-test-mode") Invoke-GwTest -Description "iam tests" -GatewayProc $gwProc ` -TestArgs @("-a", "user", "-s", "pass", "-e", "http://127.0.0.1:7070", "iam") @@ -90,9 +90,9 @@ if ($gwHttpsProc.HasExited) { } Invoke-GwTest -Description "https full flow tests" -GatewayProc $gwHttpsProc ` - -TestArgs @("--allow-insecure", "-a", "user", "-s", "pass", "-e", "https://127.0.0.1:7071", "full-flow", "--parallel") + -TestArgs @("--allow-insecure", "-a", "user", "-s", "pass", "-e", "https://127.0.0.1:7071", "full-flow", "--parallel", "--windows-test-mode") Invoke-GwTest -Description "https posix tests" -GatewayProc $gwHttpsProc ` - -TestArgs @("--allow-insecure", "-a", "user", "-s", "pass", "-e", "https://127.0.0.1:7071", "posix") + -TestArgs @("--allow-insecure", "-a", "user", "-s", "pass", "-e", "https://127.0.0.1:7071", "posix", "--windows-test-mode") Invoke-GwTest -Description "https iam tests" -GatewayProc $gwHttpsProc ` -TestArgs @("--allow-insecure", "-a", "user", "-s", "pass", "-e", "https://127.0.0.1:7071", "iam") @@ -113,9 +113,9 @@ if ($gwVsProc.HasExited) { } Invoke-GwTest -Description "versioning-enabled full-flow tests" -GatewayProc $gwVsProc ` - -TestArgs @("-a", "user", "-s", "pass", "-e", "http://127.0.0.1:7072", "full-flow", "-vs", "--parallel") + -TestArgs @("-a", "user", "-s", "pass", "-e", "http://127.0.0.1:7072", "full-flow", "-vs", "--parallel", "--windows-test-mode") Invoke-GwTest -Description "versioning-enabled posix tests" -GatewayProc $gwVsProc ` - -TestArgs @("-a", "user", "-s", "pass", "-e", "http://127.0.0.1:7072", "posix", "-vs") + -TestArgs @("-a", "user", "-s", "pass", "-e", "http://127.0.0.1:7072", "posix", "-vs", "--windows-test-mode") Stop-Process -Id $gwVsProc.Id -Force -ErrorAction SilentlyContinue @@ -135,9 +135,9 @@ if ($gwVsHttpsProc.HasExited) { } Invoke-GwTest -Description "versioning-enabled https full-flow tests" -GatewayProc $gwVsHttpsProc ` - -TestArgs @("--allow-insecure", "-a", "user", "-s", "pass", "-e", "https://127.0.0.1:7073", "full-flow", "-vs", "--parallel") + -TestArgs @("--allow-insecure", "-a", "user", "-s", "pass", "-e", "https://127.0.0.1:7073", "full-flow", "-vs", "--parallel", "--windows-test-mode") Invoke-GwTest -Description "versioning-enabled https posix tests" -GatewayProc $gwVsHttpsProc ` - -TestArgs @("--allow-insecure", "-a", "user", "-s", "pass", "-e", "https://127.0.0.1:7073", "posix", "-vs") + -TestArgs @("--allow-insecure", "-a", "user", "-s", "pass", "-e", "https://127.0.0.1:7073", "posix", "-vs", "--windows-test-mode") Stop-Process -Id $gwVsHttpsProc.Id -Force -ErrorAction SilentlyContinue diff --git a/tests/integration/PutObject.go b/tests/integration/PutObject.go index 1a01c5aa..22f028fc 100644 --- a/tests/integration/PutObject.go +++ b/tests/integration/PutObject.go @@ -19,6 +19,7 @@ import ( "context" "fmt" "net/http" + "sort" "strings" "time" @@ -50,10 +51,21 @@ func PutObject_special_chars(s *S3Conf) error { "my?key", "my^key", "my{}key", "my%key", "my`key", "my[]key", "my~key", "my<>key", "my|key", "my#key", } - if !s.azureTests { - // azure currently can't handle backslashes in object names + if !s.azureTests && !s.windowsTests { + // azure and windows cannot handle backslashes in object names: + // on Windows, '\' is a path separator so 'my\key' is stored as 'my/key' objnames = append(objnames, "my\\key") } + if s.windowsTests { + // ':', '?', '<', '>', '|', '*' are not valid filename characters on Windows + var filtered []string + for _, name := range objnames { + if !strings.ContainsAny(name, `:?<>|*`) { + filtered = append(filtered, name) + } + } + objnames = filtered + } return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { objs, err := putObjects(s3client, objnames, bucket) @@ -1105,19 +1117,30 @@ func PutObject_false_negative_object_names(s *S3Conf) error { testName := "PutObject_false_negative_object_names" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { objs := []string{ - "%252e%252e%252fetc/passwd", // double encoding - "%2e%2e/%2e%2e/%2e%2e/.ssh/id_rsa", // double URL-encoded - "%u002e%u002e/%u002e%u002e/etc/passwd", // unicode escape - "..%2f..%2f..%2fsecret/file.txt", // URL-encoded - "..%c0%af..%c0%afetc/passwd", // UTF-8 overlong trick - ".../.../.../target.txt", - "..\\u2215..\\u2215etc/passwd", // Unicode division slash - "dir/%20../file.txt", // encoded space + "%252e%252e%252fetc/passwd", // double encoding + "%2e%2e/%2e%2e/%2e%2e/.ssh/id_rsa", // double URL-encoded + "%u002e%u002e/%u002e%u002e/etc/passwd", // unicode escape + "..%2f..%2f..%2fsecret/file.txt", // URL-encoded + "..%c0%af..%c0%afetc/passwd", // UTF-8 overlong trick "dir/%c0%ae%c0%ae/%c0%ae%c0%ae/etc/passwd", // overlong UTF-8 encoding - "logs/latest -> /etc/passwd", // symlink attacks //TODO: add this test case in advanced routing // "/etc/passwd" // absolute path injection } + if !s.windowsTests { + // Windows strips trailing dots from path components, making '...' an + // invalid directory name (the filesystem rejects MkdirAll with it). + objs = append(objs, ".../.../.../target.txt") + objs = append(objs, "dir/%20../file.txt") // encoded space + // literal backslashes are treated as path separators on Windows + objs = append(objs, "..\\u2215..\\u2215etc/passwd") // Unicode division slash + // On Windows, '>' is an invalid filename character. The key + // 'logs/latest -> /etc/passwd' creates a directory component + // 'latest -> ' (containing '>') which MkdirAll rejects. + objs = append(objs, "logs/latest -> /etc/passwd") // symlink attacks + } + + sort.Strings(objs) + _, err := putObjects(s3client, objs, bucket) if err != nil { return err diff --git a/tests/integration/presigned_urls.go b/tests/integration/presigned_urls.go index ac9597ca..a2bafc3f 100644 --- a/tests/integration/presigned_urls.go +++ b/tests/integration/presigned_urls.go @@ -786,6 +786,10 @@ func PresignedAuth_Put_GetObject_with_UTF8_chars(s *S3Conf) error { testName := "PresignedAuth_Put_GetObject_with_UTF8_chars" return presignedAuthHandler(s, testName, func(client *s3.PresignClient, bucket string) error { obj := "my-$%^&*;" + if s.windowsTests { + // '*' is not a valid filename character on Windows + obj = "my-$%^&;" + } ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) v4req, err := client.PresignPutObject(ctx, &s3.PutObjectInput{Bucket: &bucket, Key: &obj}) diff --git a/tests/integration/utils.go b/tests/integration/utils.go index d1489702..a4692319 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -1410,8 +1410,17 @@ func listBuckets(s *S3Conf) error { const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" +// randCounter is an atomic counter used by genRandString. It is seeded once +// with time.Now().UnixNano() so that values are unique even when many goroutines +// call genRandString concurrently on systems with low-resolution clocks. +var randCounter = atomic.Uint64{} + +func init() { + randCounter.Store(uint64(time.Now().UnixNano())) +} + func genRandString(length int) string { - source := rnd.NewSource(time.Now().UnixNano()) + source := rnd.NewSource(int64(randCounter.Add(1))) random := rnd.New(source) result := make([]byte, length) for i := range result {