Skip to content

Improve decode safety and allocation efficiency - #32

Merged
kelindar merged 8 commits into
masterfrom
optimize3
Aug 9, 2026
Merged

Improve decode safety and allocation efficiency#32
kelindar merged 8 commits into
masterfrom
optimize3

Conversation

@kelindar

Copy link
Copy Markdown
Owner

What changed

  • Simplified and optimized the main reflection, slice, map, no-copy, unsafe, and sorted codec paths to reduce redundant work and allocations.
  • Added safer length/availability checks for malformed and truncated input, including streaming decoders and custom codecs.
  • Removed unnecessary intermediate copies where ownership is clear.
  • Added repro coverage for typed-nil values, zero-wire codecs, oversized lengths, short reads, map bounds, and custom codec paths.

Correctness fixes

  • Capped arena-backed map byte-slice capacities so appending to one value cannot overwrite a sibling value.
  • Prevented union decoding from retaining or reusing a temporary arena across subsequent decodes.
  • Preserved reusable storage where safe while avoiding borrowed-buffer lifetime and capacity hazards.

Validation

  • go test ./...
  • go-testlint
  • go-switch review of all reported candidates
  • git diff --check

@github-actions

github-actionsBot commented Aug 9, 2026

Copy link
Copy Markdown
95.0%

passed

███████████████████░95.0% on changed lines

1462 lines changed 59 uncovered 12 files

 📂 12 files changed
FileCoverageUncovered Lines
🟢nocopy/types.go90.8%271, 272, 318, 319, 348, 349, 352, 353, 386, 387 (+8 more)

| 🟢 | unsafe/types.go | 93.3% | 102, 103, 127, 128 |

| 🟢 | codecs.go | 95.3% | 82, 83, 131, 132, 194, 195, 253, 254, 338, 339 (+25 more) |

| 🟢 | reader.go | 95.5% | 204, 205 |

| 🟢 | sorted/counters.go | 100.0% | — |

| 🟢 | sorted/timeseries.go | 100.0% | — |

| 🟢 | scanner.go | 100.0% | — |

| 🟢 | sorted/types.go | 100.0% | — |

| 🟢 | encoder.go | 100.0% | — |

| 🟢 | decoder.go | 100.0% | — |

| 🟢 | union.go | 100.0% | — |

| 🟢 | sorted/codecs.go | 100.0% | — |

 📋 Full diff-cover report

Diff Coverage

Diff: origin/master...HEAD, staged and unstaged changes

  • codecs.go (95.3%): Missing lines 82-83,131-132,194-195,253-254,338-339,381-382,390-391,499,506,514-515,654-655,1051-1052,1060-1061,1076-1077,1085-1086,1184-1185,1269,1283,1314,1343-1344
  • decoder.go (100%)
  • encoder.go (100%)
  • nocopy/types.go (90.8%): Missing lines 271-272,318-319,348-349,352-353,386-387,407-408,412-413,444-445,470-471
  • reader.go (95.5%): Missing lines 204-205
  • scanner.go (100%)
  • sorted/codecs.go (100%)
  • sorted/counters.go (100%)
  • sorted/timeseries.go (100%)
  • sorted/types.go (100%)
  • union.go (100%)
  • unsafe/types.go (93.3%): Missing lines 102-103,127-128

Summary

  • Total: 1462 lines
  • Missing: 59 lines
  • Coverage: 95%

codecs.go

 78 }
