Skip to content
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions go/mysql/replication/mysql56_gtid_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ limitations under the License.
package replication

import (
"fmt"
"maps"
"reflect"
"runtime"
"strings"
"testing"

Expand Down Expand Up @@ -902,3 +904,40 @@ func TestSIDs(t *testing.T) {
require.Len(t, sids, 1)
assert.Equal(t, "8bc65cca-3fe4-11ed-bbfb-091034d48b3e", sids[0].String())
}

// TestParseMysql56GTIDSetIntervalsCapHint checks that the preallocation hint for
// the intervals slice does not follow a count taken from the input beyond what that
// input could hold, while a genuinely long interval list still parses unchanged.
// The hostile case asserts on allocation volume, because a colon run parses without
// error either way once the intervals are all discarded.
func TestParseMysql56GTIDSetIntervalsCapHint(t *testing.T) {
const sid = "00010203-0405-0607-0809-0a0b0c0d0e0f"

// Results must be identical either side of the bound.
for _, n := range []int{1, 10, 100, 1023, 1024, 1025, 2051, 5000} {
var sb strings.Builder
sb.WriteString(sid)
for i := 0; i < n; i++ {
// non-overlapping ascending intervals, so none are merged or discarded
fmt.Fprintf(&sb, ":%d-%d", 2*i+1, 2*i+1)
}
got, err := ParseMysql56GTIDSet(sb.String())
require.NoError(t, err, "n=%d", n)
sidVal, err := ParseSID(sid)
require.NoError(t, err)
assert.Len(t, got[sidVal], n, "n=%d", n)
Comment on lines +924 to +928

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make the cap-hint test exercise the clamp

This test passes unchanged against the parent implementation and therefore cannot guard the allocation fix: every generated :%d-%d interval occupies at least four bytes, so len(tail)/4+1 is never smaller than the colon-derived capacity and the new clamping branch is not taken. The colon-run test also passes before the fix because it checks only the existing parse error. Test the extracted capacity calculation or otherwise verify that malformed input receives bounded preallocation so reverting the fix makes a test fail.

AGENTS.md reference: AGENTS.md:L79-L80

Useful? React with 👍 / 👎.

}

// A long run of colons carries no intervals at all, so reserving one per colon
// is pure waste. 1MiB of colons is 16MiB of interval structs unbounded.
hostile := sid + ":" + strings.Repeat(":", 1<<20)
var before, after runtime.MemStats
runtime.GC()
runtime.ReadMemStats(&before)
_, _ = ParseMysql56GTIDSet(hostile)
runtime.ReadMemStats(&after)
allocated := after.TotalAlloc - before.TotalAlloc
assert.Less(t, allocated, uint64(8<<20),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore the cap bound before asserting its allocation ceiling

This revision adds the allocation assertion but does not include the production fix: ParseMysql56GTIDSet still preallocates strings.Count(tail, ":")+1 intervals, so this 1 MiB colon input necessarily allocates roughly 16 MiB for the slice and exceeds the 8 MiB limit. Consequently, this focused test—and therefore the package test suite—fails on the reviewed commit. Fresh evidence relative to the earlier comment is that the new assertion now exercises the regression, but the corresponding capacity clamp has disappeared from this revision.

AGENTS.md reference: AGENTS.md:L79-L80

Useful? React with 👍 / 👎.

"parsing %d colons allocated %d bytes for intervals that are all discarded",
1<<20, allocated)
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Outdated
}