Robert Griesemer (Gerrit)

unread,

Apr 28, 2026, 5:56:03 PM (11 hours ago) Apr 28

to goph...@pubsubhelper.golang.org, Robert Griesemer, golang-co...@googlegroups.com

Robert Griesemer has uploaded the change for review

Commit message

index/suffixarray: report error rather than panic for corrupted data

If the encoded suffixarray data had more indices than the (suffix)
array length, Index.Read panicked with an index-out-of-bounds error.
Guards against that panic and report an error instead.

While at it, change the error string used for other errors from
"data too large" to "corrupted data" which is more versatile.

Added testcase with detailed description.

Fixes #53352.

Change-Id: I9b1ba04c408b8d51943f7a29abf429fbe743fd3d

Change diff

diff --git a/src/index/suffixarray/suffixarray.go b/src/index/suffixarray/suffixarray.go
index 2b96c8b..5541f09 100644
--- a/src/index/suffixarray/suffixarray.go
+++ b/src/index/suffixarray/suffixarray.go
@@ -117,7 +117,7 @@
 	return
 }
 
-var errTooBig = errors.New("suffixarray: data too large")
+var errCorrupted = errors.New("suffixarray: data corrupted")
 
 // readSlice reads data[:n] from r and returns n.
 // It uses buf to buffer the read.
@@ -130,7 +130,7 @@
 	}
 	if int64(int(size64)) != size64 || int(size64) < 0 {
 		// We never write chunks this big anyway.
-		return 0, errTooBig
+		return 0, errCorrupted
 	}
 	size := int(size64)
 
@@ -140,8 +140,14 @@
 	}
 
 	// decode as many elements as present in buf
+	len := data.len()
 	for p := binary.MaxVarintLen64; p < size; n++ {
 		x, w := binary.Uvarint(buf[p:])
+		// prevent index-out-of-bounds error if data is corrupted
+		// (was go.dev/issue/53352)
+		if n >= len {
+			return n, errCorrupted
+		}
 		data.set(n, int64(x))
 		p += w
 	}
@@ -162,7 +168,7 @@
 		return err
 	}
 	if int64(int(n64)) != n64 || int(n64) < 0 {
-		return errTooBig
+		return errCorrupted
 	}
 	n := int(n64)
 
diff --git a/src/index/suffixarray/suffixarray_test.go b/src/index/suffixarray/suffixarray_test.go
index da092a7..68e4258 100644
--- a/src/index/suffixarray/suffixarray_test.go
+++ b/src/index/suffixarray/suffixarray_test.go
@@ -614,3 +614,52 @@
 		})
 	}
 }
+
+func TestIssue53352(t *testing.T) {
+	data := []byte("x")
+	index := New(data)
+	var buf bytes.Buffer
+	if err := index.Write(&buf); err != nil {
+		t.Fatal(err)
+	}
+
+	// buffer encoding is as follows:
+	//
+	// [ data length n | data bytes | index slice buffer size |  indices  ] ... (next slice, if any)
+	// \__ 10 bytes __/\_ n bytes _/\_______ 10 bytes _______/\_ varuints /
+	//
+	// n and s are encoded as varints using 10 bytes always so they can be patched.
+	// For small values x >= 0 the varint encoded value is 2*x.
+
+	// For the above data we have n == len("x") == 1.
+	n := len(data)
+	encoding := buf.Bytes()
+	if got := int(encoding[0]); got != 2*n {
+		t.Fatalf("got n = %d; want %d", got, 2*n)
+	}
+
+	// For the above data, the index slice buffer contains a single index entry (0 for "x")
+	// plus the size of the index buffer (10 bytes), so s == 10 + 1 == 11; and s is encoded
+	// immediately following the data bytes.
+	s := 10 + 1
+	if got := int(encoding[10+n]); got != 2*s {
+		t.Fatalf("got s = %d; want %d", got, 2*s)
+	}
+
+	// Reading back the encoding should work without errors.
+	if err := index.Read(bytes.NewBuffer(encoding)); err != nil {
+		t.Fatal(err)
+	}
+
+	// Adding an extra index corrupts the format:
+	// Now we have one more index than data bytes.
+	s++                            // increase slice buffer size
+	encoding = append(encoding, 0) // add one more index
+	encoding[10+n] = byte(2 * s)   // update index slice buffer size
+
+	// Reading back the corrupted encoding should report an error.
+	// Before fixing go.dev/issue/53352, this resulted in a index-out-of-range panic.
+	if err := index.Read(bytes.NewBuffer(encoding)); err == nil || err.Error() != errCorrupted.Error() {
+		t.Fatalf("got %q; want %q", err, errCorrupted)
+	}
+}

