diff --git a/cmd/testdata/xl-meta-consist.zip b/cmd/testdata/xl-meta-consist.zip new file mode 100644 index 000000000..984ade8cc Binary files /dev/null and b/cmd/testdata/xl-meta-consist.zip differ diff --git a/cmd/xl-storage-format-v2-legacy.go b/cmd/xl-storage-format-v2-legacy.go index 56e3d909e..2aa99ebd5 100644 --- a/cmd/xl-storage-format-v2-legacy.go +++ b/cmd/xl-storage-format-v2-legacy.go @@ -83,7 +83,22 @@ func (j *xlMetaV2Version) unmarshalV(v uint8, bts []byte) (o []byte, err error) switch v { // We accept un-set as latest version. case 0, xlMetaVersion: - return j.UnmarshalMsg(bts) + o, err = j.UnmarshalMsg(bts) + + // Clean up PartEtags on v1 + if j.ObjectV2 != nil { + allEmpty := true + for _, tag := range j.ObjectV2.PartETags { + if len(tag) != 0 { + allEmpty = false + break + } + } + if allEmpty { + j.ObjectV2.PartETags = nil + } + } + return o, err } return bts, fmt.Errorf("unknown xlMetaVersion: %d", v) } diff --git a/cmd/xl-storage-format-v2.go b/cmd/xl-storage-format-v2.go index 0174322f1..6cbaa584b 100644 --- a/cmd/xl-storage-format-v2.go +++ b/cmd/xl-storage-format-v2.go @@ -264,9 +264,13 @@ func (x xlMetaV2VersionHeader) String() string { // matchesNotStrict returns whether x and o have both have non-zero version, // their versions match and their type match. +// If they have zero version, modtime must match. func (x xlMetaV2VersionHeader) matchesNotStrict(o xlMetaV2VersionHeader) bool { - return x.VersionID != [16]byte{} && - x.VersionID == o.VersionID && + if x.VersionID == [16]byte{} { + return x.VersionID == o.VersionID && + x.Type == o.Type && o.ModTime == x.ModTime + } + return x.VersionID == o.VersionID && x.Type == o.Type } @@ -508,7 +512,14 @@ func (j *xlMetaV2Object) Signature() [4]byte { c.ErasureIndex = 0 // Nil 0 size allownil, so we don't differentiate between nil and 0 len. - if len(c.PartETags) == 0 { + allEmpty := true + for _, tag := range c.PartETags { + if len(tag) != 0 { + allEmpty = false + break + } + } + if allEmpty { c.PartETags = nil } if len(c.PartActualSizes) == 0 { @@ -1676,6 +1687,9 @@ func (x xlMetaV2) ListVersions(volume, path string) ([]FileInfo, error) { // Quorum should be > 1 and <= len(versions). // If strict is set to false, entries that match type func mergeXLV2Versions(quorum int, strict bool, versions ...[]xlMetaV2ShallowVersion) (merged []xlMetaV2ShallowVersion) { + if quorum <= 0 { + quorum = 1 + } if len(versions) < quorum || len(versions) == 0 { return nil } @@ -1686,14 +1700,16 @@ func mergeXLV2Versions(quorum int, strict bool, versions ...[]xlMetaV2ShallowVer // No need for non-strict checks if quorum is 1. strict = true } + // Shallow copy input + versions = append(make([][]xlMetaV2ShallowVersion, 0, len(versions)), versions...) + // Our result merged = make([]xlMetaV2ShallowVersion, 0, len(versions[0])) tops := make([]xlMetaV2ShallowVersion, len(versions)) for { // Step 1 create slice with all top versions. tops = tops[:0] - var topSig [4]byte - var topID [16]byte + var topSig xlMetaV2VersionHeader consistent := true // Are all signatures consistent (shortcut) for _, vers := range versions { if len(vers) == 0 { @@ -1703,10 +1719,9 @@ func mergeXLV2Versions(quorum int, strict bool, versions ...[]xlMetaV2ShallowVer ver := vers[0] if len(tops) == 0 { consistent = true - topSig = ver.header.Signature - topID = ver.header.VersionID + topSig = ver.header } else { - consistent = consistent && topSig == ver.header.Signature && topID == ver.header.VersionID + consistent = consistent && ver.header == topSig } tops = append(tops, vers[0]) } @@ -1732,7 +1747,7 @@ func mergeXLV2Versions(quorum int, strict bool, versions ...[]xlMetaV2ShallowVer continue } if i == 0 || ver.header.sortsBefore(latest.header) { - if i == 0 { + if i == 0 || latestCount == 0 { latestCount = 1 } else if !strict && ver.header.matchesNotStrict(latest.header) { latestCount++ @@ -1744,12 +1759,45 @@ func mergeXLV2Versions(quorum int, strict bool, versions ...[]xlMetaV2ShallowVer } // Mismatch, but older. - if !strict && ver.header.matchesNotStrict(latest.header) { - // If non-nil version ID and it matches, assume match, but keep newest. - if ver.header.sortsBefore(latest.header) { - latest = ver - } + if latestCount > 0 && !strict && ver.header.matchesNotStrict(latest.header) { latestCount++ + continue + } + if latestCount > 0 && ver.header.VersionID == latest.header.VersionID { + // Version IDs match, but otherwise unable to resolve. + // We are either strict, or don't have enough information to match. + // Switch to a pure counting algo. + x := make(map[xlMetaV2VersionHeader]int, len(tops)) + for _, a := range tops { + if a.header.VersionID != ver.header.VersionID { + continue + } + if !strict { + a.header.Signature = [4]byte{} + } + x[a.header]++ + } + latestCount = 0 + for k, v := range x { + if v < latestCount { + continue + } + if v == latestCount && latest.header.sortsBefore(k) { + // Tiebreak, use sort. + continue + } + for _, a := range tops { + hdr := a.header + if !strict { + hdr.Signature = [4]byte{} + } + if hdr == k { + latest = a + } + } + latestCount = v + } + break } } if latestCount >= quorum { diff --git a/cmd/xl-storage-format-v2_test.go b/cmd/xl-storage-format-v2_test.go index feb510d4e..d8b51c2ed 100644 --- a/cmd/xl-storage-format-v2_test.go +++ b/cmd/xl-storage-format-v2_test.go @@ -19,11 +19,15 @@ package cmd import ( "bytes" + "encoding/json" + "fmt" + "io" "sort" "testing" "time" "github.com/google/uuid" + "github.com/klauspost/compress/zip" "github.com/klauspost/compress/zstd" "github.com/minio/minio/internal/bucket/lifecycle" xhttp "github.com/minio/minio/internal/http" @@ -476,3 +480,272 @@ func Test_xlMetaV2Shallow_Load(t *testing.T) { test(t, &xl) }) } + +func Test_mergeXLV2Versions(t *testing.T) { + dataZ, err := ioutil.ReadFile("testdata/xl-meta-consist.zip") + if err != nil { + t.Fatal(err) + } + var vers [][]xlMetaV2ShallowVersion + zr, err := zip.NewReader(bytes.NewReader(dataZ), int64(len(dataZ))) + if err != nil { + t.Fatal(err) + } + for _, file := range zr.File { + if file.UncompressedSize64 == 0 { + continue + } + in, err := file.Open() + if err != nil { + t.Fatal(err) + } + defer in.Close() + buf, err := io.ReadAll(in) + if err != nil { + t.Fatal(err) + } + var xl xlMetaV2 + err = xl.LoadOrConvert(buf) + if err != nil { + t.Fatal(err) + } + vers = append(vers, xl.versions) + } + for _, v2 := range vers { + for _, ver := range v2 { + b, _ := json.Marshal(ver.header) + t.Log(string(b)) + var x xlMetaV2Version + _, _ = x.unmarshalV(0, ver.meta) + b, _ = json.Marshal(x) + t.Log(string(b), x.getSignature()) + } + } + + for i := range vers { + t.Run(fmt.Sprintf("non-strict-q%d", i), func(t *testing.T) { + merged := mergeXLV2Versions(i, false, vers...) + if len(merged) == 0 { + t.Error("Did not get any results") + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + t.Run(fmt.Sprintf("strict-q%d", i), func(t *testing.T) { + merged := mergeXLV2Versions(i, true, vers...) + if len(merged) == 0 { + t.Error("Did not get any results") + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + t.Run(fmt.Sprintf("signature-q%d", i), func(t *testing.T) { + // Mutate signature, non strict + vMod := make([][]xlMetaV2ShallowVersion, 0, len(vers)) + for i, ver := range vers { + newVers := make([]xlMetaV2ShallowVersion, 0, len(ver)) + for _, v := range ver { + v.header.Signature = [4]byte{byte(i + 10), 0, 0, 0} + newVers = append(newVers, v) + } + vMod = append(vMod, newVers) + } + merged := mergeXLV2Versions(i, false, vMod...) + if len(merged) == 0 { + t.Error("Did not get any results") + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + t.Run(fmt.Sprintf("modtime-q%d", i), func(t *testing.T) { + // Mutate modtime, but rest is consistent. + vMod := make([][]xlMetaV2ShallowVersion, 0, len(vers)) + for i, ver := range vers { + newVers := make([]xlMetaV2ShallowVersion, 0, len(ver)) + for _, v := range ver { + v.header.ModTime += int64(i) + newVers = append(newVers, v) + } + vMod = append(vMod, newVers) + } + merged := mergeXLV2Versions(i, false, vMod...) + if len(merged) == 0 && i < 2 { + t.Error("Did not get any results") + return + } + if len(merged) > 0 && i >= 2 { + t.Error("Got unexpected results") + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + t.Run(fmt.Sprintf("flags-q%d", i), func(t *testing.T) { + // Mutate signature, non strict + vMod := make([][]xlMetaV2ShallowVersion, 0, len(vers)) + for i, ver := range vers { + newVers := make([]xlMetaV2ShallowVersion, 0, len(ver)) + for _, v := range ver { + v.header.Flags += xlFlags(i) + newVers = append(newVers, v) + } + vMod = append(vMod, newVers) + } + merged := mergeXLV2Versions(i, false, vMod...) + if len(merged) == 0 { + t.Error("Did not get any results") + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + t.Run(fmt.Sprintf("versionid-q%d", i), func(t *testing.T) { + // Mutate signature, non strict + vMod := make([][]xlMetaV2ShallowVersion, 0, len(vers)) + for i, ver := range vers { + newVers := make([]xlMetaV2ShallowVersion, 0, len(ver)) + for _, v := range ver { + v.header.VersionID[0] += byte(i) + newVers = append(newVers, v) + } + vMod = append(vMod, newVers) + } + merged := mergeXLV2Versions(i, false, vMod...) + if len(merged) == 0 && i < 2 { + t.Error("Did not get any results") + return + } + if len(merged) > 0 && i >= 2 { + t.Error("Got unexpected results") + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + t.Run(fmt.Sprintf("strict-signature-q%d", i), func(t *testing.T) { + // Mutate signature, non strict + vMod := make([][]xlMetaV2ShallowVersion, 0, len(vers)) + for i, ver := range vers { + newVers := make([]xlMetaV2ShallowVersion, 0, len(ver)) + for _, v := range ver { + v.header.Signature = [4]byte{byte(i + 10), 0, 0, 0} + newVers = append(newVers, v) + } + vMod = append(vMod, newVers) + } + merged := mergeXLV2Versions(i, true, vMod...) + if len(merged) == 0 && i < 2 { + t.Error("Did not get any results") + return + } + if len(merged) > 0 && i >= 2 { + t.Error("Got unexpected results") + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + t.Run(fmt.Sprintf("strict-modtime-q%d", i), func(t *testing.T) { + // Mutate signature, non strict + vMod := make([][]xlMetaV2ShallowVersion, 0, len(vers)) + for i, ver := range vers { + newVers := make([]xlMetaV2ShallowVersion, 0, len(ver)) + for _, v := range ver { + v.header.ModTime += int64(i + 10) + newVers = append(newVers, v) + } + vMod = append(vMod, newVers) + } + merged := mergeXLV2Versions(i, true, vMod...) + if len(merged) == 0 && i < 2 { + t.Error("Did not get any results") + return + } + if len(merged) > 0 && i >= 2 { + t.Error("Got unexpected results", len(merged), merged[0].header) + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + t.Run(fmt.Sprintf("strict-flags-q%d", i), func(t *testing.T) { + // Mutate signature, non strict + vMod := make([][]xlMetaV2ShallowVersion, 0, len(vers)) + for i, ver := range vers { + newVers := make([]xlMetaV2ShallowVersion, 0, len(ver)) + for _, v := range ver { + v.header.Flags += xlFlags(i + 10) + newVers = append(newVers, v) + } + vMod = append(vMod, newVers) + } + merged := mergeXLV2Versions(i, true, vMod...) + if len(merged) == 0 && i < 2 { + t.Error("Did not get any results") + return + } + if len(merged) > 0 && i >= 2 { + t.Error("Got unexpected results", len(merged)) + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + t.Run(fmt.Sprintf("strict-type-q%d", i), func(t *testing.T) { + // Mutate signature, non strict + vMod := make([][]xlMetaV2ShallowVersion, 0, len(vers)) + for i, ver := range vers { + newVers := make([]xlMetaV2ShallowVersion, 0, len(ver)) + for _, v := range ver { + v.header.Type += VersionType(i + 10) + newVers = append(newVers, v) + } + vMod = append(vMod, newVers) + } + merged := mergeXLV2Versions(i, true, vMod...) + if len(merged) == 0 && i < 2 { + t.Error("Did not get any results") + return + } + if len(merged) > 0 && i >= 2 { + t.Error("Got unexpected results", len(merged)) + return + } + for _, ver := range merged { + if ver.header.Type == invalidVersionType { + t.Errorf("Invalid result returned: %v", ver.header) + } + } + }) + } +}