diff --git a/cmd/bucket-replication-utils.go b/cmd/bucket-replication-utils.go index 6bd747b9b..79e359e02 100644 --- a/cmd/bucket-replication-utils.go +++ b/cmd/bucket-replication-utils.go @@ -20,6 +20,7 @@ package cmd import ( "bytes" "fmt" + "net/http" "regexp" "strconv" "strings" @@ -27,6 +28,7 @@ import ( "time" "github.com/minio/minio/internal/bucket/replication" + xhttp "github.com/minio/minio/internal/http" ) //go:generate msgp -file=$GOFILE @@ -699,3 +701,26 @@ func newBucketResyncStatus(bucket string) BucketReplicationResyncStatus { Version: resyncMetaVersion, } } + +var contentRangeRegexp = regexp.MustCompile(`bytes ([0-9]+)-([0-9]+)/([0-9]+|\\*)`) + +// parse size from content-range header +func parseSizeFromContentRange(h http.Header) (sz int64, err error) { + cr := h.Get(xhttp.ContentRange) + if cr == "" { + return sz, fmt.Errorf("Content-Range not set") + } + parts := contentRangeRegexp.FindStringSubmatch(cr) + if len(parts) != 4 { + return sz, fmt.Errorf("invalid Content-Range header %s", cr) + } + if parts[3] == "*" { + return -1, nil + } + var usz uint64 + usz, err = strconv.ParseUint(parts[3], 10, 64) + if err != nil { + return sz, err + } + return int64(usz), nil +} diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index ce31a22fe..e6d3db98a 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -1531,16 +1531,21 @@ func initBackgroundReplication(ctx context.Context, objectAPI ObjectLayer) { go globalReplicationStats.loadInitialReplicationMetrics(ctx) } +type proxyResult struct { + Proxy bool + Err error +} + // get Reader from replication target if active-active replication is in place and // this node returns a 404 -func proxyGetToReplicationTarget(ctx context.Context, bucket, object string, rs *HTTPRangeSpec, h http.Header, opts ObjectOptions, proxyTargets *madmin.BucketTargets) (gr *GetObjectReader, proxy bool) { - tgt, oi, proxy := proxyHeadToRepTarget(ctx, bucket, object, opts, proxyTargets) - if !proxy { - return nil, false +func proxyGetToReplicationTarget(ctx context.Context, bucket, object string, rs *HTTPRangeSpec, h http.Header, opts ObjectOptions, proxyTargets *madmin.BucketTargets) (gr *GetObjectReader, proxy proxyResult, err error) { + tgt, oi, proxy := proxyHeadToRepTarget(ctx, bucket, object, rs, opts, proxyTargets) + if !proxy.Proxy { + return nil, proxy, nil } - fn, off, length, err := NewGetObjectReader(rs, oi, opts) + fn, _, _, err := NewGetObjectReader(nil, oi, opts) if err != nil { - return nil, false + return nil, proxy, err } gopts := miniogo.GetObjectOptions{ VersionID: opts.VersionID, @@ -1548,30 +1553,40 @@ func proxyGetToReplicationTarget(ctx context.Context, bucket, object string, rs Internal: miniogo.AdvancedGetOptions{ ReplicationProxyRequest: "true", }, + PartNumber: opts.PartNumber, } // get correct offsets for encrypted object - if off >= 0 && length >= 0 { - if err := gopts.SetRange(off, off+length-1); err != nil { - return nil, false + if rs != nil { + h, err := rs.ToHeader() + if err != nil { + return nil, proxy, err } + gopts.Set(xhttp.Range, h) } // Make sure to match ETag when proxying. if err = gopts.SetMatchETag(oi.ETag); err != nil { - return nil, false + return nil, proxy, err } c := miniogo.Core{Client: tgt.Client} - obj, _, _, err := c.GetObject(ctx, bucket, object, gopts) + obj, _, h, err := c.GetObject(ctx, bucket, object, gopts) if err != nil { - return nil, false + return nil, proxy, err } closeReader := func() { obj.Close() } - reader, err := fn(obj, h, closeReader) if err != nil { - return nil, false + return nil, proxy, err } reader.ObjInfo = oi.Clone() - return reader, true + if rs != nil { + contentSize, err := parseSizeFromContentRange(h) + if err != nil { + return nil, proxy, err + } + reader.ObjInfo.Size = contentSize + } + + return reader, proxyResult{Proxy: true}, nil } func getproxyTargets(ctx context.Context, bucket, object string, opts ObjectOptions) (tgts *madmin.BucketTargets) { @@ -1590,11 +1605,11 @@ func getproxyTargets(ctx context.Context, bucket, object string, opts ObjectOpti return tgts } -func proxyHeadToRepTarget(ctx context.Context, bucket, object string, opts ObjectOptions, proxyTargets *madmin.BucketTargets) (tgt *TargetClient, oi ObjectInfo, proxy bool) { +func proxyHeadToRepTarget(ctx context.Context, bucket, object string, rs *HTTPRangeSpec, opts ObjectOptions, proxyTargets *madmin.BucketTargets) (tgt *TargetClient, oi ObjectInfo, proxy proxyResult) { // this option is set when active-active replication is in place between site A -> B, // and site B does not have the object yet. if opts.ProxyRequest || (opts.ProxyHeaderSet && !opts.ProxyRequest) { // true only when site B sets MinIOSourceProxyRequest header - return nil, oi, false + return nil, oi, proxy } for _, t := range proxyTargets.Targets { tgt = globalBucketTargetSys.GetRemoteTargetClient(ctx, t.Arn) @@ -1612,9 +1627,22 @@ func proxyHeadToRepTarget(ctx context.Context, bucket, object string, opts Objec Internal: miniogo.AdvancedGetOptions{ ReplicationProxyRequest: "true", }, + PartNumber: opts.PartNumber, } + if rs != nil { + h, err := rs.ToHeader() + if err != nil { + logger.LogIf(ctx, fmt.Errorf("Invalid range header for %s/%s(%s) - %w", bucket, object, opts.VersionID, err)) + continue + } + gopts.Set(xhttp.Range, h) + } + objInfo, err := tgt.StatObject(ctx, t.TargetBucket, object, gopts) if err != nil { + if isErrInvalidRange(ErrorRespToObjectError(err, bucket, object)) { + return nil, oi, proxyResult{Err: err} + } continue } @@ -1645,15 +1673,15 @@ func proxyHeadToRepTarget(ctx context.Context, bucket, object string, opts Objec if ok { oi.ContentEncoding = ce } - return tgt, oi, true + return tgt, oi, proxyResult{Proxy: true} } - return nil, oi, false + return nil, oi, proxy } // get object info from replication target if active-active replication is in place and // this node returns a 404 -func proxyHeadToReplicationTarget(ctx context.Context, bucket, object string, opts ObjectOptions, proxyTargets *madmin.BucketTargets) (oi ObjectInfo, proxy bool) { - _, oi, proxy = proxyHeadToRepTarget(ctx, bucket, object, opts, proxyTargets) +func proxyHeadToReplicationTarget(ctx context.Context, bucket, object string, rs *HTTPRangeSpec, opts ObjectOptions, proxyTargets *madmin.BucketTargets) (oi ObjectInfo, proxy proxyResult) { + _, oi, proxy = proxyHeadToRepTarget(ctx, bucket, object, rs, opts, proxyTargets) return oi, proxy } diff --git a/cmd/httprange.go b/cmd/httprange.go index 232991741..db2229905 100644 --- a/cmd/httprange.go +++ b/cmd/httprange.go @@ -174,3 +174,26 @@ func (h *HTTPRangeSpec) String(resourceSize int64) string { } return fmt.Sprintf("%d-%d", off, off+length-1) } + +// ToHeader returns the Range header value. +func (h *HTTPRangeSpec) ToHeader() (string, error) { + if h == nil { + return "", nil + } + start := strconv.Itoa(int(h.Start)) + end := strconv.Itoa(int(h.End)) + switch { + case h.Start >= 0 && h.End >= 0: + if h.Start > h.End { + return "", errInvalidRange + } + case h.IsSuffixLength: + end = strconv.Itoa(int(h.Start * -1)) + start = "" + case h.Start > -1: + end = "" + default: + return "", fmt.Errorf("does not have valid range value") + } + return fmt.Sprintf("bytes=%s-%s", start, end), nil +} diff --git a/cmd/httprange_test.go b/cmd/httprange_test.go index 1614e96c3..2ce9c4e9f 100644 --- a/cmd/httprange_test.go +++ b/cmd/httprange_test.go @@ -101,3 +101,46 @@ func TestHTTPRequestRangeSpec(t *testing.T) { t.Errorf("Case %d: Expected errInvalidRange but: %v %v %d %d %v", i, rs, err1, o, l, err2) } } + +func TestHTTPRequestRangeToHeader(t *testing.T) { + validRangeSpecs := []struct { + spec string + errExpected bool + }{ + {"bytes=0-", false}, + {"bytes=1-", false}, + + {"bytes=0-9", false}, + {"bytes=1-10", false}, + {"bytes=1-1", false}, + {"bytes=2-5", false}, + + {"bytes=-5", false}, + {"bytes=-1", false}, + {"bytes=-1000", false}, + {"bytes=", true}, + {"bytes= ", true}, + {"byte=", true}, + {"bytes=A-B", true}, + {"bytes=1-B", true}, + {"bytes=B-1", true}, + {"bytes=-1-1", true}, + } + for i, testCase := range validRangeSpecs { + rs, err := parseRequestRangeSpec(testCase.spec) + if err != nil { + if !testCase.errExpected || err == nil && testCase.errExpected { + t.Errorf("unexpected err: %v", err) + } + continue + } + h, err := rs.ToHeader() + if err != nil && !testCase.errExpected || err == nil && testCase.errExpected { + t.Errorf("expected error with invalid range: %v", err) + } + if h != testCase.spec { + t.Errorf("Case %d: translated to incorrect header: %s expected: %s", + i, h, testCase.spec) + } + } +} diff --git a/cmd/object-api-errors.go b/cmd/object-api-errors.go index 58ad3dbca..62b4ee926 100644 --- a/cmd/object-api-errors.go +++ b/cmd/object-api-errors.go @@ -701,3 +701,7 @@ func isErrMethodNotAllowed(err error) bool { var methodNotAllowed MethodNotAllowed return errors.As(err, &methodNotAllowed) } + +func isErrInvalidRange(err error) bool { + return errors.As(err, &errInvalidRange) +} diff --git a/cmd/object-handlers.go b/cmd/object-handlers.go index 29e4290b5..fbca4c4a5 100644 --- a/cmd/object-handlers.go +++ b/cmd/object-handlers.go @@ -412,20 +412,28 @@ func (api objectAPIHandlers) getObjectHandler(ctx context.Context, objectAPI Obj if err != nil { var ( reader *GetObjectReader - proxy bool + proxy proxyResult ) proxytgts := getproxyTargets(ctx, bucket, object, opts) if !proxytgts.Empty() { // proxy to replication target if active-active replication is in place. - reader, proxy = proxyGetToReplicationTarget(ctx, bucket, object, rs, r.Header, opts, proxytgts) - if reader != nil && proxy { + reader, proxy, err = proxyGetToReplicationTarget(ctx, bucket, object, rs, r.Header, opts, proxytgts) + if err != nil && !isErrObjectNotFound(ErrorRespToObjectError(err, bucket, object)) && + !isErrVersionNotFound(ErrorRespToObjectError(err, bucket, object)) { + logger.LogIf(ctx, fmt.Errorf("Replication proxy failed for %s/%s(%s) - %w", bucket, object, opts.VersionID, err)) + } + if reader != nil && proxy.Proxy && err == nil { gr = reader } } - if reader == nil || !proxy { + if reader == nil || !proxy.Proxy { if isErrPreconditionFailed(err) { return } + if proxy.Err != nil { + writeErrorResponse(ctx, w, toAPIError(ctx, proxy.Err), r.URL) + return + } if globalBucketVersioningSys.Enabled(bucket) && gr != nil { if !gr.ObjInfo.VersionPurgeStatus.Empty() { // Shows the replication status of a permanent delete of a version @@ -631,22 +639,28 @@ func (api objectAPIHandlers) headObjectHandler(ctx context.Context, objectAPI Ob writeErrorResponseHeadersOnly(w, errorCodes.ToAPIErr(s3Error)) return } + // Get request range. + var rs *HTTPRangeSpec + rangeHeader := r.Header.Get(xhttp.Range) objInfo, err := getObjectInfo(ctx, bucket, object, opts) if err != nil { var ( - proxy bool + proxy proxyResult oi ObjectInfo ) // proxy HEAD to replication target if active-active replication configured on bucket proxytgts := getproxyTargets(ctx, bucket, object, opts) if !proxytgts.Empty() { - oi, proxy = proxyHeadToReplicationTarget(ctx, bucket, object, opts, proxytgts) - if proxy { + if rangeHeader != "" { + rs, _ = parseRequestRangeSpec(rangeHeader) + } + oi, proxy = proxyHeadToReplicationTarget(ctx, bucket, object, rs, opts, proxytgts) + if proxy.Proxy { objInfo = oi } } - if !proxy { + if !proxy.Proxy { if globalBucketVersioningSys.Enabled(bucket) { switch { case !objInfo.VersionPurgeStatus.Empty(): @@ -703,9 +717,6 @@ func (api objectAPIHandlers) headObjectHandler(ctx context.Context, objectAPI Ob return } - // Get request range. - var rs *HTTPRangeSpec - rangeHeader := r.Header.Get(xhttp.Range) if rangeHeader != "" { // Both 'Range' and 'partNumber' cannot be specified at the same time if opts.PartNumber > 0 {