Skip to content

Commit 43459b0

Browse files
harshavardhanaharshavardhana
authored
fix: use seeker to avoid memory without multipart upload (#1699)
refer #15754
1 parent ecbd36d commit 43459b0

21 files changed

+151
-131
lines changed

‎.github/workflows/go-windows.yml

+8-5
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,23 @@
1-
name: Go
1+
name: Build (Windows)
22

33
on:
44
pull_request:
55
branches:
66
- master
7-
push:
8-
branches:
9-
- master
7+
8+
# This ensures that previous jobs for the PR are canceled when the PR is
9+
# updated.
10+
concurrency:
11+
group: ${{ github.workflow }}-${{ github.head_ref }}
12+
cancel-in-progress: true
1013

1114
jobs:
1215
build:
1316
name: Test on Go ${{ matrix.go-version }} and ${{ matrix.os }}
1417
runs-on: ${{ matrix.os }}
1518
strategy:
1619
matrix:
17-
go-version: [1.17.x]
20+
go-version: [1.18.x, 1.19.x]
1821
os: [windows-latest]
1922
steps:
2023
- name: Set up Go ${{ matrix.go-version }} on ${{ matrix.os }}

‎.github/workflows/go.yml

+8-5
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,23 @@
1-
name: Go
1+
name: Build (Linux)
22

33
on:
44
pull_request:
55
branches:
66
- master
7-
push:
8-
branches:
9-
- master
7+
8+
# This ensures that previous jobs for the PR are canceled when the PR is
9+
# updated.
10+
concurrency:
11+
group: ${{ github.workflow }}-${{ github.head_ref }}
12+
cancel-in-progress: true
1013

1114
jobs:
1215
build:
1316
name: Test on Go ${{ matrix.go-version }} and ${{ matrix.os }}
1417
runs-on: ${{ matrix.os }}
1518
strategy:
1619
matrix:
17-
go-version: [1.17.x, 1.18.x]
20+
go-version: [1.18.x, 1.19.x]
1821
os: [ubuntu-latest]
1922
steps:
2023
- name: Set up Go ${{ matrix.go-version }} on ${{ matrix.os }}

‎.gitignore

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
*~
22
*.test
33
validator
4-
golangci-lint
4+
golangci-lint
5+
functional_tests

‎.golangci.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ linters:
1212
- govet
1313
- ineffassign
1414
- gosimple
15-
- deadcode
16-
- structcheck
15+
- unused
1716
- gocritic
1817

1918
issues:
@@ -25,3 +24,4 @@ issues:
2524
- "captLocal:"
2625
- "ifElseChain:"
2726
- "elseif:"
27+
- "should have a package comment"

‎Makefile

+3-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ checks: lint vet test examples functional-test
99

1010
lint:
1111
@mkdir -p ${GOPATH}/bin
12-
@echo "Installing golangci-lint" && curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH)/bin v1.45.2
12+
@echo "Installing golangci-lint" && curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH)/bin
1313
@echo "Running $@ check"
1414
@GO111MODULE=on ${GOPATH}/bin/golangci-lint cache clean
1515
@GO111MODULE=on ${GOPATH}/bin/golangci-lint run --timeout=5m --config ./.golangci.yml
@@ -27,7 +27,8 @@ examples:
2727
@cd ./examples/minio && $(foreach v,$(wildcard examples/minio/*.go),go build -mod=mod -o ${TMPDIR}/$(basename $(v)) $(notdir $(v)) || exit 1;)
2828

2929
functional-test:
30-
@GO111MODULE=on SERVER_ENDPOINT=localhost:9000 ACCESS_KEY=minio SECRET_KEY=minio123 ENABLE_HTTPS=1 MINT_MODE=full go run functional_tests.go
30+
@GO111MODULE=on go build -race functional_tests.go
31+
@SERVER_ENDPOINT=localhost:9000 ACCESS_KEY=minio SECRET_KEY=minio123 ENABLE_HTTPS=1 MINT_MODE=full ./functional_tests
3132

3233
clean:
3334
@echo "Cleaning up all the generated files"

‎api-error-response.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,14 @@ type ErrorResponse struct {
6767
//
6868
// For example:
6969
//
70-
// import s3 "github.com/minio/minio-go/v7"
71-
// ...
72-
// ...
73-
// reader, stat, err := s3.GetObject(...)
74-
// if err != nil {
75-
// resp := s3.ToErrorResponse(err)
76-
// }
77-
// ...
70+
// import s3 "github.com/minio/minio-go/v7"
71+
// ...
72+
// ...
73+
// reader, stat, err := s3.GetObject(...)
74+
// if err != nil {
75+
// resp := s3.ToErrorResponse(err)
76+
// }
77+
// ...
7878
func ToErrorResponse(err error) ErrorResponse {
7979
switch err := err.(type) {
8080
case ErrorResponse:

‎api-list.go

+15-17
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,10 @@ import (
3232
// This call requires explicit authentication, no anonymous requests are
3333
// allowed for listing buckets.
3434
//
35-
// api := client.New(....)
36-
// for message := range api.ListBuckets(context.Background()) {
37-
// fmt.Println(message)
38-
// }
39-
//
35+
// api := client.New(....)
36+
// for message := range api.ListBuckets(context.Background()) {
37+
// fmt.Println(message)
38+
// }
4039
func (c *Client) ListBuckets(ctx context.Context) ([]BucketInfo, error) {
4140
// Execute GET on service.
4241
resp, err := c.executeMethod(ctx, http.MethodGet, requestMetadata{contentSHA256Hex: emptySHA256Hex})
@@ -659,11 +658,10 @@ func (o *ListObjectsOptions) Set(key, value string) {
659658

660659
// ListObjects returns objects list after evaluating the passed options.
661660
//
662-
// api := client.New(....)
663-
// for object := range api.ListObjects(ctx, "mytestbucket", minio.ListObjectsOptions{Prefix: "starthere", Recursive:true}) {
664-
// fmt.Println(object)
665-
// }
666-
//
661+
// api := client.New(....)
662+
// for object := range api.ListObjects(ctx, "mytestbucket", minio.ListObjectsOptions{Prefix: "starthere", Recursive:true}) {
663+
// fmt.Println(object)
664+
// }
667665
func (c *Client) ListObjects(ctx context.Context, bucketName string, opts ListObjectsOptions) <-chan ObjectInfo {
668666
if opts.WithVersions {
669667
return c.listObjectVersions(ctx, bucketName, opts)
@@ -694,12 +692,12 @@ func (c *Client) ListObjects(ctx context.Context, bucketName string, opts ListOb
694692
// If you enable recursive as 'true' this function will return back all
695693
// the multipart objects in a given bucket name.
696694
//
697-
// api := client.New(....)
698-
// // Recurively list all objects in 'mytestbucket'
699-
// recursive := true
700-
// for message := range api.ListIncompleteUploads(context.Background(), "mytestbucket", "starthere", recursive) {
701-
// fmt.Println(message)
702-
// }
695+
// api := client.New(....)
696+
// // Recurively list all objects in 'mytestbucket'
697+
// recursive := true
698+
// for message := range api.ListIncompleteUploads(context.Background(), "mytestbucket", "starthere", recursive) {
699+
// fmt.Println(message)
700+
// }
703701
func (c *Client) ListIncompleteUploads(ctx context.Context, bucketName, objectPrefix string, recursive bool) <-chan ObjectMultipartInfo {
704702
return c.listIncompleteUploads(ctx, bucketName, objectPrefix, recursive)
705703
}
@@ -916,7 +914,7 @@ func (c *Client) findUploadIDs(ctx context.Context, bucketName, objectName strin
916914
}
917915

918916
// listObjectPartsQuery (List Parts query)
919-
// - lists some or all (up to 1000) parts that have been uploaded
917+
// - lists some or all (up to 1000) parts that have been uploaded
920918
// for a specific multipart upload
921919
//
922920
// You can use the request parameters as selection criteria to return

‎api-put-object-common.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,9 @@ func isReadAt(reader io.Reader) (ok bool) {
6565
// NOTE: Assumption here is that for any object to be uploaded to any S3 compatible
6666
// object storage it will have the following parameters as constants.
6767
//
68-
// maxPartsCount - 10000
69-
// minPartSize - 16MiB
70-
// maxMultipartPutObjectSize - 5TiB
71-
//
68+
// maxPartsCount - 10000
69+
// minPartSize - 16MiB
70+
// maxMultipartPutObjectSize - 5TiB
7271
func OptimalPartInfo(objectSize int64, configuredPartSize uint64) (totalPartsCount int, partSize int64, lastPartSize int64, err error) {
7372
// object size is '-1' set it to 5TiB.
7473
var unknownSize bool

‎api-put-object-streaming.go

+26-11
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,7 @@ func (c *Client) putObject(ctx context.Context, bucketName, objectName string, r
419419
return UploadInfo{}, errInvalidArgument("MD5Sum cannot be calculated with size '-1'")
420420
}
421421

422+
var readSeeker io.Seeker
422423
if size > 0 {
423424
if isReadAt(reader) && !isObject(reader) {
424425
seeker, ok := reader.(io.Seeker)
@@ -428,35 +429,49 @@ func (c *Client) putObject(ctx context.Context, bucketName, objectName string, r
428429
return UploadInfo{}, errInvalidArgument(err.Error())
429430
}
430431
reader = io.NewSectionReader(reader.(io.ReaderAt), offset, size)
432+
readSeeker = reader.(io.Seeker)
431433
}
432434
}
433435
}
434436

435437
var md5Base64 string
436438
if opts.SendContentMd5 {
437-
// Create a buffer.
438-
buf := make([]byte, size)
439+
// Calculate md5sum.
440+
hash := c.md5Hasher()
439441

440-
length, rErr := readFull(reader, buf)
441-
if rErr != nil && rErr != io.ErrUnexpectedEOF && rErr != io.EOF {
442-
return UploadInfo{}, rErr
442+
if readSeeker != nil {
443+
if _, err := io.Copy(hash, reader); err != nil {
444+
return UploadInfo{}, err
445+
}
446+
// Seek back to beginning of io.NewSectionReader's offset.
447+
_, err = readSeeker.Seek(0, io.SeekStart)
448+
if err != nil {
449+
return UploadInfo{}, errInvalidArgument(err.Error())
450+
}
451+
} else {
452+
// Create a buffer.
453+
buf := make([]byte, size)
454+
455+
length, err := readFull(reader, buf)
456+
if err != nil && err != io.ErrUnexpectedEOF && err != io.EOF {
457+
return UploadInfo{}, err
458+
}
459+
460+
hash.Write(buf[:length])
461+
reader = bytes.NewReader(buf[:length])
443462
}
444463

445-
// Calculate md5sum.
446-
hash := c.md5Hasher()
447-
hash.Write(buf[:length])
448464
md5Base64 = base64.StdEncoding.EncodeToString(hash.Sum(nil))
449-
reader = bytes.NewReader(buf[:length])
450465
hash.Close()
451466
}
452467

453468
// Update progress reader appropriately to the latest offset as we
454469
// read from the source.
455-
readSeeker := newHook(reader, opts.Progress)
470+
progressReader := newHook(reader, opts.Progress)
456471

457472
// This function does not calculate sha256 and md5sum for payload.
458473
// Execute put object.
459-
return c.putObjectDo(ctx, bucketName, objectName, readSeeker, md5Base64, "", size, opts)
474+
return c.putObjectDo(ctx, bucketName, objectName, progressReader, md5Base64, "", size, opts)
460475
}
461476

462477
// putObjectDo - executes the put object http operation.

‎api-remove.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ func (c *Client) RemoveBucketWithOptions(ctx context.Context, bucketName string,
8282

8383
// RemoveBucket deletes the bucket name.
8484
//
85-
// All objects (including all object versions and delete markers).
86-
// in the bucket must be deleted before successfully attempting this request.
85+
// All objects (including all object versions and delete markers).
86+
// in the bucket must be deleted before successfully attempting this request.
8787
func (c *Client) RemoveBucket(ctx context.Context, bucketName string) error {
8888
// Input validation.
8989
if err := s3utils.CheckValidBucketName(bucketName); err != nil {

‎api.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ const (
117117
// User Agent should always following the below style.
118118
// Please open an issue to discuss any new changes here.
119119
//
120-
// MinIO (OS; ARCH) LIB/VER APP/VER
120+
// MinIO (OS; ARCH) LIB/VER APP/VER
121121
const (
122122
libraryUserAgentPrefix = "MinIO (" + runtime.GOOS + "; " + runtime.GOARCH + ") "
123123
libraryUserAgent = libraryUserAgentPrefix + libraryName + "/" + libraryVersion
@@ -312,9 +312,9 @@ func (c *Client) SetS3TransferAccelerate(accelerateEndpoint string) {
312312
// Hash materials provides relevant initialized hash algo writers
313313
// based on the expected signature type.
314314
//
315-
// - For signature v4 request if the connection is insecure compute only sha256.
316-
// - For signature v4 request if the connection is secure compute only md5.
317-
// - For anonymous request compute md5.
315+
// - For signature v4 request if the connection is insecure compute only sha256.
316+
// - For signature v4 request if the connection is secure compute only md5.
317+
// - For anonymous request compute md5.
318318
func (c *Client) hashMaterials(isMd5Requested, isSha256Requested bool) (hashAlgos map[string]md5simd.Hasher, hashSums map[string][]byte) {
319319
hashSums = make(map[string][]byte)
320320
hashAlgos = make(map[string]md5simd.Hasher)

‎pkg/credentials/chain.go

+10-11
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,17 @@ package credentials
3131
// will cache that Provider for all calls to IsExpired(), until Retrieve is
3232
// called again after IsExpired() is true.
3333
//
34-
// creds := credentials.NewChainCredentials(
35-
// []credentials.Provider{
36-
// &credentials.EnvAWSS3{},
37-
// &credentials.EnvMinio{},
38-
// })
39-
//
40-
// // Usage of ChainCredentials.
41-
// mc, err := minio.NewWithCredentials(endpoint, creds, secure, "us-east-1")
42-
// if err != nil {
43-
// log.Fatalln(err)
44-
// }
34+
// creds := credentials.NewChainCredentials(
35+
// []credentials.Provider{
36+
// &credentials.EnvAWSS3{},
37+
// &credentials.EnvMinio{},
38+
// })
4539
//
40+
// // Usage of ChainCredentials.
41+
// mc, err := minio.NewWithCredentials(endpoint, creds, secure, "us-east-1")
42+
// if err != nil {
43+
// log.Fatalln(err)
44+
// }
4645
type Chain struct {
4746
Providers []Provider
4847
curr Provider

‎pkg/credentials/credentials.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,11 @@ type Provider interface {
6565
// provider's struct.
6666
//
6767
// Example:
68-
// type IAMCredentialProvider struct {
69-
// Expiry
70-
// ...
71-
// }
68+
//
69+
// type IAMCredentialProvider struct {
70+
// Expiry
71+
// ...
72+
// }
7273
type Expiry struct {
7374
// The date/time when to expire on
7475
expiration time.Time

‎pkg/credentials/doc.go

+16-18
Original file line numberDiff line numberDiff line change
@@ -28,35 +28,33 @@
2828
//
2929
// Example of using the environment variable credentials.
3030
//
31-
// creds := NewFromEnv()
32-
// // Retrieve the credentials value
33-
// credValue, err := creds.Get()
34-
// if err != nil {
35-
// // handle error
36-
// }
31+
// creds := NewFromEnv()
32+
// // Retrieve the credentials value
33+
// credValue, err := creds.Get()
34+
// if err != nil {
35+
// // handle error
36+
// }
3737
//
3838
// Example of forcing credentials to expire and be refreshed on the next Get().
3939
// This may be helpful to proactively expire credentials and refresh them sooner
4040
// than they would naturally expire on their own.
4141
//
42-
// creds := NewFromIAM("")
43-
// creds.Expire()
44-
// credsValue, err := creds.Get()
45-
// // New credentials will be retrieved instead of from cache.
42+
// creds := NewFromIAM("")
43+
// creds.Expire()
44+
// credsValue, err := creds.Get()
45+
// // New credentials will be retrieved instead of from cache.
4646
//
47-
//
48-
// Custom Provider
47+
// # Custom Provider
4948
//
5049
// Each Provider built into this package also provides a helper method to generate
5150
// a Credentials pointer setup with the provider. To use a custom Provider just
5251
// create a type which satisfies the Provider interface and pass it to the
5352
// NewCredentials method.
5453
//
55-
// type MyProvider struct{}
56-
// func (m *MyProvider) Retrieve() (Value, error) {...}
57-
// func (m *MyProvider) IsExpired() bool {...}
58-
//
59-
// creds := NewCredentials(&MyProvider{})
60-
// credValue, err := creds.Get()
54+
// type MyProvider struct{}
55+
// func (m *MyProvider) Retrieve() (Value, error) {...}
56+
// func (m *MyProvider) IsExpired() bool {...}
6157
//
58+
// creds := NewCredentials(&MyProvider{})
59+
// credValue, err := creds.Get()
6260
package credentials

0 commit comments

Comments
 (0)