From c36eaedb93bdb6f7f5df4d8b1bd25b1cba69977c Mon Sep 17 00:00:00 2001
From: Klaus Post <klauspost@gmail.com>
Date: Mon, 13 May 2024 09:30:24 -0700
Subject: [PATCH] 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 f30417d9a8a0aa477140409b66cba388e11fcf94.

* Don't override when suffix doesn't match. Instead rely on quorum for each.
---
 cmd/metacache-entries.go | 39 ++++++++++++++++++++++++++++++---------
 cmd/server_test.go       |  9 ++++++++-
 go.mod                   |  2 +-
 go.sum                   |  4 ++--
 4 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/cmd/metacache-entries.go b/cmd/metacache-entries.go
index 38ace73d2..94f274c9a 100644
--- a/cmd/metacache-entries.go
+++ b/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]
diff --git a/cmd/server_test.go b/cmd/server_test.go
index ef1c5d311..aa86368bb 100644
--- a/cmd/server_test.go
+++ b/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 {
diff --git a/go.mod b/go.mod
index f76de0e60..d321361d7 100644
--- a/go.mod
+++ b/go.mod
@@ -22,7 +22,7 @@ require (
 	github.com/dustin/go-humanize v1.0.1
 	github.com/eclipse/paho.mqtt.golang v1.4.3
 	github.com/elastic/go-elasticsearch/v7 v7.17.10
-	github.com/fatih/color v1.16.0
+	github.com/fatih/color v1.17.0
 	github.com/felixge/fgprof v0.9.4
 	github.com/fraugster/parquet-go v0.12.0
 	github.com/go-ldap/ldap/v3 v3.4.8
diff --git a/go.sum b/go.sum
index b32991a6f..8b339a9a2 100644
--- a/go.sum
+++ b/go.sum
@@ -147,8 +147,8 @@ github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1m
 github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
 github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
-github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM=
-github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE=
+github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4=
+github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI=
 github.com/fatih/structs v1.1.0 h1:Q7juDM0QtcnhCpeyLGQKyg4TOIghuNXrkL32pHAUMxo=
 github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga6PJ7M=
 github.com/felixge/fgprof v0.9.4 h1:ocDNwMFlnA0NU0zSB3I52xkO4sFXk80VK9lXjLClu88=