Change information

Files:

M src/index/suffixarray/suffixarray.go
M src/index/suffixarray/suffixarray_test.go

Change size: M

Delta: 2 files changed, 58 insertions(+), 3 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Robert Griesemer (Gerrit)

unread,

Apr 28, 2026, 6:00:06 PM (11 hours ago) Apr 28

to Robert Griesemer, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Robert Griesemer uploaded new patchset

Robert Griesemer uploaded patch set #2 to this change.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Robert Griesemer (Gerrit)

unread,

Apr 28, 2026, 6:00:31 PM (11 hours ago) Apr 28

to Robert Griesemer, goph...@pubsubhelper.golang.org, Mark Freeman, golang-co...@googlegroups.com

Attention needed from Mark Freeman and Robert Griesemer

Robert Griesemer voted

Auto-Submit	+1
Code-Review	+2
Commit-Queue	+1

Open in Gerrit

Related details

Attention is currently required from:

Mark Freeman
Robert Griesemer

Submit Requirements:

Code-Review

No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Robert Griesemer (Gerrit)

unread,

Apr 28, 2026, 6:09:24 PM (11 hours ago) Apr 28

to Robert Griesemer, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention needed from Mark Freeman, Robert Griesemer and Robert Griesemer

Robert Griesemer uploaded new patchset

Robert Griesemer uploaded patch set #3 to this change.

Open in Gerrit

Related details

Attention is currently required from:

Mark Freeman
Robert Griesemer
Robert Griesemer

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Robert Griesemer (Gerrit)

unread,

Apr 28, 2026, 6:09:51 PM (11 hours ago) Apr 28

to Robert Griesemer, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Mark Freeman, golang-co...@googlegroups.com

Attention needed from Mark Freeman and Robert Griesemer

Robert Griesemer voted and added 1 comment

Votes added by Robert Griesemer

Auto-Submit	+1
Code-Review	+2
Commit-Queue	+1

1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Robert Griesemer . resolved

Ready now.

Open in Gerrit

Related details

Attention is currently required from:

Mark Freeman
Robert Griesemer

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Robert Griesemer (Gerrit)

unread,

Apr 28, 2026, 6:13:47 PM (11 hours ago) Apr 28

to Robert Griesemer, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Mark Freeman, golang-co...@googlegroups.com

Attention needed from Mark Freeman and Robert Griesemer

Robert Griesemer added 1 comment

File src/index/suffixarray/suffixarray_test.go

Line 662, Patchset 3 (Latest): t.Fatalf("got %q; want %q", err, errCorrupted)

Robert Griesemer . unresolved

this should be %v in both cases to work correctly if err == nil

Open in Gerrit

Related details

Attention is currently required from:

Mark Freeman
Robert Griesemer

Submit Requirements:

Code-Review
No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Robert Griesemer (Gerrit)

unread,

Apr 28, 2026, 7:05:06 PM (10 hours ago) Apr 28

to Robert Griesemer, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Mark Freeman, golang-co...@googlegroups.com

Attention needed from Mark Freeman and Robert Griesemer

Robert Griesemer added 1 comment

File src/index/suffixarray/suffixarray_test.go

Line 662, Patchset 3 (Latest): t.Fatalf("got %q; want %q", err, errCorrupted)

Robert Griesemer . resolved

this should be %v in both cases to work correctly if err == nil

Robert Griesemer

fixed in subsequent CL

Open in Gerrit

Related details

Attention is currently required from:

Mark Freeman
Robert Griesemer

Submit Requirements:

Code-Review
No-Unresolved-Comments

Review-Enforcement

TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[go] index/suffixarray: report error rather than panic for corrupted data

Robert Griesemer (Gerrit)

Robert Griesemer has uploaded the change for review

Commit message

Change diff

Change information

Related details

Robert Griesemer (Gerrit)

Robert Griesemer uploaded new patchset

Related details

Robert Griesemer (Gerrit)

Robert Griesemer voted

Related details

Robert Griesemer (Gerrit)

Robert Griesemer uploaded new patchset

Related details

Robert Griesemer (Gerrit)

Robert Griesemer voted and added 1 comment

Votes added by Robert Griesemer

1 comment

Related details

Robert Griesemer (Gerrit)

Robert Griesemer added 1 comment

Related details

Robert Griesemer (Gerrit)

Robert Griesemer added 1 comment

Related details