Fix storage OS bucket not returning ErrNotExist correctly (#3188)

This PR fixes a bug of the storage bucket where it does not return an
`fs.ErrNotExist` error when it should.

It happens when both conditions met:
* The bucket is created with an absolute path, i.e.
`provider.NewReadWriteBucket("/absPath")`
* `bucket.Get(ctx, "existing.file/foo")` is called, where
`existing.file` exists as a regular file.

Fixes #3185

private/buf/bufworkspace/workspace_targeting.go

@@ -625,14 +625,6 @@ func checkForControllingWorkspaceOrV1Module(
  ignoreWorkspaceCheck bool,
 ) (buftarget.ControllingWorkspace, error) {
  path = normalpath.Normalize(path)
- // We attempt to check that the provided target path is not a file by checking the extension.
- // Any valid proto file provided as a target would have the .proto extension, so we treat
- // any path given without as a directory.
- // This could be a file without an extension, in which case an error would be returned
- // to the user when we attempt to check for a controlling workspace.
- if normalpath.Ext(path) != "" {
- path = normalpath.Dir(path)
- }
  // Keep track of any v1 module found along the way. If we find a v1 or v2 workspace, we
  // return that over the v1 module, but we return this as the fallback.
  var fallbackV1Module buftarget.ControllingWorkspace

private/pkg/storage/storageos/bucket.go

@@ -20,7 +20,6 @@ import (
  "io/fs"
  "os"
  "path/filepath"
- "strings"
 
  "github.com/bufbuild/buf/private/pkg/filepathext"
  "github.com/bufbuild/buf/private/pkg/normalpath"
@@ -284,14 +283,10 @@ func (b *bucket) validateExternalPath(path string, externalPath string) error {
  // It's important that we detect these cases so that
  // multi buckets don't unnecessarily fail when one of
  // its delegates actually defines the path.
- elements := strings.Split(normalpath.Normalize(externalPath), "/")
- if len(elements) == 1 {
- // The path is a single element, so there aren't
- // any other files to check.
- return err
- }
- for i := len(elements) - 1; i >= 0; i-- {
- parentFileInfo, err := os.Stat(filepath.Join(elements[:i]...))
+ lastParentPath := externalPath
+ parentPath := filepath.Dir(externalPath)
+ for ; parentPath != lastParentPath; lastParentPath, parentPath = parentPath, filepath.Dir(parentPath) {
+ parentFileInfo, err := os.Stat(parentPath)
  if err != nil {
  continue
  }

private/pkg/storage/storageos/storageos_test.go

@@ -15,6 +15,9 @@
 package storageos_test
 
 import (
+ "context"
+ "io/fs"
+ "os"
  "path/filepath"
  "testing"
 
@@ -37,6 +40,72 @@ func TestOS(t *testing.T) {
  testWriteBucketToReadBucket,
  true,
  )
+
+ t.Run("get_non_existent_file", func(t *testing.T) {
+ t.Parallel()
+ ctx := context.Background()
+ // Create a bucket at an absolute path.
+ tempDir := t.TempDir()
+ tempDir, err := filepath.Abs(tempDir)
+ require.NoError(t, err)
+ bucket, err := storageos.NewProvider().NewReadWriteBucket(tempDir)
+ require.NoError(t, err)
+
+ // Write a file to it.
+ writeObjectCloser, err := bucket.Put(ctx, "foo.txt")
+ require.NoError(t, err)
+ written, err := writeObjectCloser.Write([]byte(nil))
+ require.NoError(t, err)
+ require.Zero(t, written)
+ require.NoError(t, writeObjectCloser.Close())
+
+ // Try reading a file as if foo.txt is a directory.
+ _, err = bucket.Get(ctx, "foo.txt/bar.txt")
+ require.ErrorIs(t, err, fs.ErrNotExist)
+ _, err = bucket.Get(ctx, "foo.txt/bar.txt/baz.txt")
+ require.ErrorIs(t, err, fs.ErrNotExist)
+
+ // Read a file that does not exist at all.
+ _, err = bucket.Get(ctx, "baz.txt")
+ require.ErrorIs(t, err, fs.ErrNotExist)
+ })
+
+ t.Run("get_non_existent_file_symlink", func(t *testing.T) {
+ t.Parallel()
+ ctx := context.Background()
+ // Create a bucket at an absolute path.
+ actualTempDir := t.TempDir()
+ actualTempDir, err := filepath.Abs(actualTempDir)
+ require.NoError(t, err)
+ _, err = os.Create(filepath.Join(actualTempDir, "foo.txt"))
+ require.NoError(t, err)
+ tempDir := t.TempDir()
+ tempDir, err = filepath.Abs(tempDir)
+ require.NoError(t, err)
+ tempDir = filepath.Join(tempDir, "sym")
+ require.NoError(t, os.Symlink(actualTempDir, tempDir))
+ t.Cleanup(func() {
+ if err := os.Remove(tempDir); err != nil {
+ t.Error(err)
+ }
+ })
+ provider := storageos.NewProvider(storageos.ProviderWithSymlinks())
+ bucket, err := provider.NewReadWriteBucket(tempDir, storageos.ReadWriteBucketWithSymlinksIfSupported())
+ require.NoError(t, err)
+
+ _, err = bucket.Get(ctx, "foo.txt")
+ require.NoError(t, err)
+
+ // Try reading a file as if foo.txt is a directory.
+ _, err = bucket.Get(ctx, "foo.txt/bar.txt")
+ require.ErrorIs(t, err, fs.ErrNotExist)
+ _, err = bucket.Get(ctx, "foo.txt/bar.txt/baz.txt")
+ require.ErrorIs(t, err, fs.ErrNotExist)
+
+ // Read a file that does not exist at all.
+ _, err = bucket.Get(ctx, "baz.txt")
+ require.ErrorIs(t, err, fs.ErrNotExist)
+ })
 }
 
 func testNewReadBucket(t *testing.T, dirPath string, storageosProvider storageos.Provider) (storage.ReadBucket, storagetesting.GetExternalPathFunc) {