From 075e0df5df3ec08d6eadac0ac5d2e4929efcdd3a Mon Sep 17 00:00:00 2001 From: Aditya Pratap Singh Hada <49001649+apshada@users.noreply.github.com> Date: Tue, 11 Jun 2024 17:32:57 +0530 Subject: [PATCH] [MM-57034]: Check for s3 object is File or Directory (#26837) * check for path when number of object is one * add test and updated condition to check for path * updated test and removed string trim * using exported method --------- Co-authored-by: Mattermost Build --- server/platform/shared/filestore/s3store.go | 8 +++ .../platform/shared/filestore/s3store_test.go | 59 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/server/platform/shared/filestore/s3store.go b/server/platform/shared/filestore/s3store.go index aa386b06fbc..4a94c2824f6 100644 --- a/server/platform/shared/filestore/s3store.go +++ b/server/platform/shared/filestore/s3store.go @@ -8,6 +8,7 @@ import ( "context" "crypto/tls" "io" + "io/fs" "net/http" "net/url" "os" @@ -641,6 +642,7 @@ func (b *S3FileBackend) listDirectory(path string, recursion bool) ([]string, er var paths []string ctx, cancel := context.WithTimeout(context.Background(), b.timeout) defer cancel() + var count int for object := range b.client.ListObjects(ctx, b.bucket, opts) { if object.Err != nil { return nil, errors.Wrapf(object.Err, "unable to list the directory %s", path) @@ -652,6 +654,12 @@ func (b *S3FileBackend) listDirectory(path string, recursion bool) ([]string, er if trimmed != "" { paths = append(paths, trimmed) } + count++ + } + // Check if only one item was returned and it matches the path prefix + if count == 1 && len(paths) > 0 && strings.TrimRight(path, "/") == paths[0] { + // Return a fs.PathError to maintain consistency + return nil, &fs.PathError{Op: "readdir", Path: path, Err: fs.ErrNotExist} } return paths, nil diff --git a/server/platform/shared/filestore/s3store_test.go b/server/platform/shared/filestore/s3store_test.go index 598a5303bf1..3a89caf77f0 100644 --- a/server/platform/shared/filestore/s3store_test.go +++ b/server/platform/shared/filestore/s3store_test.go @@ -4,12 +4,14 @@ package filestore import ( + "bytes" "context" "crypto/rand" "encoding/base64" "errors" "fmt" "io" + "io/fs" "net/http/httptest" "net/http/httputil" "net/url" @@ -20,6 +22,7 @@ import ( "github.com/mattermost/mattermost/server/public/model" s3 "github.com/minio/minio-go/v7" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -320,3 +323,59 @@ func (fc fauxCloser) Close() error { fc.s3WithCancel.cancel() return fc.closeErr } + +func TestListDirectory(t *testing.T) { + s3Host := os.Getenv("CI_MINIO_HOST") + if s3Host == "" { + s3Host = "localhost" + } + + s3Port := os.Getenv("CI_MINIO_PORT") + if s3Port == "" { + s3Port = "9000" + } + + s3Endpoint := fmt.Sprintf("%s:%s", s3Host, s3Port) + + cfg := FileBackendSettings{ + DriverName: driverS3, + AmazonS3AccessKeyId: "minioaccesskey", + AmazonS3SecretAccessKey: "miniosecretkey", + AmazonS3Bucket: "mattermost-test-1", + AmazonS3Region: "", + AmazonS3Endpoint: s3Endpoint, + AmazonS3PathPrefix: "", + AmazonS3SSL: false, + AmazonS3SSE: false, + AmazonS3RequestTimeoutMilliseconds: 5000, + } + + fileBackend, err := NewS3FileBackend(cfg) + require.NoError(t, err) + + found, err := fileBackend.client.BucketExists(context.Background(), cfg.AmazonS3Bucket) + require.NoError(t, err) + + if !found { + err = fileBackend.MakeBucket() + require.NoError(t, err) + } + + fileBackend.pathPrefix = "19700101/" + require.NoError(t, err) + b := []byte("test") + + path1 := "19700101/" + randomString() + ".txt" + _, err = fileBackend.WriteFile(bytes.NewReader(b), path1) + require.NoError(t, err, "Failed to write file1 to S3") + + _, err = fileBackend.ListDirectory("") + var pErr *fs.PathError + assert.True(t, errors.As(err, &pErr), "error is not of type fs.PathError") + + err = fileBackend.RemoveFile(path1) + require.NoError(t, err, "Failed to remove file1 from S3") + + err = fileBackend.RemoveDirectory("19700101") + require.NoError(t, err) +}