Skip to content

Detailed Bug Analysis: Go Primitive Map Chunking Loop Bug #1576

Description

@codeCraft-Ritik

[Bug] Go: Primitive map serializers corrupt data when map size exceeds 255

Description

The Go primitive map serializers in go/fory/map_primitive.go incorrectly handle maps containing more than MAX_CHUNK_SIZE (255) entries.

Maps are serialized using a chunked protocol, where each chunk contains at most 255 entries. However, the current implementation uses a nested range m loop for every chunk. In Go, each range m starts a new map iteration rather than continuing from the previous iteration.

As a result, when a map contains more than 255 entries, subsequent chunks can serialize entries that were already written, while other entries may never be serialized at all. This can lead to silent data corruption and missing entries after deserialization.


Root Cause

The current implementation follows this pattern:

remaining := length

for remaining > 0 {
    chunkSize := remaining

    if chunkSize > MAX_CHUNK_SIZE {
        chunkSize = MAX_CHUNK_SIZE
    }

    // Write chunk header...

    count := 0

    for k, v := range m {
        // BUG: every outer-loop iteration starts a new
        // iteration over the entire map.
        writeString(buf, k)
        writeString(buf, v)

        count++

        if count >= chunkSize {
            break
        }
    }

    remaining -= chunkSize
}


The problem is:

`for k, v := range m`

does not preserve its iteration position after break.

For example, with a map containing 300 entries:

Chunk 1first 255 entries
Chunk 2starts a NEW range(m) iteration

The second chunk therefore does not continue from entry 256.

Because Go map iteration order is not guaranteed:

Entries from the first chunk can be serialized again.
Entries not visited in the first chunk may never be serialized.
The resulting serialized data can contain duplicate keys.
Some original entries can be completely missing.
Deserialization can produce a map that differs from the original.
The exact affected entries can vary between executions.
Affected Functions

File:

go/fory/map_primitive.go

All 9 specialized primitive map serializers are affected:

writeMapStringString
writeMapStringInt64
writeMapStringInt32
writeMapStringInt
writeMapStringFloat64
writeMapStringBool
writeMapInt32Int32
writeMapInt64Int64
writeMapIntInt

The generic mapSerializer in go/fory/map.go does not appear to have this issue because it uses reflect.MapIter / iter.Next() and maintains the iterator position correctly.

Why This Causes Data Corruption

For a map containing 300 entries:

MAX_CHUNK_SIZE = 255

Expected:

┌──────────────────────────────┐
│ Chunk 1: entries 1255      │
└──────────────────────────────┘
               ↓
┌──────────────────────────────┐
│ Chunk 2: entries 256300     │
└──────────────────────────────┘

The current implementation effectively does:

┌──────────────────────────────┐
│ Chunk 1                      │
│ range(m) → 255 entries       │
└──────────────────────────────┘
               ↓
┌──────────────────────────────┐
│ Chunk 2                      │
│ range(m) → NEW iteration     │
│ starts over again             │
└──────────────────────────────┘

Therefore, the serializer loses the iteration position between chunks.

Minimal Reproduction
package fory_test

import (
    "fmt"
    "testing"

    "github.com/apache/fory/go/fory"
    "github.com/stretchr/testify/require"
)

func TestLargePrimitiveMapSerialization(t *testing.T) {
    f := fory.NewFory(
        fory.WithXlang(false),
        fory.WithCompatible(false),
    )

    // 300 entries > MAX_CHUNK_SIZE (255)
    original := make(map[string]string, 300)

    for i := 0; i < 300; i++ {
        original[fmt.Sprintf("key_%04d", i)] =
            fmt.Sprintf("val_%04d", i)
    }

    data, err := f.Marshal(original)
    require.NoError(t, err)

    var decoded map[string]string

    err = f.Unmarshal(data, &decoded)
    require.NoError(t, err)

    require.Equal(
        t,
        len(original),
        len(decoded),
        "map length mismatch due to chunk iteration bug",
    )

    require.Equal(t, original, decoded)
}
Expected Result
len(decoded) == len(original)
decoded == original

The decoded map should contain all 300 entries with exactly the same key/value pairs as the original map.

Actual Result

