Skip to content

Commit c6cd014

Browse files
authored
index: surface corrupt rune and match offsets (#1108)
Malformed rune mappings previously underflowed into huge offsets, while defensive bounds checks risked turning the same corruption into an invisible non-match. Propagate the first per-document corruption error through Search so callers can distinguish incomplete results from a valid zero-match response. Note: zoekt already handles panics in search as a corrupt index, and we mark as part of the search statistics that we crashed a shard. This commit makes it so we are far more defensive.
1 parent 755cd00 commit c6cd014

8 files changed

Lines changed: 401 additions & 41 deletions

File tree

index/bits.go

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ func toLower(in []byte) []byte {
5757
}
5858

5959
// compare 'lower' and 'mixed', where lower is the needle. 'mixed' may
60-
// be larger than 'lower'. Returns whether there was a match, and if
61-
// yes, the byte size of the match.
62-
func caseFoldingEqualsRunes(lower, mixed []byte) (int, bool) {
60+
// be larger than 'lower'. Returns the byte size of the match, whether there was
61+
// a match, and whether mixed ended before lower.
62+
func caseFoldingEqualsRunes(lower, mixed []byte) (int, bool, bool) {
6363
matchTotal := 0
6464
for len(lower) > 0 && len(mixed) > 0 {
6565
lb := lower[0]
@@ -70,7 +70,7 @@ func caseFoldingEqualsRunes(lower, mixed []byte) (int, bool) {
7070
mb |= 0x20
7171
}
7272
if lb != mb {
73-
return 0, false
73+
return 0, false, !hasEnoughRunes(mixed, lower)
7474
}
7575
lower = lower[1:]
7676
mixed = mixed[1:]
@@ -86,11 +86,37 @@ func caseFoldingEqualsRunes(lower, mixed []byte) (int, bool) {
8686
matchTotal += msz
8787

8888
if lr != unicode.ToLower(mr) {
89-
return 0, false
89+
return 0, false, !hasEnoughRunes(mixed, lower)
9090
}
9191
}
9292

93-
return matchTotal, len(lower) == 0
93+
return matchTotal, len(lower) == 0, len(lower) > 0
94+
}
95+
96+
func hasEnoughRunes(mixed, lower []byte) bool {
97+
// A UTF-8 rune occupies at most UTFMax bytes, and lower cannot contain
98+
// more runes than bytes. A sufficiently long mixed suffix therefore proves
99+
// the candidate span fits without walking either slice. In practice this
100+
// keeps ordinary mismatches constant-time and reserves the exact scan below
101+
// for candidates close enough to the document end to be truncated.
102+
if len(mixed)/utf8.UTFMax >= len(lower) {
103+
return true
104+
}
105+
106+
// Compare only rune availability, not values: the caller already knows the
107+
// candidate mismatches. Reaching the end of mixed first means the candidate's
108+
// expected rune span crosses the document boundary, which is a corrupt index
109+
// invariant rather than an ordinary non-match.
110+
for len(lower) > 0 {
111+
if len(mixed) == 0 {
112+
return false
113+
}
114+
_, sz := utf8.DecodeRune(lower)
115+
lower = lower[sz:]
116+
_, sz = utf8.DecodeRune(mixed)
117+
mixed = mixed[sz:]
118+
}
119+
return true
94120
}
95121

96122
type ngram uint64
@@ -394,12 +420,12 @@ func makeRuneOffsetMap(off []uint32) runeOffsetMap {
394420
// runes to traverse, given the granularity of runeOffsetFrequency.
395421
//
396422
// It does this by finding the nearest point to interpolate from in the map.
397-
func (m runeOffsetMap) lookup(runeOffset uint32) (uint32, uint32) {
423+
func (m runeOffsetMap) lookup(runeOffset uint32) (uint64, uint32) {
398424
left := runeOffset % runeOffsetFrequency
399425
runeOffset -= left
400426
slen := len(m)
401427
if slen == 0 {
402-
return runeOffset, left
428+
return uint64(runeOffset), left
403429
}
404430
// sort.Search finds the *first* index for which the predicate is true,
405431
// but we want to find the *last* index for which the predicate is true.
@@ -410,9 +436,9 @@ func (m runeOffsetMap) lookup(runeOffset uint32) (uint32, uint32) {
410436
idx = slen - 1 - idx
411437
// idx is now in the range [-1, len(m))-- -1 indicates that the offset is smaller
412438
// than the first entry in the map, so no correction is necessary.
413-
byteOff := runeOffset
439+
byteOff := uint64(runeOffset)
414440
if idx >= 0 {
415-
byteOff = m[idx].byteOffset + runeOffset - m[idx].runeOffset
441+
byteOff = uint64(m[idx].byteOffset) + uint64(runeOffset) - uint64(m[idx].runeOffset)
416442
}
417443
return byteOff, left
418444
}

index/bits_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ func TestCondenseRuneOffsets(t *testing.T) {
224224
for j, byteOffset := range tc.arr {
225225
runeOffset := uint32(j * runeOffsetFrequency)
226226
gotByteOffset, _ := got.lookup(runeOffset)
227-
if gotByteOffset != byteOffset {
227+
if gotByteOffset != uint64(byteOffset) {
228228
t.Errorf("#%d: lookup(%v) got %v, want %v", i, runeOffset, gotByteOffset, byteOffset)
229229
}
230230
}
@@ -247,7 +247,7 @@ func TestRuneOffsetLookup(t *testing.T) {
247247
if gotLeft != tc.wantLeft {
248248
t.Errorf("#%d: got left=%v, want left=%v", i, gotLeft, tc.wantLeft)
249249
}
250-
if gotOff != tc.wantOff {
250+
if gotOff != uint64(tc.wantOff) {
251251
t.Errorf("#%d: got off=%v, want off=%v", i, gotOff, tc.wantOff)
252252
}
253253
}
@@ -257,7 +257,7 @@ func TestRuneOffsetLookup(t *testing.T) {
257257
wanted := []uint32{0, 0, 0, 105, 105, 105, 210, 210, 310, 310, 430, 430, 530, 630}
258258
for i, v := range inputs {
259259
got, _ := m.lookup(v)
260-
if got != wanted[i] {
260+
if got != uint64(wanted[i]) {
261261
t.Errorf("got off=%v, want off=%v for map=%v", got, wanted[i], m)
262262
}
263263
}

index/case_folding_bench_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func TestCaseFoldingEqualsRunes(t *testing.T) {
2525
{"äbč", "ÄBČ", true, 5}, // 'ä' (2 bytes), 'b' (1 byte), 'č' (2 bytes)
2626
{"äbč", "ÄBX", false, 0},
2727
} {
28-
sz, ok := caseFoldingEqualsRunes([]byte(tc.lower), []byte(tc.mixed))
28+
sz, ok, _ := caseFoldingEqualsRunes([]byte(tc.lower), []byte(tc.mixed))
2929
if ok != tc.wantMatch || sz != tc.wantSz {
3030
t.Errorf("caseFoldingEqualsRunes(%q, %q): got (%d, %t), want (%d, %t)",
3131
tc.lower, tc.mixed, sz, ok, tc.wantSz, tc.wantMatch)
@@ -45,7 +45,7 @@ func BenchmarkCaseFoldingEqualsRunes(b *testing.B) {
4545
b.Run("ASCII", func(b *testing.B) {
4646
b.ReportAllocs()
4747
for i := 0; i < b.N; i++ {
48-
sz, ok := caseFoldingEqualsRunes(asciiLower, asciiMixed)
48+
sz, ok, _ := caseFoldingEqualsRunes(asciiLower, asciiMixed)
4949
if !ok || sz != len(asciiMixed) {
5050
b.Fatalf("bad match: %d, %t", sz, ok)
5151
}
@@ -55,7 +55,7 @@ func BenchmarkCaseFoldingEqualsRunes(b *testing.B) {
5555
b.Run("Unicode", func(b *testing.B) {
5656
b.ReportAllocs()
5757
for i := 0; i < b.N; i++ {
58-
sz, ok := caseFoldingEqualsRunes(unicodeLower, unicodeMixed)
58+
sz, ok, _ := caseFoldingEqualsRunes(unicodeLower, unicodeMixed)
5959
if !ok || sz != len(unicodeMixed) {
6060
b.Fatalf("bad match: %d, %t", sz, ok)
6161
}

index/contentprovider.go

Lines changed: 92 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package index
1616

1717
import (
1818
"bytes"
19+
"fmt"
1920
"log"
2021
"path"
2122
"slices"
@@ -36,7 +37,6 @@ type contentProvider struct {
3637
stats *zoekt.Stats
3738

3839
// mutable
39-
err error
4040
idx uint32
4141
_data []byte
4242
_nl []uint32
@@ -58,10 +58,34 @@ func (p *contentProvider) setDocument(docID uint32) {
5858
p._data = nil
5959
}
6060

61+
// panicCorrupt stops a search as soon as it detects a corrupt shard invariant.
62+
// searchOneShard recovers the panic at the shard boundary, logs the error with
63+
// the query and stack trace, and increments Stats.Crashes. Callers therefore get
64+
// partial results with an explicit crashed-shard signal rather than a silent
65+
// non-match.
66+
func (p *contentProvider) panicCorrupt(err error) {
67+
shard := "unknown"
68+
if p.id.file != nil {
69+
shard = p.id.file.Name()
70+
}
71+
repo := "unknown"
72+
if p.idx < uint32(len(p.id.repos)) {
73+
repoID := p.id.repos[p.idx]
74+
if int(repoID) < len(p.id.repoMetaData) {
75+
repo = p.id.repoMetaData[repoID].Name
76+
}
77+
}
78+
panic(fmt.Errorf("corrupt shard %q while searching repository %q, document %d: %w", shard, repo, p.idx, err))
79+
}
80+
6181
func (p *contentProvider) docSections() []DocumentSection {
6282
if p._sects == nil {
6383
var sz uint32
64-
p._sects, sz, p.err = p.id.readDocSections(p.idx, p._sectBuf)
84+
var err error
85+
p._sects, sz, err = p.id.readDocSections(p.idx, p._sectBuf)
86+
if err != nil {
87+
p.panicCorrupt(fmt.Errorf("reading document sections: %w", err))
88+
}
6589
p.stats.ContentBytesLoaded += int64(sz)
6690
p._sectBuf = p._sects
6791
}
@@ -71,7 +95,11 @@ func (p *contentProvider) docSections() []DocumentSection {
7195
func (p *contentProvider) newlines() newlines {
7296
if p._nl == nil {
7397
var sz uint32
74-
p._nl, sz, p.err = p.id.readNewlines(p.idx, p._nlBuf)
98+
var err error
99+
p._nl, sz, err = p.id.readNewlines(p.idx, p._nlBuf)
100+
if err != nil {
101+
p.panicCorrupt(fmt.Errorf("reading newline offsets: %w", err))
102+
}
75103
p._nlBuf = p._nl
76104
p.stats.ContentBytesLoaded += int64(sz)
77105
}
@@ -84,7 +112,11 @@ func (p *contentProvider) data(fileName bool) []byte {
84112
}
85113

86114
if p._data == nil {
87-
p._data, p.err = p.id.readContents(p.idx)
115+
var err error
116+
p._data, err = p.id.readContents(p.idx)
117+
if err != nil {
118+
p.panicCorrupt(fmt.Errorf("reading content: %w", err))
119+
}
88120
p.stats.FilesLoaded++
89121
p.stats.ContentBytesLoaded += int64(len(p._data))
90122
}
@@ -95,45 +127,86 @@ func (p *contentProvider) data(fileName bool) []byte {
95127
// runes (relative to document start). If filename is set, the corpus
96128
// is the set of filenames, with the document being the name itself.
97129
func (p *contentProvider) findOffset(filename bool, r uint32) uint32 {
98-
if p.id.metaData.PlainASCII {
99-
return r
100-
}
101-
102-
sample := p.id.runeOffsets
103-
runeEnds := p.id.fileEndRunes
104-
fileStartByte := p.id.boundaries[p.idx]
130+
var sample runeOffsetMap
131+
var runeEnds []uint32
132+
var fileStartByte, fileEndByte uint32
133+
kind := "content"
105134
if filename {
106135
sample = p.id.fileNameRuneOffsets
107136
runeEnds = p.id.fileNameEndRunes
108137
fileStartByte = p.id.fileNameIndex[p.idx]
138+
fileEndByte = p.id.fileNameIndex[p.idx+1]
139+
kind = "filename"
140+
} else {
141+
sample = p.id.runeOffsets
142+
runeEnds = p.id.fileEndRunes
143+
fileStartByte = p.id.boundaries[p.idx]
144+
fileEndByte = p.id.boundaries[p.idx+1]
109145
}
110146

111-
absR := r
147+
if p.id.metaData.PlainASCII {
148+
if r > fileEndByte-fileStartByte {
149+
p.panicCorrupt(fmt.Errorf("%s rune offset %d is after file size %d", kind, r, fileEndByte-fileStartByte))
150+
return 0
151+
}
152+
return r
153+
}
154+
155+
absR64 := uint64(r)
112156
if p.idx > 0 {
113-
absR += runeEnds[p.idx-1]
157+
absR64 += uint64(runeEnds[p.idx-1])
158+
}
159+
if absR64 > uint64(^uint32(0)) {
160+
p.panicCorrupt(fmt.Errorf("%s rune offset %d overflows the corpus rune offset", kind, r))
161+
return 0
114162
}
163+
absR := uint32(absR64)
115164

116165
byteOff, left := sample.lookup(absR)
117166

118167
var data []byte
119168

120169
if filename {
121-
data = p.id.fileNameContent[byteOff:]
170+
if byteOff > uint64(len(p.id.fileNameContent)) {
171+
p.panicCorrupt(fmt.Errorf("filename rune offset %d maps to byte offset %d past filename data size %d", absR, byteOff, len(p.id.fileNameContent)))
172+
return 0
173+
}
174+
data = p.id.fileNameContent[uint32(byteOff):]
122175
} else {
123-
data, p.err = p.id.readContentSlice(byteOff, 3*runeOffsetFrequency)
124-
if p.err != nil {
176+
corpusEnd := p.id.boundaries[len(p.id.boundaries)-1]
177+
if byteOff > uint64(corpusEnd) {
178+
p.panicCorrupt(fmt.Errorf("content rune offset %d maps to byte offset %d past content data size %d", absR, byteOff, corpusEnd))
179+
return 0
180+
}
181+
var err error
182+
data, err = p.id.readContentSlice(uint32(byteOff), 3*runeOffsetFrequency)
183+
if err != nil {
184+
p.panicCorrupt(fmt.Errorf("content rune offset %d cannot load bytes at offset %d: %w", absR, byteOff, err))
125185
return 0
126186
}
127187
}
128188
for left > 0 {
189+
if len(data) == 0 {
190+
p.panicCorrupt(fmt.Errorf("%s rune offset %d has no decode bytes at byte offset %d", kind, absR, byteOff))
191+
return 0
192+
}
129193
_, sz := utf8.DecodeRune(data)
130-
byteOff += uint32(sz)
194+
byteOff += uint64(sz)
131195
data = data[sz:]
132196
left--
133197
}
134198

135-
byteOff -= fileStartByte
136-
return byteOff
199+
if byteOff < uint64(fileStartByte) {
200+
p.panicCorrupt(fmt.Errorf("%s rune offset %d maps to byte offset %d before file start %d", kind, absR, byteOff, fileStartByte))
201+
return 0
202+
}
203+
if byteOff > uint64(fileEndByte) {
204+
p.panicCorrupt(fmt.Errorf("%s rune offset %d maps to byte offset %d after file end %d", kind, absR, byteOff, fileEndByte))
205+
return 0
206+
}
207+
208+
byteOff -= uint64(fileStartByte)
209+
return uint32(byteOff)
137210
}
138211

139212
// fillMatches converts the internal candidateMatch slice into our API's LineMatch.

0 commit comments

Comments
 (0)