Skip to content

Commit 2b47810

Browse files
cdaguerredarkweak
andauthored
fix: close LZ4 writer before reading compressed buffer (#41) (#44)
* fix: close LZ4 writer before reading compressed buffer (#41) The LZ4 writer was being closed with defer, but compressed.Bytes() was called before the defer executed. This caused truncated data since Close() flushes the final compressed blocks. Applied fix to all 9 storage providers and added regression tests for providers without external dependencies (simplefs, otter, badger, nuts). * fix(ci): golangci-lint --------- Co-authored-by: darkweak <darkweak@protonmail.com>
1 parent 0bacedf commit 2b47810

File tree

14 files changed

+263
-27
lines changed

14 files changed

+263
-27
lines changed

badger/badger.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,16 +234,20 @@ func (provider *Badger) SetMultiLevel(baseKey, variedKey string, value []byte, v
234234
compressed := new(bytes.Buffer)
235235
writer := lz4.NewWriter(compressed)
236236

237-
defer func() {
237+
if _, err = writer.ReadFrom(bytes.NewReader(value)); err != nil {
238238
_ = writer.Close()
239-
}()
240239

241-
if _, err = writer.ReadFrom(bytes.NewReader(value)); err != nil {
242240
provider.logger.Errorf("Impossible to compress the key %s into Badger, %v", variedKey, err)
243241

244242
return err
245243
}
246244

245+
if err = writer.Close(); err != nil {
246+
provider.logger.Errorf("Impossible to close the compressor for key %s into Badger, %v", variedKey, err)
247+
248+
return err
249+
}
250+
247251
err = btx.SetEntry(badger.NewEntry([]byte(variedKey), compressed.Bytes()).WithTTL(duration + provider.stale))
248252
if err != nil {
249253
provider.logger.Errorf("Impossible to set the key %s into Badger, %v", variedKey, err)

badger/badger_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33
package badger_test
44

55
import (
6+
"bytes"
7+
"net/http"
68
"testing"
79
"time"
810

911
"github.com/darkweak/storages/badger"
1012
"github.com/darkweak/storages/core"
13+
"github.com/pierrec/lz4/v4"
1114
"go.uber.org/zap"
1215
)
1316

@@ -132,3 +135,50 @@ func TestBadger_Init(t *testing.T) {
132135
t.Error("Impossible to init Badger provider")
133136
}
134137
}
138+
139+
// TestBadger_SetMultiLevel_LargeValue tests that large values are not truncated
140+
// when stored and retrieved via SetMultiLevel. This reproduces issue #41.
141+
// See: https://github.com/darkweak/storages/issues/41
142+
func TestBadger_SetMultiLevel_LargeValue(t *testing.T) {
143+
client, err := getBadgerInstance()
144+
if err != nil {
145+
t.Fatalf("Failed to create badger instance: %v", err)
146+
}
147+
148+
_ = client.Init()
149+
150+
// Create a large value (5MB) to simulate the issue where large cached
151+
// responses were being truncated
152+
largeSize := 5 * 1024 * 1024
153+
154+
largeValue := make([]byte, largeSize)
155+
for i := range largeValue {
156+
largeValue[i] = byte(i % 256)
157+
}
158+
159+
key := "large_value_test"
160+
161+
err = client.SetMultiLevel(key, key, largeValue, http.Header{}, "", time.Minute, key)
162+
if err != nil {
163+
t.Fatalf("Failed to set large value: %v", err)
164+
}
165+
166+
// Retrieve and decompress
167+
compressed := client.Get(key)
168+
if compressed == nil {
169+
t.Fatal("Retrieved value is nil")
170+
}
171+
172+
reader := lz4.NewReader(bytes.NewBuffer(compressed))
173+
174+
decompressed := new(bytes.Buffer)
175+
if _, err := reader.WriteTo(decompressed); err != nil {
176+
t.Fatalf("Failed to decompress: %v", err)
177+
}
178+
179+
retrieved := decompressed.Bytes()
180+
if len(retrieved) != len(largeValue) {
181+
t.Errorf("Data truncation: expected %d bytes, got %d bytes (%.2f%%)",
182+
len(largeValue), len(retrieved), float64(len(retrieved))/float64(len(largeValue))*100)
183+
}
184+
}

etcd/etcd.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,16 +219,20 @@ func (provider *Etcd) SetMultiLevel(baseKey, variedKey string, value []byte, var
219219
compressed := new(bytes.Buffer)
220220
writer := lz4.NewWriter(compressed)
221221

222-
defer func() {
222+
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
223223
_ = writer.Close()
224-
}()
225224

226-
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
227225
provider.logger.Errorf("Impossible to compress the key %s into Etcd, %v", variedKey, err)
228226

229227
return err
230228
}
231229

230+
if err := writer.Close(); err != nil {
231+
provider.logger.Errorf("Impossible to close the compressor for key %s into Etcd, %v", variedKey, err)
232+
233+
return err
234+
}
235+
232236
rs, err := provider.Grant(context.TODO(), int64(duration.Seconds()))
233237
if err == nil {
234238
_, err = provider.Put(provider.ctx, variedKey, compressed.String(), clientv3.WithLease(rs.ID))

go-redis/go-redis.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,16 +201,20 @@ func (provider *Redis) SetMultiLevel(baseKey, variedKey string, value []byte, va
201201
compressed := new(bytes.Buffer)
202202
writer := lz4.NewWriter(compressed)
203203

204-
defer func() {
204+
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
205205
_ = writer.Close()
206-
}()
207206

208-
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
209207
provider.logger.Errorf("Impossible to compress the key %s into Redis, %v", variedKey, err)
210208

211209
return err
212210
}
213211

212+
if err := writer.Close(); err != nil {
213+
provider.logger.Errorf("Impossible to close the compressor for key %s into Redis, %v", variedKey, err)
214+
215+
return err
216+
}
217+
214218
if err := provider.Set(provider.hashtags+variedKey, compressed.Bytes(), duration); err != nil {
215219
provider.logger.Errorf("Impossible to set value into Redis, %v", err)
216220

nats/nats.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,16 +267,20 @@ func (provider *Nats) SetMultiLevel(baseKey, variedKey string, value []byte, var
267267
compressed := new(bytes.Buffer)
268268
writer := lz4.NewWriter(compressed)
269269

270-
defer func() {
270+
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
271271
_ = writer.Close()
272-
}()
273272

274-
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
275273
provider.logger.Errorf("Impossible to compress the key %s into Nats: %v", variedKey, err)
276274

277275
return err
278276
}
279277

278+
if err := writer.Close(); err != nil {
279+
provider.logger.Errorf("Impossible to close the compressor for key %s into Nats, %v", variedKey, err)
280+
281+
return err
282+
}
283+
280284
property := item{
281285
invalidAt: now.Add(duration + provider.stale),
282286
value: compressed.Bytes(),

nuts/nuts.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,16 +298,20 @@ func (provider *Nuts) SetMultiLevel(baseKey, variedKey string, value []byte, var
298298
compressed := new(bytes.Buffer)
299299
writer := lz4.NewWriter(compressed)
300300

301-
defer func() {
301+
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
302302
_ = writer.Close()
303-
}()
304303

305-
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
306304
provider.logger.Errorf("Impossible to compress the key %s into Nuts, %v", variedKey, err)
307305

308306
return err
309307
}
310308

309+
if err := writer.Close(); err != nil {
310+
provider.logger.Errorf("Impossible to close the compressor for key %s into Nuts, %v", variedKey, err)
311+
312+
return err
313+
}
314+
311315
_ = provider.Update(func(tx *nutsdb.Tx) error {
312316
return tx.NewBucket(nutsdb.DataStructureBTree, bucket)
313317
})

nuts/nuts_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package nuts_test
22

33
import (
4+
"bytes"
5+
"net/http"
46
"testing"
57
"time"
68

79
"github.com/darkweak/storages/core"
810
"github.com/darkweak/storages/nuts"
11+
"github.com/pierrec/lz4/v4"
912
"go.uber.org/zap"
1013
)
1114

@@ -107,3 +110,50 @@ func TestNuts_Init(t *testing.T) {
107110
t.Error("Impossible to init Nuts provider")
108111
}
109112
}
113+
114+
// TestNuts_SetMultiLevel_LargeValue tests that large values are not truncated
115+
// when stored and retrieved via SetMultiLevel. This reproduces issue #41.
116+
// See: https://github.com/darkweak/storages/issues/41
117+
func TestNuts_SetMultiLevel_LargeValue(t *testing.T) {
118+
client, err := getNutsInstance()
119+
if err != nil {
120+
t.Fatalf("Failed to create nuts instance: %v", err)
121+
}
122+
123+
_ = client.Init()
124+
125+
// Create a large value (5MB) to simulate the issue where large cached
126+
// responses were being truncated
127+
largeSize := 5 * 1024 * 1024
128+
129+
largeValue := make([]byte, largeSize)
130+
for i := range largeValue {
131+
largeValue[i] = byte(i % 256)
132+
}
133+
134+
key := "large_value_test"
135+
136+
err = client.SetMultiLevel(key, key, largeValue, http.Header{}, "", time.Minute, key)
137+
if err != nil {
138+
t.Fatalf("Failed to set large value: %v", err)
139+
}
140+
141+
// Retrieve and decompress
142+
compressed := client.Get(key)
143+
if compressed == nil {
144+
t.Fatal("Retrieved value is nil")
145+
}
146+
147+
reader := lz4.NewReader(bytes.NewBuffer(compressed))
148+
149+
decompressed := new(bytes.Buffer)
150+
if _, err := reader.WriteTo(decompressed); err != nil {
151+
t.Fatalf("Failed to decompress: %v", err)
152+
}
153+
154+
retrieved := decompressed.Bytes()
155+
if len(retrieved) != len(largeValue) {
156+
t.Errorf("Data truncation: expected %d bytes, got %d bytes (%.2f%%)",
157+
len(largeValue), len(retrieved), float64(len(retrieved))/float64(len(largeValue))*100)
158+
}
159+
}

olric/olric.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -261,16 +261,20 @@ func (provider *Olric) SetMultiLevel(baseKey, variedKey string, value []byte, va
261261
compressed := new(bytes.Buffer)
262262
writer := lz4.NewWriter(compressed)
263263

264-
defer func() {
264+
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
265265
_ = writer.Close()
266-
}()
267266

268-
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
269267
provider.logger.Errorf("Impossible to compress the key %s into Olric, %v", variedKey, err)
270268

271269
return err
272270
}
273271

272+
if err := writer.Close(); err != nil {
273+
provider.logger.Errorf("Impossible to close the compressor for key %s into Olric, %v", variedKey, err)
274+
275+
return err
276+
}
277+
274278
if err := dmap.Put(context.Background(), variedKey, compressed.Bytes(), olric.EX(duration)); err != nil {
275279
provider.logger.Errorf("Impossible to set value into Olric, %v", err)
276280

otter/otter.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,16 +149,20 @@ func (provider *Otter) SetMultiLevel(baseKey, variedKey string, value []byte, va
149149
compressed := new(bytes.Buffer)
150150
writer := lz4.NewWriter(compressed)
151151

152-
defer func() {
152+
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
153153
_ = writer.Close()
154-
}()
155154

156-
if _, err := writer.ReadFrom(bytes.NewReader(value)); err != nil {
157155
provider.logger.Errorf("Impossible to compress the key %s into Otter, %v", variedKey, err)
158156

159157
return err
160158
}
161159

160+
if err := writer.Close(); err != nil {
161+
provider.logger.Errorf("Impossible to close the compressor for key %s into Otter, %v", variedKey, err)
162+
163+
return err
164+
}
165+
162166
inserted := provider.cache.Set(variedKey, compressed.Bytes(), duration)
163167
if !inserted {
164168
provider.logger.Errorf("Impossible to set value into Otter, too large for the cost function")

otter/otter_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package otter_test
22

33
import (
4+
"bytes"
5+
"net/http"
46
"testing"
57
"time"
68

79
"github.com/darkweak/storages/core"
810
"github.com/darkweak/storages/otter"
11+
"github.com/pierrec/lz4/v4"
912
"go.uber.org/zap"
1013
)
1114

@@ -133,3 +136,50 @@ func TestOtter_Init(t *testing.T) {
133136
t.Error("Impossible to init Otter provider")
134137
}
135138
}
139+
140+
// TestOtter_SetMultiLevel_LargeValue tests that large values are not truncated
141+
// when stored and retrieved via SetMultiLevel. This reproduces issue #41.
142+
// See: https://github.com/darkweak/storages/issues/41
143+
func TestOtter_SetMultiLevel_LargeValue(t *testing.T) {
144+
client, err := getOtterInstance()
145+
if err != nil {
146+
t.Fatalf("Failed to create otter instance: %v", err)
147+
}
148+
149+
_ = client.Init()
150+
151+
// Create a large value (5MB) to simulate the issue where large cached
152+
// responses were being truncated
153+
largeSize := 5 * 1024 * 1024
154+
155+
largeValue := make([]byte, largeSize)
156+
for i := range largeValue {
157+
largeValue[i] = byte(i % 256)
158+
}
159+
160+
key := "large_value_test"
161+
162+
err = client.SetMultiLevel(key, key, largeValue, http.Header{}, "", time.Minute, key)
163+
if err != nil {
164+
t.Fatalf("Failed to set large value: %v", err)
165+
}
166+
167+
// Retrieve and decompress
168+
compressed := client.Get(key)
169+
if compressed == nil {
170+
t.Fatal("Retrieved value is nil")
171+
}
172+
173+
reader := lz4.NewReader(bytes.NewBuffer(compressed))
174+
175+
decompressed := new(bytes.Buffer)
176+
if _, err := reader.WriteTo(decompressed); err != nil {
177+
t.Fatalf("Failed to decompress: %v", err)
178+
}
179+
180+
retrieved := decompressed.Bytes()
181+
if len(retrieved) != len(largeValue) {
182+
t.Errorf("Data truncation: expected %d bytes, got %d bytes (%.2f%%)",
183+
len(largeValue), len(retrieved), float64(len(retrieved))/float64(len(largeValue))*100)
184+
}
185+
}

0 commit comments

Comments
 (0)