Re-add "Fix incorrect merging of slash-suffixed objects (#19729)

Adds regression test for #19699

Failures are a bit luck based, since it requires objects to be placed on different sets.

However this generates a failure prior to #19699

* Revert "Revert "Fix incorrect merging of slash-suffixed objects (#19699)""

This reverts commit f30417d9a8.

* Don't override when suffix doesn't match. Instead rely on quorum for each.

cmd/metacache-entries.go

@@ -736,16 +736,37 @@ func mergeEntryChannels(ctx context.Context, in []chan metaCacheEntry, out chan<
 				bestIdx = otherIdx
 				continue
 			}
-			// We should make sure to avoid objects and directories
-			// of this fashion such as
-			//  - foo-1
-			//  - foo-1/
-			// we should avoid this situation by making sure that
-			// we compare the `foo-1/` after path.Clean() to
-			// de-dup the entries.
 			if path.Clean(best.name) == path.Clean(other.name) {
-				toMerge = append(toMerge, otherIdx)
-				continue
+				// We may be in a situation where we have a directory and an object with the same name.
+				// In that case we will drop the directory entry.
+				// This should however not be confused with an object with a trailing slash.
+				dirMatches := best.isDir() == other.isDir()
+				suffixMatches := strings.HasSuffix(best.name, slashSeparator) == strings.HasSuffix(other.name, slashSeparator)
+
+				// Simple case. Both are same type with same suffix.
+				if dirMatches && suffixMatches {
+					toMerge = append(toMerge, otherIdx)
+					continue
+				}
+
+				if !dirMatches {
+					// We have an object `name` or 'name/' and a directory `name/`.
+					if other.isDir() {
+						console.Debugln("mergeEntryChannels: discarding directory", other.name, "for object", best.name)
+						// Discard the directory.
+						if err := selectFrom(otherIdx); err != nil {
+							return err
+						}
+						continue
+					}
+					// Replace directory with object.
+					console.Debugln("mergeEntryChannels: discarding directory", best.name, "for object", other.name)
+					toMerge = toMerge[:0]
+					best = other
+					bestIdx = otherIdx
+					continue
+				}
+				// Leave it to be resolved. Names are different.
 			}
 			if best.name > other.name {
 				toMerge = toMerge[:0]

cmd/server_test.go

@@ -1586,7 +1586,7 @@ func (s *TestSuiteCommon) TestListObjectsHandler(c *check) {
 	c.Assert(err, nil)
 	c.Assert(response.StatusCode, http.StatusOK)
 
-	for _, objectName := range []string{"foo bar 1", "foo bar 2"} {
+	for _, objectName := range []string{"foo bar 1", "foo bar 2", "obj2", "obj2/"} {
 		buffer := bytes.NewReader([]byte("Hello World"))
 		request, err = newTestSignedRequest(http.MethodPut, getPutObjectURL(s.endPoint, bucketName, objectName),
 			int64(buffer.Len()), buffer, s.accessKey, s.secretKey, s.signer)
@@ -1619,6 +1619,13 @@ func (s *TestSuiteCommon) TestListObjectsHandler(c *check) {
 			},
 		},
 		{getListObjectsV2URL(s.endPoint, bucketName, "", "1000", "", "url"), []string{"<Key>foo+bar+1</Key>", "<Key>foo+bar+2</Key>"}},
+		{
+			getListObjectsV2URL(s.endPoint, bucketName, "", "1000", "", ""),
+			[]string{
+				"<Key>obj2</Key>",
+				"<Key>obj2/</Key>",
+			},
+		},
 	}
 
 	for _, testCase := range testCases {