Skip unsupported object names in list output

internal/fs/fs.go

@@ -1893,7 +1893,18 @@ func (fs *fileSystem) RmDir(
 
 		// Are there any entries?
 		if len(entries) != 0 {
-			err = fuse.ENOTEMPTY
+			isThereAnySupportedObject := func(parent inode.DirInode, name string) bool {
+				// dummy implementation.. to be replaced with proper implementation.
+				return false
+			}
+
+			for i := range len(entries) {
+				if isThereAnySupportedObject(childDir, entries[i].Name) {
+					err = fuse.ENOTEMPTY
+				} else {
+					logger.Tracef("******* Ignored prefix %q which only has unsupported objects  *******", childDir.Name().GcsObjectName()+"/"+entries[i].Name)
+				}
+			}
 			return
 		}
 

internal/fs/handle/dir_handle.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/fs/inode"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/locker"
+	"github.com/googlecloudplatform/gcsfuse/v2/internal/logger"
 	"github.com/jacobsa/fuse"
 	"github.com/jacobsa/fuse/fuseops"
 	"github.com/jacobsa/fuse/fuseutil"
@@ -299,9 +300,11 @@ func (dh *DirHandle) ReadDir(
 		return
 	}
 
+	logger.Debugf("dirEntries for %q: ", dh.in.Name())
 	// We copy out entries until we run out of entries or space.
 	for i := index; i < len(dh.entries); i++ {
 		n := fuseutil.WriteDirent(op.Dst[op.BytesRead:], dh.entries[i])
+		logger.Debugf("dirEntry[%d]= %q", i, dh.entries[i].Name)
 		if n == 0 {
 			break
 		}

internal/fs/implicit_dirs_test.go

@@ -19,8 +19,12 @@
 package fs_test
 
 import (
+	"fmt"
+	"io/fs"
 	"os"
 	"path"
+	"path/filepath"
+	"strings"
 	"syscall"
 	"time"
 
@@ -569,3 +573,105 @@ func (t *ImplicitDirsTest) AtimeCtimeAndMtime() {
 	ExpectThat(ctime, timeutil.TimeNear(mountTime, delta))
 	ExpectThat(mtime, timeutil.TimeNear(mountTime, delta))
 }
+
+func verifyInvalidPath(path string) {
+	_, err := os.Stat(path)
+
+	AssertNe(nil, err, "Failed to get error in stat of %q", path)
+}
+
+// Create objects with unsupported object names and
+// verify the behavior of mount using os.Stat and WalkDir.
+func (t *ImplicitDirsTest) UnsupportedDirNames() {
+	// Set up contents.
+	AssertEq(
+		nil,
+		t.createObjects(
+			map[string]string{
+				"foo//1":   "", // unsupported
+				"foo/2":    "", // supported
+				"foo/3//4": "", // unsupported
+				"foo//3/5": "", // unsupported
+				"foo/3/6":  "", // supported
+				"7":        "", // supported
+				"/8":       "", // unsupported
+				"/9/10":    "", // unsupported
+				"/":        "", // unsupported
+				"11/12":    "", // supported
+			}))
+
+	// Verify that the unsupported objects fail os.Stat.
+	verifyInvalidPath(path.Join(mntDir, "foo//1"))
+	verifyInvalidPath(path.Join(mntDir, "foo/3//4"))
+	verifyInvalidPath(path.Join(mntDir, "foo//3/5"))
+	verifyInvalidPath(path.Join(mntDir, "/8"))
+	verifyInvalidPath(path.Join(mntDir, "/9/10"))
+
+	// Verify that the supported objects appear in WalkDir.
+	expectedWalkedEntries := []struct {
+		path  string
+		name  string
+		isDir bool
+		found bool
+	}{{
+		path:  mntDir,
+		name:  mntDir[strings.LastIndex(mntDir, "/")+1:],
+		isDir: true,
+	}, {
+		path:  path.Join(mntDir, "foo"),
+		name:  "foo",
+		isDir: true,
+	}, {
+		path: path.Join(mntDir, "foo/2"),
+		name: "2",
+	}, {
+		path:  path.Join(mntDir, "foo/3"),
+		name:  "3",
+		isDir: true,
+	}, {
+		path: path.Join(mntDir, "foo/3/6"),
+		name: "6",
+	}, {
+		path: path.Join(mntDir, "7"),
+		name: "7",
+	}, {
+		path:  path.Join(mntDir, "11"),
+		name:  "11",
+		isDir: true,
+	}, {
+		path: path.Join(mntDir, "11/12"),
+		name: "12",
+	},
+	}
+
+	AssertEq(nil, filepath.WalkDir(mntDir, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		foundMatchingExpectedWalkingEntry := false
+		for i := range expectedWalkedEntries {
+			expectedWalkedEntry := &expectedWalkedEntries[i]
+			if expectedWalkedEntry.path == path && expectedWalkedEntry.name == d.Name() && d.IsDir() == expectedWalkedEntry.isDir {
+				if expectedWalkedEntry.found {
+					return fmt.Errorf("found duplicate walked entry: path=%s, name=%s, isDir=%v", path, d.Name(), d.IsDir())
+				}
+
+				foundMatchingExpectedWalkingEntry = true
+				expectedWalkedEntry.found = true
+			}
+		}
+
+		if !foundMatchingExpectedWalkingEntry {
+			return fmt.Errorf("got unexpected walk entry: path=%s, name=%s, isDir=%v", path, d.Name(), d.IsDir())
+		}
+
+		return nil
+	}))
+
+	for _, expectedWalkedEntry := range expectedWalkedEntries {
+		if !expectedWalkedEntry.found {
+			AddFailure("Missing walked entry: path=%s, name=%s, isDir=%v", expectedWalkedEntry.path, expectedWalkedEntry.name, expectedWalkedEntry.isDir)
+		}
+	}
+}

internal/fs/inode/dir.go

@@ -24,8 +24,10 @@ import (
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/cache/metadata"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/gcsx"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/locker"
+	"github.com/googlecloudplatform/gcsfuse/v2/internal/logger"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/storage/gcs"
 	"github.com/googlecloudplatform/gcsfuse/v2/internal/storage/storageutil"
+	"github.com/googlecloudplatform/gcsfuse/v2/internal/util"
 	"github.com/jacobsa/fuse/fuseops"
 	"github.com/jacobsa/fuse/fuseutil"
 	"github.com/jacobsa/timeutil"
@@ -652,7 +654,24 @@ func (d *dirInode) ReadDescendants(ctx context.Context, limit int) (map[Name]*Co
 			return descendants, nil
 		}
 	}
+}
+
+func logUnsupportedListings(removedListings *gcs.Listing) {
+	if removedListings != nil {
+		if len(removedListings.CollapsedRuns) > 0 {
+			logger.Warnf("Ignored following unsupported prefixes: %v", removedListings.CollapsedRuns)
+		}
+		if len(removedListings.Objects) > 0 {
+			objectNames := []string{}
+			for _, object := range removedListings.Objects {
+				if object != nil {
+					objectNames = append(objectNames, object.Name)
+				}
+			}
 
+			logger.Warnf("Ignored following unsupported objects: %v", objectNames)
+		}
+	}
 }
 
 // LOCKS_REQUIRED(d)
@@ -681,6 +700,12 @@ func (d *dirInode) readObjects(
 		return
 	}
 
+	// Remove unsupported prefixes/objects such as those
+	// containing '//' in them.
+	var removedListings *gcs.Listing
+	listing, removedListings = util.RemoveUnsupportedObjectsFromListing(listing)
+	logUnsupportedListings(removedListings)
+
 	cores = make(map[Name]*Core)
 	defer func() {
 		now := d.cacheClock.Now()

internal/fs/inode/dir_test.go

@@ -956,6 +956,62 @@ func (t *DirTest) ReadEntries_TypeCaching() {
 	AssertFalse(d.prevDirListingTimeStamp.IsZero())
 }
 
+func (t *DirTest) ReadEntries_IncludingUnsupportedObjects() {
+	var err error
+	var entry fuseutil.Dirent
+
+	// Enable implicit dirs.
+	t.resetInode(true, false, true)
+
+	// Set up contents.
+	objs := []string{
+		dirInodeName + "supported_dir_explicit/",
+		dirInodeName + "supported_dir_implicit1/supported_file",
+		dirInodeName + "supported_dir_implicit2//unsupported_file",
+		dirInodeName + "/unsupported_dir_explicit/",
+		dirInodeName + "/unsupported_dir_implicit1/supported_file",
+		dirInodeName + "top_level_supported_file",
+		dirInodeName + "/top_level_unsupported_file",
+	}
+
+	err = storageutil.CreateEmptyObjects(t.ctx, t.bucket, objs)
+	AssertEq(nil, err)
+
+	// Nil prevDirListingTimeStamp
+	d := t.in.(*dirInode)
+	AssertNe(nil, d)
+	AssertTrue(d.prevDirListingTimeStamp.IsZero())
+
+	// Read entries.
+	entries, err := t.readAllEntries()
+
+	AssertEq(nil, err)
+	AssertEq(4, len(entries))
+
+	entry = entries[0]
+	ExpectEq("supported_dir_explicit", entry.Name)
+	ExpectEq(fuseutil.DT_Directory, entry.Type)
+	ExpectEq(metadata.ExplicitDirType, t.getTypeFromCache("supported_dir_explicit"))
+
+	entry = entries[1]
+	ExpectEq("supported_dir_implicit1", entry.Name)
+	ExpectEq(fuseutil.DT_Directory, entry.Type)
+	ExpectEq(metadata.ImplicitDirType, t.getTypeFromCache("supported_dir_implicit1"))
+
+	entry = entries[2]
+	ExpectEq("supported_dir_implicit2", entry.Name)
+	ExpectEq(fuseutil.DT_Directory, entry.Type)
+	ExpectEq(metadata.ImplicitDirType, t.getTypeFromCache("supported_dir_implicit2"))
+
+	entry = entries[3]
+	ExpectEq("top_level_supported_file", entry.Name)
+	ExpectEq(fuseutil.DT_File, entry.Type)
+	ExpectEq(metadata.RegularFileType, t.getTypeFromCache("top_level_supported_file"))
+
+	// Make sure prevDirListingTimeStamp is not nil.
+	AssertFalse(d.prevDirListingTimeStamp.IsZero())
+}
+
 func (t *DirTest) CreateChildFile_DoesntExist() {
 	const name = "qux"
 	objName := path.Join(dirInodeName, name)

internal/gcsx/bucket_manager.go

@@ -234,7 +234,7 @@ func (bm *bucketManager) SetUpBucket(
 	}
 
 	// Periodically garbage collect temporary objects
-	go garbageCollect(bm.gcCtx, bm.config.TmpObjectPrefix, sb)
+	//go garbageCollect(bm.gcCtx, bm.config.TmpObjectPrefix, sb)
 
 	return
 }

internal/storage/bucket_handle_test.go

@@ -49,6 +49,16 @@ var ContentDisposition string = "ContentDisposition"
 // Hence, we are not writing tests for these flows.
 // https://github.com/GoogleCloudPlatform/gcsfuse/blob/master/vendor/github.com/fsouza/fake-gcs-server/fakestorage/object.go#L515
 
+func objectsToObjectNames(objects []*gcs.Object) (objectNames []string) {
+	objectNames = make([]string, len(objects))
+	for i, object := range objects {
+		if object != nil {
+			objectNames[i] = object.Name
+		}
+	}
+	return
+}
+
 type BucketHandleTest struct {
 	suite.Suite
 	bucketHandle  *bucketHandle
@@ -447,13 +457,9 @@ func (testSuite *BucketHandleTest) TestListObjectMethodWithPrefixObjectExist() {
 		})
 
 	assert.Nil(testSuite.T(), err)
-	assert.Equal(testSuite.T(), 4, len(obj.Objects))
-	assert.Equal(testSuite.T(), 1, len(obj.CollapsedRuns))
-	assert.Equal(testSuite.T(), TestObjectRootFolderName, obj.Objects[0].Name)
-	assert.Equal(testSuite.T(), TestObjectSubRootFolderName, obj.Objects[1].Name)
-	assert.Equal(testSuite.T(), TestObjectName, obj.Objects[2].Name)
+	assert.Equal(testSuite.T(), []string{TestObjectRootFolderName, TestObjectSubRootFolderName, TestObjectName, TestGzipObjectName}, objectsToObjectNames(obj.Objects))
 	assert.Equal(testSuite.T(), TestObjectGeneration, obj.Objects[0].Generation)
-	assert.Equal(testSuite.T(), TestObjectSubRootFolderName, obj.CollapsedRuns[0])
+	assert.Equal(testSuite.T(), []string{TestObjectSubRootFolderName}, obj.CollapsedRuns)
 }
 
 func (testSuite *BucketHandleTest) TestListObjectMethodWithPrefixObjectDoesNotExist() {
@@ -484,12 +490,8 @@ func (testSuite *BucketHandleTest) TestListObjectMethodWithIncludeTrailingDelimi
 		})
 
 	assert.Nil(testSuite.T(), err)
