fix list objects for directory type objects

backend/posix/posix.go

@@ -51,12 +51,7 @@ const (
 	metaTmpMultipartDir = metaTmpDir + "/multipart"
 	onameAttr           = "user.objname"
 	tagHdr              = "X-Amz-Tagging"
-	dirObjKey           = "user.dirisobject"
-)
-
-var (
-	newObjUID = 0
-	newObjGID = 0
+	emptyMD5            = "d41d8cd98f00b204e9800998ecf8427e"
 )
 
 func New(rootdir string) (*Posix, error) {
@@ -309,15 +304,6 @@ func (p *Posix) CompleteMultipartUpload(bucket, object, uploadID string, parts [
 		return nil, fmt.Errorf("set etag attr: %w", err)
 	}
 
-	if newObjUID != 0 || newObjGID != 0 {
-		err = os.Chown(objname, newObjUID, newObjGID)
-		if err != nil {
-			// cleanup object if returning error
-			os.Remove(objname)
-			return nil, fmt.Errorf("set object uid/gid: %w", err)
-		}
-	}
-
 	// cleanup tmp dirs
 	os.RemoveAll(upiddir)
 	// use Remove for objdir in case there are still other uploads
@@ -438,12 +424,6 @@ func mkdirAll(path string, perm os.FileMode, bucket, object string) error {
 		}
 		return s3err.GetAPIError(s3err.ErrObjectParentIsFile)
 	}
-	if newObjUID != 0 || newObjGID != 0 {
-		err = os.Chown(path, newObjUID, newObjGID)
-		if err != nil {
-			return fmt.Errorf("set parent ownership: %w", err)
-		}
-	}
 	return nil
 }
 
@@ -737,13 +717,10 @@ func (p *Posix) PutObject(po *s3.PutObjectInput) (string, error) {
 			xattr.Set(name, "user."+k, []byte(v))
 		}
 
-		// set our attribute that this dir was specifically put
-		xattr.Set(name, dirObjKey, nil)
+		// set etag attribute to signify this dir was specifically put
+		xattr.Set(name, "user.etag", []byte(emptyMD5))
 
-		// TODO: what etag should be returned here
-		// and we should set etag xattr to identify dir was
-		// specifically uploaded
-		return "", nil
+		return emptyMD5, nil
 	}
 
 	// object is file
@@ -781,13 +758,6 @@ func (p *Posix) PutObject(po *s3.PutObjectInput) (string, error) {
 	etag := hex.EncodeToString(dataSum[:])
 	xattr.Set(name, "user.etag", []byte(etag))
 
-	if newObjUID != 0 || newObjGID != 0 {
-		err = os.Chown(name, newObjUID, newObjGID)
-		if err != nil {
-			return "", fmt.Errorf("set object uid/gid: %v", err)
-		}
-	}
-
 	return etag, nil
 }
 
@@ -826,7 +796,7 @@ func (p *Posix) removeParents(bucket, object string) error {
 			break
 		}
 
-		_, err := xattr.Get(parent, dirObjKey)
+		_, err := xattr.Get(parent, "user.etag")
 		if err == nil {
 			break
 		}
@@ -1015,7 +985,20 @@ func (p *Posix) ListObjects(bucket, prefix, marker, delim string, maxkeys int) (
 	}
 
 	fileSystem := os.DirFS(bucket)
-	results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys)
+	results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys,
+		func(path string) (bool, error) {
+			_, err := xattr.Get(filepath.Join(bucket, path), "user.etag")
+			if isNoAttr(err) {
+				return false, nil
+			}
+			if err != nil {
+				return false, err
+			}
+			return true, nil
+		}, func(path string) (string, error) {
+			etag, err := xattr.Get(filepath.Join(bucket, path), "user.etag")
+			return string(etag), err
+		}, []string{metaTmpDir})
 	if err != nil {
 		return nil, fmt.Errorf("walk %v: %w", bucket, err)
 	}
