diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index ee0f8209a..8b133e427 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -423,7 +423,7 @@ func isReqAuthenticated(ctx context.Context, r *http.Request, region string, sty if err != nil { return toAPIErrorCode(ctx, err) } - r.Body = ioutil.NopCloser(reader) + r.Body = reader return ErrNone } diff --git a/cmd/object-api-putobject_test.go b/cmd/object-api-putobject_test.go index e09b8bc05..9886d31f0 100644 --- a/cmd/object-api-putobject_test.go +++ b/cmd/object-api-putobject_test.go @@ -172,22 +172,27 @@ func testObjectAPIPutObject(obj ObjectLayer, instanceType string, t TestErrHandl } for i, testCase := range testCases { - objInfo, actualErr := obj.PutObject(context.Background(), testCase.bucketName, testCase.objName, mustGetPutObjReader(t, bytes.NewReader(testCase.inputData), testCase.intputDataSize, testCase.inputMeta["etag"], testCase.inputSHA256), ObjectOptions{UserDefined: testCase.inputMeta}) + in := mustGetPutObjReader(t, bytes.NewReader(testCase.inputData), testCase.intputDataSize, testCase.inputMeta["etag"], testCase.inputSHA256) + objInfo, actualErr := obj.PutObject(context.Background(), testCase.bucketName, testCase.objName, in, ObjectOptions{UserDefined: testCase.inputMeta}) if actualErr != nil && testCase.expectedError == nil { t.Errorf("Test %d: %s: Expected to pass, but failed with: error %s.", i+1, instanceType, actualErr.Error()) + continue } if actualErr == nil && testCase.expectedError != nil { t.Errorf("Test %d: %s: Expected to fail with error \"%s\", but passed instead.", i+1, instanceType, testCase.expectedError.Error()) + continue } // Failed as expected, but does it fail for the expected reason. if actualErr != nil && actualErr != testCase.expectedError { t.Errorf("Test %d: %s: Expected to fail with error \"%v\", but instead failed with error \"%v\" instead.", i+1, instanceType, testCase.expectedError, actualErr) + continue } // Test passes as expected, but the output values are verified for correctness here. if actualErr == nil { // Asserting whether the md5 output is correct. if expectedMD5, ok := testCase.inputMeta["etag"]; ok && expectedMD5 != objInfo.ETag { t.Errorf("Test %d: %s: Calculated Md5 different from the actual one %s.", i+1, instanceType, objInfo.ETag) + continue } } } diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 38e53c7e7..936aab7d3 100644 --- a/cmd/test-utils_test.go +++ b/cmd/test-utils_test.go @@ -222,6 +222,8 @@ func initFSObjects(disk string, t *testing.T) (obj ObjectLayer) { // This makes it easy to run the TestServer from any of the tests. // Using this interface, functionalities to be used in tests can be made generalized, and can be integrated in benchmarks/unit tests/go check suite tests. type TestErrHandler interface { + Log(args ...interface{}) + Logf(format string, args ...interface{}) Error(args ...interface{}) Errorf(format string, args ...interface{}) Failed() bool diff --git a/pkg/hash/errors.go b/pkg/hash/errors.go index ddb2d740e..1b5622c6d 100644 --- a/pkg/hash/errors.go +++ b/pkg/hash/errors.go @@ -16,6 +16,8 @@ package hash +import "fmt" + // SHA256Mismatch - when content sha256 does not match with what was sent from client. type SHA256Mismatch struct { ExpectedSHA256 string @@ -23,7 +25,7 @@ type SHA256Mismatch struct { } func (e SHA256Mismatch) Error() string { - return "Bad sha256: Expected " + e.ExpectedSHA256 + " is not valid with what we calculated " + e.CalculatedSHA256 + return "Bad sha256: Expected " + e.ExpectedSHA256 + " does not match calculated " + e.CalculatedSHA256 } // BadDigest - Content-MD5 you specified did not match what we received. @@ -33,5 +35,14 @@ type BadDigest struct { } func (e BadDigest) Error() string { - return "Bad digest: Expected " + e.ExpectedMD5 + " is not valid with what we calculated " + e.CalculatedMD5 + return "Bad digest: Expected " + e.ExpectedMD5 + " does not match calculated " + e.CalculatedMD5 +} + +type ErrSizeMismatch struct { + Want int64 + Got int64 +} + +func (e ErrSizeMismatch) Error() string { + return fmt.Sprintf("Size mismatch: got %d, want %d", e.Got, e.Want) } diff --git a/pkg/hash/reader.go b/pkg/hash/reader.go index 44e7d1ba1..8f3e5d7b1 100644 --- a/pkg/hash/reader.go +++ b/pkg/hash/reader.go @@ -28,14 +28,13 @@ import ( sha256 "github.com/minio/sha256-simd" ) -var errNestedReader = errors.New("Nesting of Reader detected, not allowed") - // Reader writes what it reads from an io.Reader to an MD5 and SHA256 hash.Hash. // Reader verifies that the content of the io.Reader matches the expected checksums. type Reader struct { src io.Reader size int64 actualSize int64 + bytesRead int64 md5sum, sha256sum []byte // Byte values of md5sum, sha256sum of client sent values. md5Hash, sha256Hash hash.Hash @@ -44,44 +43,14 @@ type Reader struct { // NewReader returns a new hash Reader which computes the MD5 sum and // SHA256 sum (if set) of the provided io.Reader at EOF. func NewReader(src io.Reader, size int64, md5Hex, sha256Hex string, actualSize int64, strictCompat bool) (*Reader, error) { - if _, ok := src.(*Reader); ok { - return nil, errNestedReader - } - - sha256sum, err := hex.DecodeString(sha256Hex) - if err != nil { - return nil, SHA256Mismatch{} - } - - md5sum, err := hex.DecodeString(md5Hex) - if err != nil { - return nil, BadDigest{} + if r, ok := src.(*Reader); ok { + // Merge expectations and return parent. + return r.merge(size, md5Hex, sha256Hex, actualSize, strictCompat) } - var sha256Hash hash.Hash - if len(sha256sum) != 0 { - sha256Hash = sha256.New() - } - var md5Hash hash.Hash - if strictCompat { - // Strict compatibility is set then we should - // calculate md5sum always. - md5Hash = md5.New() - } else if len(md5sum) != 0 { - md5Hash = md5.New() - } - if size >= 0 { - src = io.LimitReader(src, size) - } - return &Reader{ - md5sum: md5sum, - sha256sum: sha256sum, - src: src, - size: size, - md5Hash: md5Hash, - sha256Hash: sha256Hash, - actualSize: actualSize, - }, nil + // Create empty reader and merge into that. + r := Reader{src: src, size: -1, actualSize: -1} + return r.merge(size, md5Hex, sha256Hex, actualSize, strictCompat) } func (r *Reader) Read(p []byte) (n int, err error) { @@ -94,10 +63,11 @@ func (r *Reader) Read(p []byte) (n int, err error) { r.sha256Hash.Write(p[:n]) } } + r.bytesRead += int64(n) // At io.EOF verify if the checksums are right. if err == io.EOF { - if cerr := r.Verify(); cerr != nil { + if cerr := r.verify(); cerr != nil { return 0, cerr } } @@ -150,9 +120,9 @@ func (r *Reader) SHA256HexString() string { return hex.EncodeToString(r.sha256sum) } -// Verify verifies if the computed MD5 sum and SHA256 sum are +// verify verifies if the computed MD5 sum and SHA256 sum are // equal to the ones specified when creating the Reader. -func (r *Reader) Verify() error { +func (r *Reader) verify() error { if r.sha256Hash != nil && len(r.sha256sum) > 0 { if sum := r.sha256Hash.Sum(nil); !bytes.Equal(r.sha256sum, sum) { return SHA256Mismatch{hex.EncodeToString(r.sha256sum), hex.EncodeToString(sum)} @@ -165,3 +135,63 @@ func (r *Reader) Verify() error { } return nil } + +// merge another hash into this one. +// There cannot be conflicting information given. +func (r *Reader) merge(size int64, md5Hex, sha256Hex string, actualSize int64, strictCompat bool) (*Reader, error) { + if r.bytesRead > 0 { + return nil, errors.New("internal error: Already read from hash reader") + } + // Merge sizes. + // If not set before, just add it. + if r.size < 0 && size >= 0 { + r.src = io.LimitReader(r.src, size) + r.size = size + } + // If set before and set now they must match. + if r.size >= 0 && size >= 0 && r.size != size { + return nil, ErrSizeMismatch{Want: r.size, Got: size} + } + + if r.actualSize <= 0 && actualSize >= 0 { + r.actualSize = actualSize + } + + // Merge SHA256. + sha256sum, err := hex.DecodeString(sha256Hex) + if err != nil { + return nil, SHA256Mismatch{} + } + + // If both are set, they must be the same. + if r.sha256Hash != nil && len(sha256sum) > 0 { + if !bytes.Equal(r.sha256sum, sha256sum) { + return nil, SHA256Mismatch{} + } + } else if len(sha256sum) > 0 { + r.sha256Hash = sha256.New() + r.sha256sum = sha256sum + } + + // Merge MD5 Sum. + md5sum, err := hex.DecodeString(md5Hex) + if err != nil { + return nil, BadDigest{} + } + // If both are set, they must expect the same. + if r.md5Hash != nil && len(md5sum) > 0 { + if !bytes.Equal(r.md5sum, md5sum) { + return nil, BadDigest{} + } + } else if len(md5sum) > 0 || (r.md5Hash == nil && strictCompat) { + r.md5Hash = md5.New() + r.md5sum = md5sum + } + return r, nil +} + +// Close and release resources. +func (r *Reader) Close() error { + // Support the io.Closer interface. + return nil +} diff --git a/pkg/hash/reader_test.go b/pkg/hash/reader_test.go index 22c8b6af0..1942b377a 100644 --- a/pkg/hash/reader_test.go +++ b/pkg/hash/reader_test.go @@ -19,6 +19,7 @@ package hash import ( "bytes" "encoding/hex" + "fmt" "io" "io/ioutil" "testing" @@ -71,20 +72,21 @@ func TestHashReaderHelperMethods(t *testing.T) { // Tests hash reader checksum verification. func TestHashReaderVerification(t *testing.T) { testCases := []struct { + desc string src io.Reader size int64 actualSize int64 md5hex, sha256hex string err error }{ - // Success, no checksum verification provided. { + desc: "Success, no checksum verification provided.", src: bytes.NewReader([]byte("abcd")), size: 4, actualSize: 4, }, - // Failure md5 mismatch. { + desc: "Failure md5 mismatch.", src: bytes.NewReader([]byte("abcd")), size: 4, actualSize: 4, @@ -94,8 +96,8 @@ func TestHashReaderVerification(t *testing.T) { "e2fc714c4727ee9395f324cd2e7f331f", }, }, - // Failure sha256 mismatch. { + desc: "Failure sha256 mismatch.", src: bytes.NewReader([]byte("abcd")), size: 4, actualSize: 4, @@ -105,33 +107,123 @@ func TestHashReaderVerification(t *testing.T) { "88d4266fd4e6338d13b845fcf289579d209c897823b9217da3e161936f031589", }, }, + { + desc: "Nested hash reader NewReader() should merge.", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "", "", 4, false), + size: 4, + actualSize: 4, + }, + { + desc: "Incorrect sha256, nested", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "", "", 4, false), + size: 4, + actualSize: 4, + sha256hex: "50d858e0985ecc7f60418aaf0cc5ab587f42c2570a884095a9e8ccacd0f6545c", + err: SHA256Mismatch{ + ExpectedSHA256: "50d858e0985ecc7f60418aaf0cc5ab587f42c2570a884095a9e8ccacd0f6545c", + CalculatedSHA256: "88d4266fd4e6338d13b845fcf289579d209c897823b9217da3e161936f031589", + }, + }, + { + desc: "Correct sha256, nested", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "", "", 4, false), + size: 4, + actualSize: 4, + sha256hex: "88d4266fd4e6338d13b845fcf289579d209c897823b9217da3e161936f031589", + }, + { + desc: "Correct sha256, nested, truncated", + src: mustReader(t, bytes.NewReader([]byte("abcd-more-stuff-to-be ignored")), 4, "", "", 4, false), + size: 4, + actualSize: -1, + sha256hex: "88d4266fd4e6338d13b845fcf289579d209c897823b9217da3e161936f031589", + }, + { + desc: "Correct sha256, nested, truncated, swapped", + src: mustReader(t, bytes.NewReader([]byte("abcd-more-stuff-to-be ignored")), 4, "", "", -1, false), + size: 4, + actualSize: -1, + sha256hex: "88d4266fd4e6338d13b845fcf289579d209c897823b9217da3e161936f031589", + }, + { + desc: "Incorrect MD5, nested", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "", "", 4, false), + size: 4, + actualSize: 4, + md5hex: "0773da587b322af3a8718cb418a715ce", + err: BadDigest{ + ExpectedMD5: "0773da587b322af3a8718cb418a715ce", + CalculatedMD5: "e2fc714c4727ee9395f324cd2e7f331f", + }, + }, + { + desc: "Correct sha256, truncated", + src: bytes.NewReader([]byte("abcd-morethan-4-bytes")), + size: 4, + actualSize: 4, + sha256hex: "88d4266fd4e6338d13b845fcf289579d209c897823b9217da3e161936f031589", + }, + { + desc: "Correct MD5, nested", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "", "", 4, false), + size: 4, + actualSize: 4, + md5hex: "e2fc714c4727ee9395f324cd2e7f331f", + }, + { + desc: "Correct MD5, truncated", + src: bytes.NewReader([]byte("abcd-morethan-4-bytes")), + size: 4, + actualSize: 4, + sha256hex: "", + md5hex: "e2fc714c4727ee9395f324cd2e7f331f", + }, + { + desc: "Correct MD5, nested, truncated", + src: mustReader(t, bytes.NewReader([]byte("abcd-morestuff")), -1, "", "", -1, false), + size: 4, + actualSize: 4, + md5hex: "e2fc714c4727ee9395f324cd2e7f331f", + }, } for i, testCase := range testCases { - r, err := NewReader(testCase.src, testCase.size, testCase.md5hex, testCase.sha256hex, testCase.actualSize, false) - if err != nil { - t.Fatalf("Test %d: Initializing reader failed %s", i+1, err) - } - _, err = io.Copy(ioutil.Discard, r) - if err != nil { - if err.Error() != testCase.err.Error() { - t.Errorf("Test %d: Expected error %s, got error %s", i+1, testCase.err, err) + t.Run(fmt.Sprintf("case-%d", i+1), func(t *testing.T) { + r, err := NewReader(testCase.src, testCase.size, testCase.md5hex, testCase.sha256hex, testCase.actualSize, false) + if err != nil { + t.Fatalf("Test %q: Initializing reader failed %s", testCase.desc, err) + } + _, err = io.Copy(ioutil.Discard, r) + if err != nil { + if err.Error() != testCase.err.Error() { + t.Errorf("Test %q: Expected error %s, got error %s", testCase.desc, testCase.err, err) + } } - } + }) + } +} + +func mustReader(t *testing.T, src io.Reader, size int64, md5Hex, sha256Hex string, actualSize int64, strictCompat bool) *Reader { + r, err := NewReader(src, size, md5Hex, sha256Hex, actualSize, strictCompat) + if err != nil { + t.Fatal(err) } + return r } // Tests NewReader() constructor with invalid arguments. func TestHashReaderInvalidArguments(t *testing.T) { testCases := []struct { + desc string src io.Reader size int64 actualSize int64 md5hex, sha256hex string success bool expectedErr error + strict bool }{ - // Invalid md5sum NewReader() will fail. { + desc: "Invalid md5sum NewReader() will fail.", src: bytes.NewReader([]byte("abcd")), size: 4, actualSize: 4, @@ -139,8 +231,8 @@ func TestHashReaderInvalidArguments(t *testing.T) { success: false, expectedErr: BadDigest{}, }, - // Invalid sha256 NewReader() will fail. { + desc: "Invalid sha256 NewReader() will fail.", src: bytes.NewReader([]byte("abcd")), size: 4, actualSize: 4, @@ -148,33 +240,78 @@ func TestHashReaderInvalidArguments(t *testing.T) { success: false, expectedErr: SHA256Mismatch{}, }, - // Nested hash reader NewReader() will fail. { - src: &Reader{src: bytes.NewReader([]byte("abcd"))}, + desc: "Nested hash reader NewReader() should merge.", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "", "", 4, false), + size: 4, + actualSize: 4, + success: true, + }, + { + desc: "Mismatching sha256", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "", "88d4266fd4e6338d13b845fcf289579d209c897823b9217da3e161936f031589", 4, false), + size: 4, + actualSize: 4, + sha256hex: "50d858e0985ecc7f60418aaf0cc5ab587f42c2570a884095a9e8ccacd0f6545c", + success: false, + expectedErr: SHA256Mismatch{}, + }, + { + desc: "Correct sha256", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "", "88d4266fd4e6338d13b845fcf289579d209c897823b9217da3e161936f031589", 4, false), + size: 4, + actualSize: 4, + sha256hex: "88d4266fd4e6338d13b845fcf289579d209c897823b9217da3e161936f031589", + success: true, + }, + { + desc: "Mismatching MD5", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "e2fc714c4727ee9395f324cd2e7f331f", "", 4, false), size: 4, actualSize: 4, + md5hex: "0773da587b322af3a8718cb418a715ce", success: false, - expectedErr: errNestedReader, + expectedErr: BadDigest{}, }, - // Expected inputs, NewReader() will succeed. { + desc: "Correct MD5", + src: mustReader(t, bytes.NewReader([]byte("abcd")), 4, "e2fc714c4727ee9395f324cd2e7f331f", "", 4, false), + size: 4, + actualSize: 4, + md5hex: "e2fc714c4727ee9395f324cd2e7f331f", + success: true, + }, + { + desc: "Nothing, all ok", src: bytes.NewReader([]byte("abcd")), size: 4, actualSize: 4, success: true, }, + { + desc: "Nested, size mismatch", + src: mustReader(t, bytes.NewReader([]byte("abcd-morestuff")), 4, "", "", -1, false), + size: 2, + actualSize: -1, + success: false, + expectedErr: ErrSizeMismatch{Want: 4, Got: 2}, + }, } for i, testCase := range testCases { - _, err := NewReader(testCase.src, testCase.size, testCase.md5hex, testCase.sha256hex, testCase.actualSize, false) - if err != nil && testCase.success { - t.Errorf("Test %d: Expected success, but got error %s instead", i+1, err) - } - if err == nil && !testCase.success { - t.Errorf("Test %d: Expected error, but got success", i+1) - } - if err != testCase.expectedErr { - t.Errorf("Test %d: Expected error %v, but got %v", i+1, testCase.expectedErr, err) - } + t.Run(fmt.Sprintf("case-%d", i+1), func(t *testing.T) { + _, err := NewReader(testCase.src, testCase.size, testCase.md5hex, testCase.sha256hex, testCase.actualSize, testCase.strict) + if err != nil && testCase.success { + t.Errorf("Test %q: Expected success, but got error %s instead", testCase.desc, err) + } + if err == nil && !testCase.success { + t.Errorf("Test %q: Expected error, but got success", testCase.desc) + } + if !testCase.success { + if err != testCase.expectedErr { + t.Errorf("Test %q: Expected error %v, but got %v", testCase.desc, testCase.expectedErr, err) + } + } + }) } }