-	assert.Equal(testSuite.T(), 3, len(obj.Objects))
-	assert.Equal(testSuite.T(), 1, len(obj.CollapsedRuns))
-	assert.Equal(testSuite.T(), TestObjectRootFolderName, obj.Objects[0].Name)
-	assert.Equal(testSuite.T(), TestObjectName, obj.Objects[1].Name)
-	assert.Equal(testSuite.T(), TestGzipObjectName, obj.Objects[2].Name)
-	assert.Equal(testSuite.T(), TestObjectSubRootFolderName, obj.CollapsedRuns[0])
+	assert.Equal(testSuite.T(), []string{TestObjectRootFolderName, TestObjectName, TestGzipObjectName}, objectsToObjectNames(obj.Objects))
+	assert.Equal(testSuite.T(), []string{TestObjectSubRootFolderName}, obj.CollapsedRuns)
 }
 
 // If Delimiter is empty, all the objects will appear with same prefix.
@@ -505,12 +507,7 @@ func (testSuite *BucketHandleTest) TestListObjectMethodWithEmptyDelimiter() {
 		})
 
 	assert.Nil(testSuite.T(), err)
-	assert.Equal(testSuite.T(), 5, len(obj.Objects))
-	assert.Equal(testSuite.T(), TestObjectRootFolderName, obj.Objects[0].Name)
-	assert.Equal(testSuite.T(), TestObjectSubRootFolderName, obj.Objects[1].Name)
-	assert.Equal(testSuite.T(), TestSubObjectName, obj.Objects[2].Name)
-	assert.Equal(testSuite.T(), TestObjectName, obj.Objects[3].Name)
-	assert.Equal(testSuite.T(), TestGzipObjectName, obj.Objects[4].Name)
+	assert.Equal(testSuite.T(), []string{TestObjectRootFolderName, TestObjectSubRootFolderName, TestSubObjectName, TestObjectName, TestGzipObjectName}, objectsToObjectNames(obj.Objects))
 	assert.Equal(testSuite.T(), TestObjectGeneration, obj.Objects[0].Generation)
 	assert.Nil(testSuite.T(), obj.CollapsedRuns)
 }
