From 5f962e0465af8dd48d9783a91b1cbfc58d43237d Mon Sep 17 00:00:00 2001 From: Airat Arifullin Date: Tue, 5 Nov 2024 16:07:40 +0300 Subject: [PATCH] [#293] object: Fix payload size limiter * Sending empty chunks by `writeChunk` should not release new objects as this doesn't change `payloadSizeLimiter` internal state. * This also fixes the bug with patcher when an offset of a patch equals to `MaxSize` - `payloadSizeLimiter` releases object again although state is the same. This led to error because EC-encoder receieved empty payload and couldn't not process it. Signed-off-by: Airat Arifullin --- object/transformer/transformer.go | 2 +- object/transformer/transformer_test.go | 72 ++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/object/transformer/transformer.go b/object/transformer/transformer.go index 16c4066..f7e5cd3 100644 --- a/object/transformer/transformer.go +++ b/object/transformer/transformer.go @@ -261,7 +261,7 @@ func (s *payloadSizeLimiter) initializeLinking(parHdr *object.Object) { func (s *payloadSizeLimiter) writeChunk(ctx context.Context, chunk []byte) error { for { // statement is true if the previous write of bytes reached exactly the boundary. - if s.written > 0 && s.written%s.MaxSize == 0 { + if len(chunk) > 0 && s.written > 0 && s.written%s.MaxSize == 0 { if s.written == s.MaxSize { s.prepareFirstChild() } diff --git a/object/transformer/transformer_test.go b/object/transformer/transformer_test.go index a170835..319bf37 100644 --- a/object/transformer/transformer_test.go +++ b/object/transformer/transformer_test.go @@ -15,6 +15,78 @@ import ( "github.com/stretchr/testify/require" ) +func TestTransformerNonMonotonicWrites(t *testing.T) { + const maxSize = 16 + + tt := new(testTarget) + target, pk := newPayloadSizeLimiter(maxSize, 0, func() ObjectWriter { return tt }) + cnr := cidtest.ID() + hdr := newObject(cnr) + var owner user.ID + user.IDFromKey(&owner, pk.PrivateKey.PublicKey) + hdr.SetOwnerID(owner) + payload := make([]byte, 3*maxSize) + _, _ = rand.Read(payload) + + ctx := context.Background() + require.NoError(t, target.WriteHeader(ctx, hdr)) + + lessThanMaxSizePayload := make([]byte, maxSize/2) + _, _ = rand.Read(lessThanMaxSizePayload) + + _, err := target.Write(ctx, lessThanMaxSizePayload) + require.NoError(t, err) + + moreThanMaxSizePayload := make([]byte, maxSize*1.5) + _, _ = rand.Read(moreThanMaxSizePayload) + + _, err = target.Write(ctx, moreThanMaxSizePayload) + require.NoError(t, err) + + _, err = target.Close(ctx) + require.NoError(t, err) + + require.Equal(t, 3, len(tt.objects)) + require.Equal(t, append(lessThanMaxSizePayload, moreThanMaxSizePayload[:maxSize-len(lessThanMaxSizePayload)]...), tt.objects[0].Payload()) + require.Equal(t, moreThanMaxSizePayload[maxSize-len(lessThanMaxSizePayload):], tt.objects[1].Payload()) + require.Equal(t, 2, len(tt.objects[2].Children())) +} + +func TestTransformerNonEffectiveWrites(t *testing.T) { + const maxSize = 16 + + tt := new(testTarget) + target, pk := newPayloadSizeLimiter(maxSize, 0, func() ObjectWriter { return tt }) + cnr := cidtest.ID() + hdr := newObject(cnr) + var owner user.ID + user.IDFromKey(&owner, pk.PrivateKey.PublicKey) + hdr.SetOwnerID(owner) + payload := make([]byte, 3*maxSize) + _, _ = rand.Read(payload) + + ctx := context.Background() + require.NoError(t, target.WriteHeader(ctx, hdr)) + + _, err := target.Write(ctx, payload) + require.NoError(t, err) + + for range 5 { + // run onto unchanged target state + _, err = target.Write(ctx, []byte{}) + require.NoError(t, err) + } + + _, err = target.Close(ctx) + require.NoError(t, err) + + require.Equal(t, 4, len(tt.objects)) + require.Equal(t, payload[:maxSize], tt.objects[0].Payload()) + require.Equal(t, payload[maxSize:2*maxSize], tt.objects[1].Payload()) + require.Equal(t, payload[2*maxSize:], tt.objects[2].Payload()) + require.Equal(t, 3, len(tt.objects[3].Children())) +} + func TestTransformer(t *testing.T) { const maxSize = 100