79 if codec, ok := c.elemCodec.(*reflectStructCodec); ok {
80 for i := range l {
81 if err = codec.EncodeTo(e, rv.Index(i)); err != nil {
! 82 return
! 83 }
84 }
85 return
86 }
87 for i := range l {

 127 if err = d.ensureElements(n, minBytes); err != nil {
128 return
129 }
130 if err = resizeSliceChecked(rv, n); err != nil {
! 131 return
! 132 }
133 }
134 if isStruct {
135 if wireless {
136 return nil

 190 if err = d.ensureAvailable(n); err != nil {
191 return err
192 }
193 if err = resizeSliceChecked(rv, n); err != nil {
! 194 return err
! 195 }
196 }
197 for i := 0; i < n; i++ {
198 if stream {
199 appendSliceElement(rv)

 249 if err = d.ensureAvailable(n); err != nil {
250 return
251 }
252 if err = resizeSliceChecked(rv, n); err != nil {
! 253 return
! 254 }
255 if l > 0 {
256 _, err = d.Read(rv.Bytes())
257 }
258 }

 334 if err = d.ensureAvailable(n); err != nil {
335 return
336 }
337 if err = resizeSliceChecked(rv, n); err != nil {
! 338 return
! 339 }
340 }
341 if c.array {
342 if err = d.ensureAvailable(n); err != nil {
343 return

 377 if readErr != nil {
378 return readErr
379 }
380 if err = resizeSliceChecked(rv, n); err != nil {
! 381 return
! 382 }
383 copy(unsafe.Slice((*byte)(rv.UnsafePointer()), n), data)
384 return nil
385 }
386 if err = d.ensureAvailable(n); err != nil {

 386 if err = d.ensureAvailable(n); err != nil {
387 return
388 }
389 if err = resizeSliceChecked(rv, n); err != nil {
! 390 return
! 391 }
392 if l > 0 {
393 _, err = d.Read(unsafe.Slice((*byte)(rv.UnsafePointer()), n))
394 }
395 }

 495 stream := d.Available() < 0
496 var data []byte
497 if stream {
498 if data, err = d.Slice(size); err != nil {
! 499 return err
500 }
501 } else if err = d.ensureAvailable(size); err != nil {
502 return err
503 }

 502 return err
503 }
504 if !c.array {
505 if err = resizeSliceChecked(rv, n); err != nil {
! 506 return err
507 }
508 }
509 if n == 0 {
510 return nil

 510 return nil
511 }
512 if !stream {
513 if data, err = d.Slice(size); err != nil {
! 514 return err
! 515 }
516 }
517 var base unsafe.Pointer
518 if c.array {
519 base = unsafe.Pointer(rv.UnsafeAddr())

 650 if err = d.ensureAvailable(n); err != nil {
651 return
652 }
653 if err = resizeSliceChecked(rv, n); err != nil {
! 654 return
! 655 }
656 if c.signed {
657 err = decodeVarints(d, rv.UnsafePointer(), n, c.elemSize)
658 } else {
659 err = decodeVaruints(d, rv.UnsafePointer(), n, c.elemSize)

 1047 return
1048 }
1049 var n int
1050 if n, err = decodeLength(length); err != nil {
! 1051 return
! 1052 }
1053 if sliceCap(pointer) >= n {
1054 *(*int)(unsafe.Add(pointer, unsafe.Sizeof(uintptr(0)))) = n
1055 } else {
1056 if err = d.ensureAvailable(n); err != nil {

 1056 if err = d.ensureAvailable(n); err != nil {
1057 return
1058 }
1059 if err = resizeSliceChecked(rv.Field(i), n); err != nil {
! 1060 return
! 1061 }
1062 }
1063 if n > 0 {
1064 _, err = d.Read(unsafe.Slice((*byte)(sliceData(pointer)), n))
1065 if err != nil {

 1072 return
1073 }
1074 var n int
1075 if n, err = decodeLength(length); err != nil {
! 1076 return
! 1077 }
1078 if sliceCap(pointer) >= n {
1079 *(*int)(unsafe.Add(pointer, unsafe.Sizeof(uintptr(0)))) = n
1080 } else {
1081 if err = d.ensureAvailable(n); err != nil {

 1081 if err = d.ensureAvailable(n); err != nil {
1082 return
1083 }
1084 if err = resizeSliceChecked(rv.Field(i), n); err != nil {
! 1085 return
! 1086 }
1087 }
1088 size := uintptr(8)
1089 switch field.kind() {
1090 case fieldVaruint2:

 1180 return err
1181 }
1182 buffer = make([]byte, n)
1183 if _, err = io.ReadFull(d, buffer); err != nil {
! 1184 return err
! 1185 }
1186 }
1187 ret := m.Call([]reflect.Value{reflect.ValueOf(buffer)})
1188 if !ret[0].IsNil() {
1189 err = ret[0].Interface().(error)

 1265 return uvarintSize(uint64(len(value))) + len(value)
1266 case uint64:
1267 return uvarintSize(value)
1268 }
! 1269 return 0
1270 }
1271 1272 func appendStringMapValue[V stringMapValue](buffer []byte, value V) []byte {
1273 switch value := any(value).(type) {

 1279 return append(buffer, value...)
1280 case uint64:
1281 return binary.AppendUvarint(buffer, value)
1282 }
! 1283 return buffer
1284 }
1285 1286 func readStringMapValue[V stringMapValue](d *Decoder, arena *[]byte) (V, error) {
1287 var zero V

 1310 case uint64:
1311 value, err := d.ReadUvarint()
1312 return any(value).(V), err
1313 }
! 1314 return zero, nil
1315 }
1316 1317 func (stringMapCodec[V]) EncodeTo(e *Encoder, rv reflect.Value) (err error) {
1318 m := rv.Interface().(map[string]V)

 1339 }
1340 e.WriteUvarint(uint64(len(m)))
1341 for key, value := range m {
1342 if err = checkMapKey(key); err != nil {
! 1343 return
! 1344 }
1345 e.WriteUint16(uint16(len(key)))
1346 e.Write(ToBytes(key))
1347 switch value := any(value).(type) {
1348 case string:

nocopy/types.go

 267 268 func (c *byteMapCodec) EncodeTo(e *binary.Encoder, rv reflect.Value) (err error) {
269 dict := rv.Interface().(ByteMap)
270 if len(dict) > 1<<16-1 {
! 271 return errMapTooLarge
! 272 }
273 if len(dict) >= 8 {
274 if out, ok := e.Buffer().(*bytes.Buffer); ok && out != nil {
275 size := 2
276 for key, value := range dict {

 314 }
315 for i := 0; i < n; i++ {
316 k, err := decodeString(d)
317 if err != nil {
! 318 return err
! 319 }
320 var l uint64
321 if l, err = d.ReadUvarint(); err != nil {
322 return err
323 }

 344 345 func (c *hashMapCodec) EncodeTo(e *binary.Encoder, rv reflect.Value) (err error) {
346 dict := rv.Interface().(HashMap)
347 if uint64(len(dict)) > uint64(^uint32(0)) {
! 348 return errMapTooLarge
! 349 }
350 for _, value := range dict {
351 if uint64(len(value)) > uint64(^uint32(0)) {
! 352 return errMapTooLarge
! 353 }
354 }
355 if len(dict) >= 8 {
356 if out, ok := e.Buffer().(*bytes.Buffer); ok && out != nil {
357 size := 4

 382 var size uint32
383 if size, err = d.ReadUint32(); err == nil {
384 n, err := decodeLength(uint64(size))
385 if err != nil {
! 386 return err
! 387 }
388 capacity, err := mapCapacity(d, n, 12)
389 if err != nil {
390 return err
391 }

 403 }
404 var l uint32
405 var b []byte
406 if l, err = d.ReadUint32(); err != nil {
! 407 return err
! 408 }
409 if l > 0 {
410 var n int
411 if n, err = decodeLength(uint64(l)); err != nil {
! 412 return err
! 413 }
414 if b, err = d.Slice(n); err != nil {
415 return err
416 }
417 b = b[:len(b):len(b)]

 440 441 func (c *dictionaryCodec) EncodeTo(e *binary.Encoder, rv reflect.Value) (err error) {
442 dict := rv.Interface().(Dictionary)
443 if len(dict) > 1<<16-1 {
! 444 return errMapTooLarge
! 445 }
446 e.WriteUint16(uint16(len(dict)))
447 for k, v := range dict {
448 e.WriteString(k)
449 e.WriteString(v)

 466 }
467 for i := 0; i < int(size); i++ {
468 k, err := decodeString(d)
469 if err != nil {
! 470 return err
! 471 }
472 v, err := decodeString(d)
473 if err != nil {
474 return err
475 }

reader.go

 200 }
201 202 func (r *streamReader) Slice(n int) (buffer []byte, err error) {
203 if n < 0 || uint64(n) > uint64(^uint(0)>>1)/2 {
! 204 return nil, io.ErrUnexpectedEOF
! 205 }
206 if n > 64<<10 {
207 buffer := bytes.NewBuffer(make([]byte, 0, 64<<10))
208 _, err = buffer.ReadFrom(io.LimitReader(r, int64(n)))
209 if err == nil && buffer.Len() != n {

unsafe/types.go

 98 }
99 100 func decodeLength(n uint64) (int, error) {
101 if n > uint64(^uint(0)>>1) {
! 102 return 0, io.ErrUnexpectedEOF
! 103 }
104 return int(n), nil
105 }
106 107 func integerCodec[T any](size int) binary.Codec {

 123 return nil
124 }
125 n, err := decodeLength(l)
126 if err != nil {
! 127 return err
! 128 }
129 if n > int(^uint(0)>>1)/c.sizeOfInt {
130 return io.ErrUnexpectedEOF
131 }
132 size := n * c.sizeOfInt

🛡️ diff-cover-action

@kelindar
kelindar marked this pull request as ready for review August 9, 2026 10:01
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 31307283130

Coverage decreased (-10.5%) to 87.399%

Details

  • Coverage decreased (-10.5%) from the base build.
  • Patch coverage: 294 uncovered changes across 10 files (1168 of 1462 lines covered, 79.89%).
  • 57 coverage regressions across 11 files.

Uncovered Changes

FileChangedCovered%
codecs.go74156876.65%
nocopy/types.go19614372.96%
encoder.go614675.41%
unsafe/types.go604880.0%
decoder.go756485.33%
reader.go443477.27%
scanner.go857891.76%
sorted/counters.go262180.77%
sorted/timeseries.go393487.18%
union.go242187.5%
Total (12 files)1462116879.89%

Coverage Regressions

57 previously-covered lines in 11 files lost coverage.

Top 10 Files by Coverage LossLines Losing CoverageCoverage
codecs.go2784.09%
sorted/counters.go779.66%
union.go786.82%
sorted/timeseries.go589.13%
decoder.go291.05%
nocopy/types.go283.33%
reader.go291.61%
sorted/codecs.go298.87%
encoder.go188.68%
scanner.go195.71%

Coverage Stats

Coverage Status
Relevant Lines:3079
Covered Lines:2691
Line Coverage:87.4%
Coverage Strength:429.4 hits per line

💛 - Coveralls

@kelindar
kelindar merged commit a338a57 into masterAug 9, 2026
1 check passed
@kelindar
kelindar deleted the optimize3 branch August 9, 2026 10:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@kelindar@coveralls