Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable stat operations for specific ranges #1846

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion .github/workflows/vulncheck.yml
Expand Up @@ -14,7 +14,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
go-version: [ 1.20.6 ]
go-version: [ 1.20.7 ]
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@v3
Expand Down
6 changes: 5 additions & 1 deletion .gitignore
Expand Up @@ -3,4 +3,8 @@
validator
golangci-lint
functional_tests
.idea
.idea
.DS_Store

# Go workspace file
go.work
47 changes: 34 additions & 13 deletions api-get-object.go
Expand Up @@ -84,7 +84,9 @@ func (c *Client) GetObject(ctx context.Context, bucketName, objectName string, o
// Range is set with respect to the offset and length of the buffer requested.
// Do not set objectInfo from the first readAt request because it will not get
// the whole object.
opts.SetRange(req.Offset, req.Offset+int64(len(req.Buffer))-1)
rangeStart := req.Offset
rangeEnd := req.Offset + int64(len(req.Buffer)) - 1
opts.SetRange(rangeStart, rangeEnd)
} else if req.Offset > 0 {
opts.SetRange(req.Offset, 0)
}
Expand Down Expand Up @@ -129,19 +131,39 @@ func (c *Client) GetObject(ctx context.Context, bucketName, objectName string, o
// Only need to run a StatObject until an actual Read or ReadAt request comes through.

// Remove range header if already set, for stat Operations to get original file size.
delete(opts.headers, "Range")
harshavardhana marked this conversation as resolved.
Show resolved Hide resolved
objectInfo, err = c.StatObject(gctx, bucketName, objectName, StatObjectOptions(opts))
if err != nil {
_, rangeHeaderPresent := opts.headers["Range"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_, rangeHeaderPresent := opts.headers["Range"]
_, ok := opts.headers["Range"]

if rangeHeaderPresent {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if rangeHeaderPresent {
if ok {

objectInfo, err = c.StatObject(gctx, bucketName, objectName, StatObjectOptions(opts))

if err != nil {
resCh <- getResponse{
Error: err,
}
// Exit the go-routine.
return
}
etag = objectInfo.ETag
// Send back the first response.
resCh <- getResponse{
Error: err,
objectInfo: objectInfo,
}
} else {
// The range header is not present, continue with the existing objectInfo.
delete(opts.headers, "Range")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
delete(opts.headers, "Range")

Not needed in else {

objectInfo, err = c.StatObject(gctx, bucketName, objectName, StatObjectOptions(opts))

if err != nil {
resCh <- getResponse{
Error: err,
}
// Exit the go-routine.
return
}
etag = objectInfo.ETag
// Send back the first response.
resCh <- getResponse{
objectInfo: objectInfo,
}
// Exit the go-routine.
return
}
etag = objectInfo.ETag
// Send back the first response.
resCh <- getResponse{
objectInfo: objectInfo,
}
}
} else if req.settingObjectInfo { // Request is just to get objectInfo.
Expand Down Expand Up @@ -415,7 +437,6 @@ func (o *Object) Stat() (ObjectInfo, error) {
if o.prevErr != nil && o.prevErr != io.EOF || o.isClosed {
return ObjectInfo{}, o.prevErr
}

// This is the first request.
if !o.isStarted || !o.objectInfoSet {
// Send the request and get the response.
Expand Down
21 changes: 6 additions & 15 deletions functional_tests.go
Expand Up @@ -2021,7 +2021,7 @@ func testObjectTaggingWithVersioning() {
successLogger(testName, function, args, startTime).Info()
}

// Test PutObject with custom checksums.
// Test put object with checksums.
func testPutObjectWithChecksums() {
// initialize logging params
startTime := time.Now()
Expand Down Expand Up @@ -2120,14 +2120,15 @@ func testPutObjectWithChecksums() {
})
if err == nil {
if i == 0 && resp.ChecksumCRC32 == "" {
ignoredLog(testName, function, args, startTime, "Checksums does not appear to be supported by backend").Info()
ignoredLog(testName, function, args, startTime, "Checksums do not appear to be supported by backend").Info()
return
}
logError(testName, function, args, startTime, "", "PutObject failed", err)
return
}

// Set correct CRC.
h.Reset()
h.Write(b)
meta[test.header] = base64.StdEncoding.EncodeToString(h.Sum(nil))
reader.Close()
Expand Down Expand Up @@ -2169,7 +2170,6 @@ func testPutObjectWithChecksums() {
logError(testName, function, args, startTime, "", "Number of bytes returned by PutObject does not match GetObject, expected "+string(bufSize)+" got "+string(st.Size), err)
return
}

if err := r.Close(); err != nil {
logError(testName, function, args, startTime, "", "Object Close failed", err)
return
Expand All @@ -2192,22 +2192,13 @@ func testPutObjectWithChecksums() {
}

b, err = io.ReadAll(r)
// Discard the object
minio-trusted marked this conversation as resolved.
Show resolved Hide resolved
_, err = r.Stat()
if err != nil {
logError(testName, function, args, startTime, "", "Read failed", err)
return
}
st, err = r.Stat()
if err != nil {
logError(testName, function, args, startTime, "", "Stat failed", err)
logError(testName, function, args, startTime, "", "Discard object failed", err)
return
}

// Range requests should return empty checksums...
cmpChecksum(st.ChecksumSHA256, "")
cmpChecksum(st.ChecksumSHA1, "")
cmpChecksum(st.ChecksumCRC32, "")
cmpChecksum(st.ChecksumCRC32C, "")

delete(args, "range")
delete(args, "metadata")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/tags/tags_test.go
Expand Up @@ -39,7 +39,7 @@ func TestParseTags(t *testing.T) {
2,
},
{
" store forever =false&factory=true",
"_store_forever_=false&factory=true",
minio-trusted marked this conversation as resolved.
Show resolved Hide resolved
false,
2,
},
Expand Down