@@ -1043,7 +1026,20 @@ func (p *Posix) ListObjectsV2(bucket, prefix, marker, delim string, maxkeys int)
 	}
 
 	fileSystem := os.DirFS(bucket)
-	results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys)
+	results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys,
+		func(path string) (bool, error) {
+			_, err := xattr.Get(filepath.Join(bucket, path), "user.etag")
+			if isNoAttr(err) {
+				return false, nil
+			}
+			if err != nil {
+				return false, err
+			}
+			return true, nil
+		}, func(path string) (string, error) {
+			etag, err := xattr.Get(filepath.Join(bucket, path), "user.etag")
+			return string(etag), err
+		}, []string{metaTmpDir})
 	if err != nil {
 		return nil, fmt.Errorf("walk %v: %w", bucket, err)
 	}

backend/walk.go

@@ -31,9 +31,12 @@ type WalkResults struct {
 	NextMarker     string
 }
 
+type DirObjCheck func(path string) (bool, error)
+type GetETag func(path string) (string, error)
+
 // Walk walks the supplied fs.FS and returns results compatible with list
 // objects responses
-func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResults, error) {
+func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int, dirchk DirObjCheck, getetag GetETag, skipdirs []string) (WalkResults, error) {
 	cpmap := make(map[string]struct{})
 	var objects []types.Object
 
@@ -63,6 +66,10 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu
 				return nil
 			}
 
+			if contains(d.Name(), skipdirs) {
+				return fs.SkipDir
+			}
+
 			// If prefix is defined and the directory does not match prefix,
 			// do not descend into the directory because nothing will
 			// match this prefix. Make sure to append the / at the end of
@@ -76,8 +83,35 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu
 				return fs.SkipDir
 			}
 
-			// TODO: special case handling if directory is empty
-			// and was "PUT" explicitly
+			// TODO: can we do better here rather than a second readdir
+			// per directory?
+			ents, err := fs.ReadDir(fileSystem, path)
+			if err != nil {
+				return fmt.Errorf("readdir %q: %w", path, err)
+			}
+			if len(ents) == 0 {
+				dirobj, err := dirchk(path)
+				if err != nil {
+					return fmt.Errorf("directory object check %q: %w", path, err)
+				}
+				if dirobj {
+					fi, err := d.Info()
+					if err != nil {
+						return fmt.Errorf("dir info %q: %w", path, err)
+					}
+					etag, err := getetag(path)
+					if err != nil {
+						return fmt.Errorf("get etag %q: %w", path, err)
+					}
+					path := path + "/"
+					objects = append(objects, types.Object{
+						ETag:         &etag,
+						Key:          &path,
+						LastModified: GetTimePtr(fi.ModTime()),
+					})
+				}
+			}
+
 			return nil
 		}
 
@@ -100,16 +134,22 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu
 			if err != nil {
 				return fmt.Errorf("get info for %v: %w", path, err)
 			}
+			etag, err := getetag(path)
+			if err != nil {
+				return fmt.Errorf("get etag %q: %w", path, err)
+			}
 
 			objects = append(objects, types.Object{
-				ETag:         new(string),
+				ETag:         &etag,
 				Key:          &path,
 				LastModified: GetTimePtr(fi.ModTime()),
 				Size:         fi.Size(),
 			})
+
 			if max > 0 && (len(objects)+len(cpmap)) == max {
 				pastMax = true
 			}
+
 			return nil
 		}
 
@@ -133,7 +173,7 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu
 		// these are all rolled up into the common prefix.
 		// Note: The delimeter can be anything, so we have to operate on
 		// the full path without any assumptions on posix directory heirarchy