For maps larger than 255 entries, the primitive serializer can produce an incorrect serialized representation because subsequent chunks restart the map iteration.

This can result in missing and/or overwritten entries after deserialization.

Proposed Fix

Replace the nested for remaining > 0 / range m loops with a single iteration over the map.

A new chunk header should be written whenever:

count % MAX_CHUNK_SIZE == 0

For example:

func writeMapStringString(
    buf *ByteBuffer,
    m map[string]string,
    hasGenerics bool,
) {
    length := len(m)

    buf.WriteVarUint32(uint32(length))

    if length == 0 {
        return
    }

    count := 0

    for k, v := range m {
        if count%MAX_CHUNK_SIZE == 0 {
            chunkSize := length - count

            if chunkSize > MAX_CHUNK_SIZE {
                chunkSize = MAX_CHUNK_SIZE
            }

            if hasGenerics {
                buf.WriteUint8(KEY_DECL_TYPE | VALUE_DECL_TYPE)
                buf.WriteUint8(uint8(chunkSize))
            } else {
                buf.WriteUint8(0)
                buf.WriteUint8(uint8(chunkSize))
                buf.WriteUint8(uint8(STRING))
                buf.WriteUint8(uint8(STRING))
            }
        }

        writeString(buf, k)
        writeString(buf, v)

        count++
    }
}

The same single-pass iteration approach should be applied to all nine primitive map serializers.

Recommended Regression Tests

Tests should specifically cover the chunk boundaries:

testCases := []int{
    0,
    1,
    MAX_CHUNK_SIZE - 1,
    MAX_CHUNK_SIZE,
    MAX_CHUNK_SIZE + 1,
    300,
    MAX_CHUNK_SIZE * 2,
    MAX_CHUNK_SIZE*2 + 1,
}

For every test size:

Create a map containing exactly that number of entries.
Serialize the map.
Deserialize the serialized data.
Verify that the decoded map has the same length.
Verify that the decoded map is equal to the original map.

Example:

func TestPrimitiveMapChunkBoundaries(t *testing.T) {
    testCases := []int{
        0,
        1,
        MAX_CHUNK_SIZE - 1,
        MAX_CHUNK_SIZE,
        MAX_CHUNK_SIZE + 1,
        300,
        MAX_CHUNK_SIZE * 2,
        MAX_CHUNK_SIZE*2 + 1,
    }

    for _, size := range testCases {
        t.Run(fmt.Sprintf("size_%d", size), func(t *testing.T) {
            f := fory.NewFory(
                fory.WithXlang(false),
                fory.WithCompatible(false),
            )

            original := make(map[string]string, size)

            for i := 0; i < size; i++ {
                original[fmt.Sprintf("key_%04d", i)] =
                    fmt.Sprintf("value_%04d", i)
            }

            data, err := f.Marshal(original)
            require.NoError(t, err)

            var decoded map[string]string

            err = f.Unmarshal(data, &decoded)
            require.NoError(t, err)

            require.Equal(t, original, decoded)
        })
    }
}

It would also be useful to run equivalent tests against the different primitive map types to ensure all nine optimized serializers are covered.


Suggested Patch Direction

The essential change is:

// Current approach — BUGGY

remaining := length

for remaining > 0 {
    // Write chunk header...

    count := 0

    for k, v := range m {
        // Write entry...

        count++

        if count >= chunkSize {
            break
        }
    }

    remaining -= chunkSize
}

to:

// Proposed approach — SINGLE PASS

count := 0

for k, v := range m {
    if count%MAX_CHUNK_SIZE == 0 {
        // Write chunk header.
    }

    // Write entry.

    count++
}

This keeps the existing chunked serialization format while ensuring the map is traversed continuously.

Conclusion

The primitive map serializers currently create a new range m iteration for every chunk. Since Go map iteration does not resume after break, maps larger than MAX_CHUNK_SIZE can be serialized incorrectly.

Using a single-pass map iteration and emitting chunk headers every MAX_CHUNK_SIZE entries should prevent duplicate/missing entries while preserving the existing serialization protocol.

This should be fixed because it can cause silent data loss and data corruption for maps containing more than 255 entries.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions