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

services/object: Allow zero payload ranges #3171

Merged
merged 4 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Changelog for NeoFS Node
- New network map support solving the limit of ~320 nodes per network
- Calculation of VUB for zero hash (#3134)
- More efficient block header subscription is used now instead of block-based one (#3163)
- `ObjectService.GetRange(Hash)` ops now handle zero ranges as full payload (#3071)

### Removed
- Drop creating new eacl tables with public keys (#3096)
Expand Down
8 changes: 4 additions & 4 deletions cmd/neofs-cli/modules/object/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,10 @@
}

if length == 0 {
return nil, fmt.Errorf("invalid '%s' range: zero length", vs[i])
}

if offset+length <= offset {
if offset != 0 {
return nil, fmt.Errorf("invalid '%s' range: zero length with non-zero offset", vs[i])
}
} else if offset+length <= offset {

Check warning on line 208 in cmd/neofs-cli/modules/object/range.go

View check run for this annotation

Codecov / codecov/patch

cmd/neofs-cli/modules/object/range.go#L205-L208

Added lines #L205 - L208 were not covered by tests
return nil, fmt.Errorf("invalid '%s' range: uint64 overflow", vs[i])
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ require (
github.com/nspcc-dev/neo-go v0.108.0
github.com/nspcc-dev/neofs-api-go/v2 v2.14.1-0.20240827150555-5ce597aa14ea
github.com/nspcc-dev/neofs-contract v0.21.0
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.12.0.20250213175709-2dc274a1721b
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.12.0.20250224145523-bf4a81ef7d79
github.com/nspcc-dev/tzhash v1.8.2
github.com/olekukonko/tablewriter v0.0.5
github.com/panjf2000/ants/v2 v2.9.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ github.com/nspcc-dev/neofs-api-go/v2 v2.14.1-0.20240827150555-5ce597aa14ea h1:mK
github.com/nspcc-dev/neofs-api-go/v2 v2.14.1-0.20240827150555-5ce597aa14ea/go.mod h1:YzhD4EZmC9Z/PNyd7ysC7WXgIgURc9uCG1UWDeV027Y=
github.com/nspcc-dev/neofs-contract v0.21.0 h1:dJkrZr7C8ngMiShwdUl27nbLCGez1KD7W1l7RRXFoN8=
github.com/nspcc-dev/neofs-contract v0.21.0/go.mod h1:TKR2DJiZCdzq9CPljI101c9S3uK5CsVK5WQrX4C+Y7A=
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.12.0.20250213175709-2dc274a1721b h1:xW6QTdAZRnED/2gwlD493c5ILdCtuQvXeeuICSlLt1E=
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.12.0.20250213175709-2dc274a1721b/go.mod h1:Hd0zRo7mfGDf/S+yGPnRi9eWipxJNhec2Oik1w7lwQo=
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.12.0.20250224145523-bf4a81ef7d79 h1:iJYbi1e7rx1Ga6fjcwH4bQMjuR/cD0VGeTM8wmqmjuM=
github.com/nspcc-dev/neofs-sdk-go v1.0.0-rc.12.0.20250224145523-bf4a81ef7d79/go.mod h1:Hd0zRo7mfGDf/S+yGPnRi9eWipxJNhec2Oik1w7lwQo=
github.com/nspcc-dev/rfc6979 v0.2.3 h1:QNVykGZ3XjFwM/88rGfV3oj4rKNBy+nYI6jM7q19hDI=
github.com/nspcc-dev/rfc6979 v0.2.3/go.mod h1:q3sCL1Ed7homjqYK8KmFSzEmm+7Ngyo7PePbZanhaDE=
github.com/nspcc-dev/tzhash v1.8.2 h1:ebRCbPoEuoqrhC6sSZmrT/jI3h1SzCWakxxV6gp5QAg=
Expand Down
10 changes: 8 additions & 2 deletions pkg/local_object_storage/blobstor/fstree/fstree.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,15 @@ func (t *FSTree) GetRange(addr oid.Address, from uint64, length uint64) ([]byte,
}

payload := obj.Payload()
to := from + length
pLen := uint64(len(payload))
var to uint64
if length != 0 {
to = from + length
} else {
to = pLen
}

if pLen := uint64(len(payload)); to < from || pLen < from || pLen < to {
if to < from || pLen < from || pLen < to {
return nil, logicerr.Wrap(apistatus.ObjectOutOfRange{})
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/local_object_storage/blobstor/get_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import (
// GetRange reads object payload data from b.
// If the descriptor is present, only one sub-storage is tried,
// Otherwise, each sub-storage is tried in order.
//
// Zero length is interpreted as requiring full object length independent of the
// offset.
func (b *BlobStor) GetRange(addr oid.Address, offset uint64, length uint64, storageID []byte) ([]byte, error) {
b.modeMtx.RLock()
defer b.modeMtx.RUnlock()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,18 @@ func TestGetRange(t *testing.T, cons Constructor, minSize, maxSize uint64) {
_, err := s.GetRange(objects[0].addr, 10, math.MaxUint64-2)
require.ErrorAs(t, err, new(apistatus.ObjectOutOfRange))
})

t.Run("zero range", func(t *testing.T) {
for i := range objects {
res, err := s.GetRange(objects[i].addr, 0, 0)
require.NoError(t, err)
pld := objects[i].obj.Payload()
require.Equal(t, pld, res)
if off := len(pld) / 2; off > 0 {
res, err = s.GetRange(objects[i].addr, uint64(off), 0)
require.NoError(t, err)
require.Equal(t, pld[off:], res)
}
}
})
}
10 changes: 8 additions & 2 deletions pkg/local_object_storage/blobstor/peapod/peapod.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,15 @@ func (x *Peapod) GetRange(addr oid.Address, from uint64, length uint64) ([]byte,
}

payload := obj.Payload()
to := from + length
pLen := uint64(len(payload))
var to uint64
if length != 0 {
to = from + length
} else {
to = pLen
}

if pLen := uint64(len(payload)); to < from || pLen < from || pLen < to {
if to < from || pLen < from || pLen < to {
return nil, logicerr.Wrap(apistatus.ObjectOutOfRange{})
}

Expand Down
13 changes: 11 additions & 2 deletions pkg/local_object_storage/shard/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
// GetRange reads part of an object from shard. If skipMeta is specified
// data will be fetched directly from the blobstor, bypassing metabase.
//
// Zero length is interpreted as requiring full object length independent of the
// offset.
//
// Returns any error encountered that
// did not allow to completely read the object part.
//
Expand Down Expand Up @@ -46,9 +49,15 @@
}

payload := o.Payload()
pLen := uint64(len(payload))
from := offset
to := from + length
if pLen := uint64(len(payload)); to < from || pLen < from || pLen < to {
var to uint64
if length != 0 {
to = from + length
} else {
to = pLen
}

Check warning on line 59 in pkg/local_object_storage/shard/range.go

View check run for this annotation

Codecov / codecov/patch

pkg/local_object_storage/shard/range.go#L58-L59

Added lines #L58 - L59 were not covered by tests
if to < from || pLen < from || pLen < to {
return logicerr.Wrap(apistatus.ObjectOutOfRange{})
}

Expand Down
9 changes: 8 additions & 1 deletion pkg/local_object_storage/shard/range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,15 @@ func testShardGetRange(t *testing.T, hasWriteCache bool) {
if tc.hasErr {
require.ErrorAs(t, err, &apistatus.ObjectOutOfRange{})
} else {
ln := tc.rng.GetLength()
var to uint64
if ln != 0 {
to = tc.rng.GetOffset() + ln
} else {
to = uint64(len(obj.Payload()))
}
require.Equal(t,
payload[tc.rng.GetOffset():tc.rng.GetOffset()+tc.rng.GetLength()],
payload[tc.rng.GetOffset():to],
res.Payload())
}
})
Expand Down
7 changes: 5 additions & 2 deletions pkg/services/object/get/assemble.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,11 @@
if rng := exec.ctxRange(); rng != nil {
seekOff := rng.GetOffset()
seekLen := rng.GetLength()
seekTo := seekOff + seekLen
parSize := par.PayloadSize()
if seekLen == 0 {
seekLen = parSize
}

Check warning on line 117 in pkg/services/object/get/assemble.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/object/get/assemble.go#L116-L117

Added lines #L116 - L117 were not covered by tests
seekTo := seekOff + seekLen

if seekTo < seekOff || parSize < seekOff || parSize < seekTo {
var errOutOfRange apistatus.ObjectOutOfRange
Expand All @@ -138,7 +141,7 @@
}

payload = child.Payload()[from:to]
rng.SetLength(rng.GetLength() - to + from)
rng.SetLength(seekLen - to + from)
} else {
payload = child.Payload()
}
Expand Down
16 changes: 12 additions & 4 deletions pkg/services/object/get/assembly_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
}

exec.collectedObject = lastObj.Parent()
if r := exec.ctxRange(); r != nil && r.GetLength() == 0 {
r.SetLength(exec.collectedObject.PayloadSize())
}

Check warning on line 45 in pkg/services/object/get/assembly_v2.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/object/get/assembly_v2.go#L43-L45

Added lines #L43 - L45 were not covered by tests

// copied from V1, and it has the same problems as V1;
// see it for comments and optimization suggestions
Expand All @@ -58,6 +61,10 @@
}

exec.collectedObject = linkObj.Parent()
rng := exec.ctxRange()
if rng != nil && rng.GetLength() == 0 {
rng.SetLength(exec.collectedObject.PayloadSize())
}

Check warning on line 67 in pkg/services/object/get/assembly_v2.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/object/get/assembly_v2.go#L64-L67

Added lines #L64 - L67 were not covered by tests

var link objectSDK.Link
err := linkObj.ReadLink(&link)
Expand All @@ -66,7 +73,7 @@
return false
}

if exec.ctxRange() == nil {
if rng == nil {

Check warning on line 76 in pkg/services/object/get/assembly_v2.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/object/get/assembly_v2.go#L76

Added line #L76 was not covered by tests
// GET case

if exec.writeCollectedHeader() {
Expand All @@ -81,12 +88,13 @@
}

// RANGE case

rng := exec.ctxRange()
seekOff := rng.GetOffset()
seekLen := rng.GetLength()
seekTo := seekOff + seekLen
parSize := linkObj.Parent().PayloadSize()
if seekLen == 0 {
seekLen = parSize
}
seekTo := seekOff + seekLen

Check warning on line 97 in pkg/services/object/get/assembly_v2.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/object/get/assembly_v2.go#L94-L97

Added lines #L94 - L97 were not covered by tests

if seekTo < seekOff || parSize < seekOff || parSize < seekTo {
var errOutOfRange apistatus.ObjectOutOfRange
Expand Down
25 changes: 0 additions & 25 deletions pkg/services/object/get/prm.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package getsvc
import (
"context"
"crypto/ecdsa"
"errors"
"hash"

coreclient "github.com/nspcc-dev/neofs-node/pkg/core/client"
Expand All @@ -25,30 +24,6 @@ type RangePrm struct {
rng *object.Range
}

var (
errRangeZeroLength = errors.New("zero range length")
errRangeOverflow = errors.New("range overflow")
)

// Validate pre-validates `OBJECTRANGE` request's parameters content
// without access to the requested object's payload.
func (p RangePrm) Validate() error {
if p.rng != nil {
off := p.rng.GetOffset()
l := p.rng.GetLength()

if l == 0 {
return errRangeZeroLength
}

if off+l <= off {
return errRangeOverflow
}
}

return nil
}

// RangeHashPrm groups parameters of GetRange service call.
type RangeHashPrm struct {
commonPrm
Expand Down
6 changes: 5 additions & 1 deletion pkg/services/object/get/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,11 @@

payload := obj.Payload()
from := rng.GetOffset()
to := from + rng.GetLength()
ln := rng.GetLength()
if ln == 0 {
ln = obj.PayloadSize()
}
to := from + ln

Check warning on line 159 in pkg/services/object/get/util.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/object/get/util.go#L155-L159

Added lines #L155 - L159 were not covered by tests

if pLen := uint64(len(payload)); to < from || pLen < from || pLen < to {
return nil, new(apistatus.ObjectOutOfRange)
Expand Down
7 changes: 4 additions & 3 deletions pkg/services/object/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1378,9 +1378,10 @@

rln := body.Range.GetLength()
if rln == 0 { // includes nil range
return getsvc.RangePrm{}, errors.New("zero range length")
}
if body.Range.Offset+rln <= body.Range.Offset {
if body.Range.Offset != 0 {
return getsvc.RangePrm{}, errors.New("zero range length")
} // else whole payload
} else if body.Range.Offset+rln <= body.Range.Offset {

Check warning on line 1384 in pkg/services/object/server.go

View check run for this annotation

Codecov / codecov/patch

pkg/services/object/server.go#L1381-L1384

Added lines #L1381 - L1384 were not covered by tests
return getsvc.RangePrm{}, errors.New("range overflow")
}

Expand Down
Loading