Repository: minio/minio (language: go)

Issue:
feat: Completing Multipart Uploads with Checksum That Contains Number of Parts Fails
## Expected Behavior
When completing multipart upload checksum in format `{checksum}-{numberOfParts}` should be supported.

## Current Behavior
When completing multipart upload checksum in format `{checksum}-{numberOfParts}`, `InvalidArgument: Invalid arguments provided for checksum-bug/minio-checksum-bug/2024-05-04T17:52:34.793Z.mp4: (invalid/unknown checksum sent: invalid checksum)` error is returned.

Various scenarios:
1. both s3 and minio allow completing without checksum 
2. both s3 and minio fail on invalid checkum
3. both s3 and minio allow completing with checksum that does not contain number of parts, eg. `ejt070PMd2pF/mFNTgP6LDtAo2px3L+i/l91VTgvRSc=`.
4. **only s3 allows completing when checkusm with number of parts as a suffix is provided, eg. `ejt070PMd2pF/mFNTgP6LDtAo2px3L+i/l91VTgvRSc=-2`.**

## Possible Solution
Fix somewhere in [erasure-multipart.go](https://github.com/minio/minio/blob/8ff70ea5a9210b545e3c95acb23b5a983b64da9c/cmd/erasure-multipart.go).

## Steps to Reproduce (for bugs)
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->
<!--- and make sure you have followed https://github.com/minio/minio/tree/release/docs/debugging to capture relevant logs -->

1. Create multipart upload with `ChecksumAlgorithm: "SHA256"` (likely all other algorithms are also impacted)
5. Upload parts with checksum
6. Complete multipart upload with checksum combined with number of parts

<details>
<summary>Sample code (JS)</summary>

```javascript

// tested with node@20.12.2, minio version RELEASE.2024-05-01T01-11-10Z
// save as .mjs file
// set flowing env variables before starting or here:
// process.env.S3_BUCKET_NAME = "";
// process.env.AWS_REGION = ""; // only for s3
// process.env.AWS_ACCESS_KEY_ID = "";
// process.env.AWS_SECRET_ACCESS_KEY = "";
// process.env.AWS_ENDPOINT_URL = ""; // only for minio

import {
  CompleteMultipartUploadCommand,
  CreateMultipartUploadCommand,
  ListPartsCommand,
  S3Client,
  UploadPartCommand,
} from "@aws-sdk/client-s3"; // tested with version 3.569.0
import crypto from "node:crypto";

function getChecksum(buffer) {
  const hash = crypto.createHash("sha256");
  hash.update(new Uint8Array(buffer));
  return hash.digest();
}

const video = await fetch(
  "https://github.com/minio/minio/assets/610941/2653d680-2c87-42ea-98d0-8feab199d3ef"
).then((response) => response.arrayBuffer());

const s3 = new S3Client({
  forcePathStyle: !!process.env.AWS_ENDPOINT_URL,
});

const MB_5 = 5 * 1024 * 1024;
const KEY = `minio-checksum-bug/${new Date().toISOString()}.mp4`;
const PARTS = Math.ceil(video.byteLength / MB_5);

console.log({ KEY, PARTS });

const multipartUpload = await s3.send(
  new CreateMultipartUploadCommand({
    Bucket: process.env.S3_BUCKET_NAME,
    Key: KEY,
    ChecksumAlgorithm: "SHA256",
  })
);

const uploadId = multipartUpload.UploadId;

const checksums = [];

for (let i = 0; i < PARTS; i++) {
  const start = i * MB_5;
  const end = Math.min((i + 1) * MB_5, video.byteLength);
  const body = video.slice(start, end);
  const checksum = getChecksum(body);
  const checksumBase64 = checksum.toString("base64");
  checksums.push(checksum);
  console.log({ i, checksumBase64 });

  await s3.send(
    new UploadPartCommand({
      Bucket: process.env.S3_BUCKET_NAME,
      Key: KEY,
      UploadId: uploadId,
      PartNumber: i + 1,
      Body: body,
      ChecksumSHA256: checksum.toString("base64"),
    })
  );
}

const partsList = await s3.send(
  new ListPartsCommand({
    Bucket: process.env.S3_BUCKET_NAME,
    Key: KEY,
    UploadId: uploadId,
  })
);

const checksumTotal = getChecksum(Buffer.concat(checksums));
const checksumTotalBase64 = checksumTotal.toString("base64");
console.log({ checksumTotalBase64 });

await s3.send(
  new CompleteMultipartUploadCommand({
    Bucket: process.env.S3_BUCKET_NAME,
    Key: KEY,
    UploadId: uploadId,
    MultipartUpload: {
      Parts: partsList.Parts.map((part) => ({
        PartNumber: part.PartNumber,
        ETag: part.ETag,
        ChecksumSHA256: part.ChecksumSHA256,
      })),
    },
    ChecksumSHA256: "a" + checksumTotalBase64, // GOOD does not work in both s3 and minio
    ChecksumSHA256: checksumTotalBase64, // GOOD works in both s3 and minio
    ChecksumSHA256: `${checksumTotalBase64}-${PARTS}`, // BAD works in s3, does not work in minio
  })
);
```
</details>

<details>
<summary>Test file storage</summary>

https://github.com/minio/minio/assets/610941/2653d680-2c87-42ea-98d0-8feab199d3ef

</details>

## Context
Not a big deal for us at this point, we can remove part numbers from the checksum, but reporting this as a it is compatibility issue.

## Regression
Probably not.

## Your Environment
Testing locally on Windows WSL, not really relevant for this issue.
```
minio version RELEASE.2024-05-01T01-11-10Z (commit-id=7926401cbd5cceaacd9509f2e50e1f7d636c2eb8)
Runtime: go1.21.9 linux/amd64
License: GNU AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>
Copyright: 2015-2024 MinIO, Inc.
```
```
Linux KUBA-PC 5.15.146.1-microsoft-standard-WSL2 #1 SMP Thu Jan 11 04:09:03 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
```




Discussion / hints:
Yeah. Seems like S3 added the part count later.

Fix patch:
diff --git a/cmd/erasure-multipart.go b/cmd/erasure-multipart.go
index 536b0f3db4bab..16183609eaf81 100644
--- a/cmd/erasure-multipart.go
+++ b/cmd/erasure-multipart.go
@@ -1231,7 +1231,7 @@ func (er erasureObjects) CompleteMultipartUpload(ctx context.Context, bucket str
 	}
 
 	if opts.WantChecksum != nil {
-		err := opts.WantChecksum.Matches(checksumCombined)
+		err := opts.WantChecksum.Matches(checksumCombined, len(parts))
 		if err != nil {
 			return oi, err
 		}
diff --git a/internal/hash/checksum.go b/internal/hash/checksum.go
index eaf81de6d7bf0..1466cec137551 100644
--- a/internal/hash/checksum.go
+++ b/internal/hash/checksum.go
@@ -27,6 +27,7 @@ import (
 	"hash"
 	"hash/crc32"
 	"net/http"
+	"strconv"
 	"strings"
 
 	"github.com/minio/minio/internal/hash/sha256"
@@ -71,9 +72,10 @@ const (
 
 // Checksum is a type and base 64 encoded value.
 type Checksum struct {
-	Type    ChecksumType
-	Encoded string
-	Raw     []byte
+	Type      ChecksumType
+	Encoded   string
+	Raw       []byte
+	WantParts int
 }
 
 // Is returns if c is all of t.
@@ -260,13 +262,14 @@ func ReadPartCheckSums(b []byte) (res []map[string]string) {
 		}
 		// Skip main checksum
 		b = b[length:]
-		if !typ.Is(ChecksumIncludesMultipart) {
-			continue
-		}
 		parts, n := binary.Uvarint(b)
 		if n <= 0 {
 			break
 		}
+		if !typ.Is(ChecksumIncludesMultipart) {
+			continue
+		}
+
 		if len(res) == 0 {
 			res = make([]map[string]string, parts)
 		}
@@ -292,11 +295,25 @@ func NewChecksumWithType(alg ChecksumType, value string) *Checksum {
 	if !alg.IsSet() {
 		return nil
 	}
+	wantParts := 0
+	if strings.ContainsRune(value, '-') {
+		valSplit := strings.Split(value, "-")
+		if len(valSplit) != 2 {
+			return nil
+		}
+		value = valSplit[0]
+		nParts, err := strconv.Atoi(valSplit[1])
+		if err != nil {
+			return nil
+		}
+		alg |= ChecksumMultipart
+		wantParts = nParts
+	}
 	bvalue, err := base64.StdEncoding.DecodeString(value)
 	if err != nil {
 		return nil
 	}
-	c := Checksum{Type: alg, Encoded: value, Raw: bvalue}
+	c := Checksum{Type: alg, Encoded: value, Raw: bvalue, WantParts: wantParts}
 	if !c.Valid() {
 		return nil
 	}
@@ -325,12 +342,15 @@ func (c *Checksum) AppendTo(b []byte, parts []byte) []byte {
 	b = append(b, crc...)
 	if c.Type.Is(ChecksumMultipart) {
 		var checksums int
+		if c.WantParts > 0 && !c.Type.Is(ChecksumIncludesMultipart) {
+			checksums = c.WantParts
+		}
 		// Ensure we don't divide by 0:
 		if c.Type.RawByteLen() == 0 || len(parts)%c.Type.RawByteLen() != 0 {
 			hashLogIf(context.Background(), fmt.Errorf("internal error: Unexpected checksum length: %d, each checksum %d", len(parts), c.Type.RawByteLen()))
 			checksums = 0
 			parts = nil
-		} else {
+		} else if len(parts) > 0 {
 			checksums = len(parts) / c.Type.RawByteLen()
 		}
 		if !c.Type.Is(ChecksumIncludesMultipart) {
@@ -358,7 +378,7 @@ func (c Checksum) Valid() bool {
 }
 
 // Matches returns whether given content matches c.
-func (c Checksum) Matches(content []byte) error {
+func (c Checksum) Matches(content []byte, parts int) error {
 	if len(c.Encoded) == 0 {
 		return nil
 	}
@@ -368,6 +388,13 @@ func (c Checksum) Matches(content []byte) error {
 		return err
 	}
 	sum := hasher.Sum(nil)
+	if c.WantParts > 0 && c.WantParts != parts {
+		return ChecksumMismatch{
+			Want: fmt.Sprintf("%s-%d", c.Encoded, c.WantParts),
+			Got:  fmt.Sprintf("%s-%d", base64.StdEncoding.EncodeToString(sum), parts),
+		}
+	}
+
 	if !bytes.Equal(sum, c.Raw) {
 		return ChecksumMismatch{
 			Want: c.Encoded,


Test patch: