From e1fbbbd69184f6d883593de7fd9c763fb36df582 Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Mon, 14 Jul 2025 13:35:12 +0300 Subject: [PATCH 1/6] object/slicer: Test LINK object payload is under the limit All objects must be under payload len limit, linker is no exception. Moreover, f5ffd70ccc3ec05964c54adaabbd956fe711c3ab added check for this. Signed-off-by: Leonard Lyubich --- object/slicer/slicer_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/object/slicer/slicer_test.go b/object/slicer/slicer_test.go index 36ebf8c6..dd48dfbc 100644 --- a/object/slicer/slicer_test.go +++ b/object/slicer/slicer_test.go @@ -488,10 +488,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 From 5f50907b5e455cd7393b029ecda429760cde1309 Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Mon, 14 Jul 2025 13:57:43 +0300 Subject: [PATCH 2/6] object/slicer: Clarify parameterization in test functions Signed-off-by: Leonard Lyubich --- object/slicer/slicer_test.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/object/slicer/slicer_test.go b/object/slicer/slicer_test.go index dd48dfbc..a6b2536e 100644 --- a/object/slicer/slicer_test.go +++ b/object/slicer/slicer_test.go @@ -53,18 +53,18 @@ func TestSliceDataIntoObjects(t *testing.T) { 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) + testSlicer(t, defaultLimit-1, defaultLimit) + testSlicer(t, defaultLimit, defaultLimit) }) t.Run("multiple size", func(t *testing.T) { - testSlicer(t, 3*defaultLimit, 0) - testSlicer(t, 3*defaultLimit+1, 0) + testSlicer(t, 3*defaultLimit, defaultLimit) + testSlicer(t, 3*defaultLimit+1, defaultLimit) }) }) t.Run("no payload", func(t *testing.T) { - testSlicer(t, 0, 0) + testSlicer(t, 0, defaultLimit) testSlicer(t, 0, 1024) }) } @@ -206,11 +206,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() From 8cb9fa93b1250bbe47cd7eaf80cdbdb0d9b5e373 Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Mon, 14 Jul 2025 13:59:45 +0300 Subject: [PATCH 3/6] object/slicer: Fix input in split test One of splitting tests never actually done splitting before. Now it does. Signed-off-by: Leonard Lyubich --- object/slicer/slicer_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/object/slicer/slicer_test.go b/object/slicer/slicer_test.go index a6b2536e..be737836 100644 --- a/object/slicer/slicer_test.go +++ b/object/slicer/slicer_test.go @@ -37,17 +37,17 @@ import ( const defaultLimit = 1 << 20 func TestSliceDataIntoObjects(t *testing.T) { - const size = 1 << 10 + const limit = 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) + testSlicer(t, limit, limit) + testSlicer(t, limit, limit+1) }) t.Run("multiple size", func(t *testing.T) { - testSlicer(t, size, 3*size) - testSlicer(t, size, 3*size+1) + testSlicer(t, 3*limit, limit) + testSlicer(t, 3*limit+1, limit) }) }) From 57a50cf81945dcdc4b275500e3a66c88bcd6833d Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Mon, 14 Jul 2025 14:05:50 +0300 Subject: [PATCH 4/6] object/slicer: Deduplicate payload len-based tests In these tests, only relative payload len is important, not the absolute limit. Also, there is no "unknown limit" concept: the limit is always known. Signed-off-by: Leonard Lyubich --- object/slicer/slicer_test.go | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/object/slicer/slicer_test.go b/object/slicer/slicer_test.go index be737836..ace62cb0 100644 --- a/object/slicer/slicer_test.go +++ b/object/slicer/slicer_test.go @@ -34,37 +34,20 @@ import ( "github.com/stretchr/testify/require" ) -const defaultLimit = 1 << 20 - func TestSliceDataIntoObjects(t *testing.T) { const limit = 1 << 10 - t.Run("known limit", func(t *testing.T) { - t.Run("under limit", func(t *testing.T) { - testSlicer(t, limit, limit) - testSlicer(t, limit, limit+1) - }) - - t.Run("multiple size", func(t *testing.T) { - testSlicer(t, 3*limit, limit) - testSlicer(t, 3*limit+1, limit) - }) + t.Run("under limit", func(t *testing.T) { + testSlicer(t, limit, limit) + testSlicer(t, limit, limit+1) }) - t.Run("unknown limit", func(t *testing.T) { - t.Run("under limit", func(t *testing.T) { - testSlicer(t, defaultLimit-1, defaultLimit) - testSlicer(t, defaultLimit, defaultLimit) - }) - - t.Run("multiple size", func(t *testing.T) { - testSlicer(t, 3*defaultLimit, defaultLimit) - testSlicer(t, 3*defaultLimit+1, defaultLimit) - }) + t.Run("multiple size", func(t *testing.T) { + testSlicer(t, 3*limit, limit) + testSlicer(t, 3*limit+1, limit) }) t.Run("no payload", func(t *testing.T) { - testSlicer(t, 0, defaultLimit) testSlicer(t, 0, 1024) }) } From d9e2dcb4f098e43d6189d10f615e496406a69c76 Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Mon, 14 Jul 2025 14:10:19 +0300 Subject: [PATCH 5/6] object/slicer: Regroup len-based tests Signed-off-by: Leonard Lyubich --- object/slicer/slicer_test.go | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/object/slicer/slicer_test.go b/object/slicer/slicer_test.go index ace62cb0..4d4621c8 100644 --- a/object/slicer/slicer_test.go +++ b/object/slicer/slicer_test.go @@ -37,19 +37,20 @@ import ( func TestSliceDataIntoObjects(t *testing.T) { const limit = 1 << 10 - t.Run("under limit", func(t *testing.T) { - testSlicer(t, limit, limit) - testSlicer(t, limit, limit+1) - }) - - t.Run("multiple size", func(t *testing.T) { - testSlicer(t, 3*limit, limit) - testSlicer(t, 3*limit+1, limit) - }) - - t.Run("no payload", func(t *testing.T) { - testSlicer(t, 0, 1024) - }) + 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) + }) + } } func BenchmarkSliceDataIntoObjects(b *testing.B) { From 1211ce3e21e1df78943739b1c269a0cafaaaa78b Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Mon, 14 Jul 2025 15:15:30 +0300 Subject: [PATCH 6/6] object/slicer: Fix double payload in produced LINK objects Defect of e1a82c5e589cab20e2c52623a101565fa11a3c7f or 44a2cc4d565f5c1496aa7d97cd9985e825e796a7. Previously, slicer wrote resulting LINK object like this: 1. pass full object to `ObjectWriter.ObjectPutInit`; 2. write whole payload again to payload `io.Writer`. Thus, the output was an incorrect object due to double payload. Now LINK header is passed without payload. Signed-off-by: Leonard Lyubich --- object/slicer/slicer.go | 6 +++--- object/slicer/slicer_test.go | 25 ++++++++++++++----------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/object/slicer/slicer.go b/object/slicer/slicer.go index c27e6e03..952b2cdc 100644 --- a/object/slicer/slicer.go +++ b/object/slicer/slicer.go @@ -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} if uint64(len(payload)) > x.payloadSizeLimit { return fmt.Errorf("link's payload exceeds max available size: %d > %d", uint64(len(payload)), x.payloadSizeLimit) diff --git a/object/slicer/slicer_test.go b/object/slicer/slicer_test.go index 4d4621c8..ad78138d 100644 --- a/object/slicer/slicer_test.go +++ b/object/slicer/slicer_test.go @@ -509,7 +509,7 @@ type chainCollector struct { mPayloads map[oid.ID]payloadWithChecksum - children []object.MeasuredObject + link oid.ID } func newChainCollector(tb testing.TB) *chainCollector { @@ -556,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") @@ -628,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 } } @@ -725,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()