-		// here.  Usually the delimeter with be "/", but thats not required.
+		// here.  Usually the delimeter will be "/", but thats not required.
 		suffix := strings.TrimPrefix(path, prefix)
 		before, _, found := strings.Cut(suffix, delimiter)
 		if !found {
@@ -141,8 +181,12 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu
 			if err != nil {
 				return fmt.Errorf("get info for %v: %w", path, err)
 			}
+			etag, err := getetag(path)
+			if err != nil {
+				return fmt.Errorf("get etag %q: %w", path, err)
+			}
 			objects = append(objects, types.Object{
-				ETag:         new(string),
+				ETag:         &etag,
 				Key:          &path,
 				LastModified: GetTimePtr(fi.ModTime()),
 				Size:         fi.Size(),
@@ -187,3 +231,12 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int) (WalkResu
 		NextMarker:     newMarker,
 	}, nil
 }
+
+func contains(a string, strs []string) bool {
+	for _, s := range strs {
+		if s == a {
+			return true
+		}
+	}
+	return false
+}

backend/walk_test.go

@@ -26,31 +26,50 @@ import (
 type walkTest struct {
 	fsys     fs.FS
 	expected backend.WalkResults
+	dc       backend.DirObjCheck
 }
 
+func gettag(string) (string, error) { return "myetag", nil }
+
 func TestWalk(t *testing.T) {
-	tests := []walkTest{{
-		// test case from
-		// https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-prefixes.html
-		fsys: fstest.MapFS{
-			"sample.jpg":                       {},
-			"photos/2006/January/sample.jpg":   {},
-			"photos/2006/February/sample2.jpg": {},
-			"photos/2006/February/sample3.jpg": {},
-			"photos/2006/February/sample4.jpg": {},
+	tests := []walkTest{
+		{
+			// test case from
+			// https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-prefixes.html
+			fsys: fstest.MapFS{
+				"sample.jpg":                       {},
+				"photos/2006/January/sample.jpg":   {},
+				"photos/2006/February/sample2.jpg": {},
+				"photos/2006/February/sample3.jpg": {},
+				"photos/2006/February/sample4.jpg": {},
+			},
+			expected: backend.WalkResults{
+				CommonPrefixes: []types.CommonPrefix{{
+					Prefix: backend.GetStringPtr("photos/"),
+				}},
+				Objects: []types.Object{{
+					Key: backend.GetStringPtr("sample.jpg"),
+				}},
+			},
+			dc: func(string) (bool, error) { return false, nil },
 		},
-		expected: backend.WalkResults{
-			CommonPrefixes: []types.CommonPrefix{{
-				Prefix: backend.GetStringPtr("photos/"),
-			}},
-			Objects: []types.Object{{
-				Key: backend.GetStringPtr("sample.jpg"),
-			}},
+		{
+			// test case single dir/single file
+			fsys: fstest.MapFS{
+				"test/file": {},
+			},
+			expected: backend.WalkResults{
+				CommonPrefixes: []types.CommonPrefix{{
+					Prefix: backend.GetStringPtr("test/"),
+				}},
+				Objects: []types.Object{},
+			},
+			dc: func(string) (bool, error) { return true, nil },
 		},
-	}}
+	}
 
 	for _, tt := range tests {
-		res, err := backend.Walk(tt.fsys, "", "/", "", 1000)
+		res, err := backend.Walk(tt.fsys, "", "/", "", 1000, tt.dc, gettag, []string{})
 		if err != nil {
 			t.Fatalf("walk: %v", err)
 		}
@@ -67,13 +86,16 @@ func compareResults(got, wanted backend.WalkResults, t *testing.T) {
 	}
 
 	if !compareObjects(got.Objects, wanted.Objects) {
-		t.Errorf("unexpected common prefix, got %v wanted %v",
+		t.Errorf("unexpected object, got %v wanted %v",
 			printObjects(got.Objects),
 			printObjects(wanted.Objects))
 	}
 }
 
 func compareCommonPrefix(a, b []types.CommonPrefix) bool {
+	if len(a) == 0 && len(b) == 0 {
+		return true
+	}
 	if len(a) != len(b) {
 		return false
 	}
@@ -108,6 +130,9 @@ func printCommonPrefixes(list []types.CommonPrefix) string {
 }
 
 func compareObjects(a, b []types.Object) bool {
+	if len(a) == 0 && len(b) == 0 {
+		return true
+	}
 	if len(a) != len(b) {
 		return false
 	}