Skip to content

Fix LINK in slicer #725

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.

Sign up for GitHub

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

Merged
cthulhu-rider merged 6 commits into from
Jul 15, 2025
Merged

Fix LINK in slicer #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e1fbbbd
object/slicer: Test LINK object payload is under the limit
cthulhu-rider Jul 14, 2025
5f50907
object/slicer: Clarify parameterization in test functions
cthulhu-rider Jul 14, 2025
8cb9fa9
object/slicer: Fix input in split test
cthulhu-rider Jul 14, 2025
57a50cf
object/slicer: Deduplicate payload len-based tests
cthulhu-rider Jul 14, 2025
d9e2dcb
object/slicer: Regroup len-based tests
cthulhu-rider Jul 14, 2025
1211ce3
object/slicer: Fix double payload in produced LINK objects
cthulhu-rider Jul 14, 2025
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
6 changes: 3 additions & 3 deletions object/slicer/slicer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,15 +569,15 @@ func (x *PayloadWriter) _writeChild(ctx context.Context, meta dynamicObjectMetad
if x.withSplit && last {
var linkObj object.Link
linkObj.SetObjects(x.writtenChildren)
obj.WriteLink(linkObj)
obj.SetType(object.TypeLink)

obj.ResetPreviousID()
// we reuse already written object, we should reset these fields, to eval them one more time in writeInMemObject.
obj.ResetID()
obj.SetSignature(nil)

payload := obj.Payload()
payloadAsBuffers := [][]byte{obj.Payload()}
payload := linkObj.Marshal()
payloadAsBuffers := [][]byte{payload}
Comment thread
roman-khimov marked this conversation as resolved.

if uint64(len(payload)) > x.payloadSizeLimit {
return fmt.Errorf("link's payload exceeds max available size: %d > %d", uint64(len(payload)), x.payloadSizeLimit)
Expand Down
79 changes: 31 additions & 48 deletions object/slicer/slicer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,39 +34,23 @@ import (
"github.com/stretchr/testify/require"
)

const defaultLimit = 1 << 20

func TestSliceDataIntoObjects(t *testing.T) {
const size = 1 << 10

t.Run("known limit", func(t *testing.T) {
t.Run("under limit", func(t *testing.T) {
testSlicer(t, size, size)
testSlicer(t, size, size+1)
})
const limit = 1 << 10

t.Run("multiple size", func(t *testing.T) {
testSlicer(t, size, 3*size)
testSlicer(t, size, 3*size+1)
})
})

t.Run("unknown limit", func(t *testing.T) {
t.Run("under limit", func(t *testing.T) {
testSlicer(t, defaultLimit-1, 0)
testSlicer(t, defaultLimit, 0)
})

t.Run("multiple size", func(t *testing.T) {
testSlicer(t, 3*defaultLimit, 0)
testSlicer(t, 3*defaultLimit+1, 0)
for _, tc := range []struct {
name string
ln uint64
}{
{name: "no payload", ln: 0},
{name: "limit-1B", ln: limit - 1},
{name: "exactly limit", ln: limit},
{name: "limitX3", ln: limit * 3},
{name: "limitX3+1B", ln: limit*3 + 1},
} {
t.Run(tc.name, func(t *testing.T) {
testSlicer(t, tc.ln, limit)
})
})

t.Run("no payload", func(t *testing.T) {
testSlicer(t, 0, 0)
testSlicer(t, 0, 1024)
})
}
}

func BenchmarkSliceDataIntoObjects(b *testing.B) {
Expand Down Expand Up @@ -206,11 +190,7 @@ func randomInput(size, sizeLimit uint64) (input, slicer.Options) {
in.signer = user.NewAutoIDSigner(*key)
in.container = cidtest.ID()
in.currentEpoch = rand.Uint64()
if sizeLimit > 0 {
in.payloadLimit = sizeLimit
} else {
in.payloadLimit = defaultLimit
}
in.payloadLimit = sizeLimit
in.payload = testutil.RandByteSlice(size)
in.attributes = attrs
in.owner = usertest.ID()
Expand Down Expand Up @@ -488,10 +468,10 @@ func (x *writeSizeChecker) Close() error {

var testLink object.Link
require.NoError(x.tb, testLink.Unmarshal(payload), "link object's payload must be structured")
} else {
require.LessOrEqual(x.tb, x.processed, x.limit, "object payload must not overflow the limit")
}

require.LessOrEqual(x.tb, x.processed, x.limit, "object payload must not overflow the limit")

require.Equal(x.tb, x.processed, x.hdr.PayloadSize())

// deprecated things
Expand Down Expand Up @@ -529,7 +509,7 @@ type chainCollector struct {

mPayloads map[oid.ID]payloadWithChecksum

children []object.MeasuredObject
link oid.ID
}

func newChainCollector(tb testing.TB) *chainCollector {
Expand Down Expand Up @@ -576,6 +556,8 @@ func (x *chainCollector) handleOutgoingObject(headerOriginal object.Object, payl
var header object.Object
headerOriginal.CopyTo(&header)

require.Empty(x.tb, header.Payload(), "payload must be unset in header")

id := header.GetID()
require.False(x.tb, id.IsZero(), "all objects must have an ID")

Expand Down Expand Up @@ -648,14 +630,7 @@ func (x *chainCollector) handleOutgoingObject(headerOriginal object.Object, payl
var testLink object.Link
require.NoError(x.tb, header.ReadLink(&testLink))

children := testLink.Objects()
if len(children) > 0 {
if len(x.children) > 0 {
require.Equal(x.tb, x.children, children, "children list must be the same")
} else {
x.children = children
}
}
x.link = id
}
}

Expand Down Expand Up @@ -745,9 +720,17 @@ func (x *chainCollector) verify(in input, rootID oid.ID) {
rootObj.SetPayload(restoredPayload.Bytes())

if uint64(len(in.payload)) <= in.payloadLimit {
require.Empty(x.tb, x.children)
require.Zero(x.tb, x.link)
} else {
require.Equal(x.tb, x.children, restoredChain)
require.NotZero(x.tb, x.link)
p, ok := x.mPayloads[x.link]
require.True(x.tb, ok)
payload, err := io.ReadAll(p.r)
require.NoError(x.tb, err)

var l object.Link
require.NoError(x.tb, l.Unmarshal(payload))
require.Equal(x.tb, l.Objects(), restoredChain)
}

id := rootObj.GetID()
Expand Down
Loading