diff --git a/go/mysql/replication/mysql56_gtid_set.go b/go/mysql/replication/mysql56_gtid_set.go index b336b1d4ef2..dd15a670290 100644 --- a/go/mysql/replication/mysql56_gtid_set.go +++ b/go/mysql/replication/mysql56_gtid_set.go @@ -91,7 +91,28 @@ func ParseMysql56GTIDSet(s string) (Mysql56GTIDSet, error) { return nil, vterrors.Wrapf(err, "invalid MySQL 5.6 GTID set (%q)", s) } - intervals := make([]interval, 0, strings.Count(tail, ":")+1) + // The capacity is only a hint to append, so bounding it cannot change the + // parsed result. It is bounded because the hint is derived from untrusted + // input: a valid SID followed by a long run of colons would otherwise + // reserve one interval per colon before any interval is parsed, and + // parseInterval below rejects the very first one. + // + // The bound is a fraction of the remaining input rather than a constant, so + // a genuinely long interval list still gets an exact hint. parseInterval + // accepts a singleton "N" as well as a range "N-M", so the shortest interval + // that contributes an element is two bytes including its separator ("1:"), + // and duplicates are retained rather than merged. Dividing by more than two + // would under-reserve a valid singleton list such as "1:1:1:..." and force + // append to grow and copy: measured at 100k singletons, a len(tail)/4 bound + // allocated 6,685,104 bytes against 1,606,032 for the unbounded hint, a + // 4.16x regression on valid input. Two is therefore the tightest correct + // divisor, and it still cuts a 1MiB colon run from 19,963,872 bytes to + // 11,036,616. + nIntervals := strings.Count(tail, ":") + 1 + if max := len(tail)/2 + 1; nIntervals > max { + nIntervals = max + } + intervals := make([]interval, 0, nIntervals) for len(tail) > 0 { if idx := strings.IndexByte(tail, ':'); idx >= 0 { head = tail[:idx] diff --git a/go/mysql/replication/mysql56_gtid_set_test.go b/go/mysql/replication/mysql56_gtid_set_test.go index 4b544220ee5..db9f2709ca3 100644 --- a/go/mysql/replication/mysql56_gtid_set_test.go +++ b/go/mysql/replication/mysql56_gtid_set_test.go @@ -17,8 +17,10 @@ limitations under the License. package replication import ( + "fmt" "maps" "reflect" + "runtime" "strings" "testing" @@ -902,3 +904,74 @@ 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 := range n { + // 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) + } + + // 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. + // + // The bound cannot be tightened past len(tail)/2 to shrink this further: the + // densest valid list is one singleton per two bytes ("1:"), so a smaller + // divisor under-reserves legitimate input. That trade-off is why the threshold + // here is 12MiB rather than the 8MiB a len(tail)/4 bound would reach -- + // measured, /4 cut this case to 5.0MiB but inflated a valid 100k-singleton + // list from 1,606,032 to 6,685,104 bytes. Bounding a hostile input is not + // worth a 4.16x regression on a valid one. + 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(12<<20), + "parsing %d colons allocated %d bytes for intervals that are all discarded", + 1<<20, allocated) + + // The bound must not under-reserve a valid list. parseInterval accepts a + // singleton "N", so "1:1:1:..." needs one interval per two input bytes -- the + // densest valid form. Note that duplicate singletons are each retained rather + // than merged, so this really does need `singletons` intervals. + const singletons = 100000 + var sb strings.Builder + sb.WriteString(sid) + for range singletons { + sb.WriteString(":1") + } + runtime.GC() + runtime.ReadMemStats(&before) + dense, err := ParseMysql56GTIDSet(sb.String()) + runtime.ReadMemStats(&after) + require.NoError(t, err) + denseAlloc := after.TotalAlloc - before.TotalAlloc + sidVal, err := ParseSID(sid) + require.NoError(t, err) + assert.Len(t, dense[sidVal], singletons, + "duplicate singletons are retained, not merged") + // 100k intervals at 16 bytes is 1.6MiB; allow headroom for the strings but not + // for a doubling-and-copying append. + assert.Less(t, denseAlloc, uint64(3<<20), + "a dense valid singleton list allocated %d bytes, which means the capacity "+ + "hint under-reserved and append had to grow", denseAlloc) +}