@@ -539,12 +536,7 @@ func (testSuite *BucketHandleTest) TestListObjectMethodForMaxResult() {
 
 	// Validate that 5 objects are listed when MaxResults is passed 5.
 	assert.Nil(testSuite.T(), err)
-	assert.Equal(testSuite.T(), 5, len(fiveObj.Objects))
-	assert.Equal(testSuite.T(), TestObjectRootFolderName, fiveObj.Objects[0].Name)
-	assert.Equal(testSuite.T(), TestObjectSubRootFolderName, fiveObj.Objects[1].Name)
-	assert.Equal(testSuite.T(), TestSubObjectName, fiveObj.Objects[2].Name)
-	assert.Equal(testSuite.T(), TestObjectName, fiveObj.Objects[3].Name)
-	assert.Equal(testSuite.T(), TestGzipObjectName, fiveObj.Objects[4].Name)
+	assert.Equal(testSuite.T(), []string{TestObjectRootFolderName, TestObjectSubRootFolderName, TestSubObjectName, TestObjectName, TestGzipObjectName}, objectsToObjectNames(fiveObj.Objects))
 	assert.Nil(testSuite.T(), fiveObj.CollapsedRuns)
 
 	// Note: The behavior is different in real GCS storage JSON API. In real API,
@@ -554,10 +546,7 @@ func (testSuite *BucketHandleTest) TestListObjectMethodForMaxResult() {
 	// This is because fake storage doesn'testSuite support pagination and hence maxResults
 	// has no affect.
 	assert.Nil(testSuite.T(), err2)
-	assert.Equal(testSuite.T(), 3, len(threeObj.Objects))
-	assert.Equal(testSuite.T(), TestObjectRootFolderName, threeObj.Objects[0].Name)
-	assert.Equal(testSuite.T(), TestObjectName, threeObj.Objects[1].Name)
-	assert.Equal(testSuite.T(), TestGzipObjectName, threeObj.Objects[2].Name)
+	assert.Equal(testSuite.T(), []string{TestObjectRootFolderName, TestObjectName, TestGzipObjectName}, objectsToObjectNames(threeObj.Objects))
 	assert.Equal(testSuite.T(), 1, len(threeObj.CollapsedRuns))
 }
 
@@ -586,12 +575,7 @@ func (testSuite *BucketHandleTest) TestListObjectMethodWithMissingMaxResult() {
 
 	// Validate that all objects (5) are listed when MaxResults is not passed.
 	assert.Nil(testSuite.T(), err2)
-	assert.Equal(testSuite.T(), 5, len(fiveObjWithoutMaxResults.Objects))
-	assert.Equal(testSuite.T(), TestObjectRootFolderName, fiveObjWithoutMaxResults.Objects[0].Name)
-	assert.Equal(testSuite.T(), TestObjectSubRootFolderName, fiveObjWithoutMaxResults.Objects[1].Name)
-	assert.Equal(testSuite.T(), TestSubObjectName, fiveObjWithoutMaxResults.Objects[2].Name)
-	assert.Equal(testSuite.T(), TestObjectName, fiveObjWithoutMaxResults.Objects[3].Name)
-	assert.Equal(testSuite.T(), TestGzipObjectName, fiveObjWithoutMaxResults.Objects[4].Name)
+	assert.Equal(testSuite.T(), []string{TestObjectRootFolderName, TestObjectSubRootFolderName, TestSubObjectName, TestObjectName, TestGzipObjectName}, objectsToObjectNames(fiveObjWithoutMaxResults.Objects))
 	assert.Nil(testSuite.T(), fiveObjWithoutMaxResults.CollapsedRuns)
 }
 
@@ -622,12 +606,7 @@ func (testSuite *BucketHandleTest) TestListObjectMethodWithZeroMaxResult() {
 	// Validate that all objects (5) are listed when MaxResults is 0. This has
 	// same behavior as not passing MaxResults in request.
 	assert.Nil(testSuite.T(), err2)
-	assert.Equal(testSuite.T(), 5, len(fiveObjWithZeroMaxResults.Objects))
-	assert.Equal(testSuite.T(), TestObjectRootFolderName, fiveObjWithZeroMaxResults.Objects[0].Name)
-	assert.Equal(testSuite.T(), TestObjectSubRootFolderName, fiveObjWithZeroMaxResults.Objects[1].Name)
-	assert.Equal(testSuite.T(), TestSubObjectName, fiveObjWithZeroMaxResults.Objects[2].Name)
-	assert.Equal(testSuite.T(), TestObjectName, fiveObjWithZeroMaxResults.Objects[3].Name)
-	assert.Equal(testSuite.T(), TestGzipObjectName, fiveObjWithZeroMaxResults.Objects[4].Name)
+	assert.Equal(testSuite.T(), []string{TestObjectRootFolderName, TestObjectSubRootFolderName, TestSubObjectName, TestObjectName, TestGzipObjectName}, objectsToObjectNames(fiveObjWithZeroMaxResults.Objects))
 	assert.Nil(testSuite.T(), fiveObjWithZeroMaxResults.CollapsedRuns)
 }