Skip to content

Commit

Permalink
bindings/go: Fix bug with incomplete reads
Browse files Browse the repository at this point in the history
Using Reader.read doesn't guarantee that the buffer passed
will be filled if enough data is available, and the buffer
has enough space. This meant that sometimes an initial Read
call would not suffice for large Document data.

Use ReadAtLeast to avoid that.
  • Loading branch information
varungandhi-src committed Jun 23, 2023
1 parent 9a03eb7 commit 5ce20e1
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
2 changes: 1 addition & 1 deletion bindings/go/scip/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (pi *IndexVisitor) ParseStreaming(r io.Reader) error {
}
// Keep going when len == 0 instead of short-circuiting to preserve empty sub-messages
if dataLen > 0 {
numRead, err := r.Read(dataBuf)
numRead, err := io.ReadAtLeast(r, dataBuf, dataLen)
if err != nil {
return errors.Wrapf(err, "failed to read data for %s", indexFieldName(fieldNumber))
}
Expand Down
30 changes: 30 additions & 0 deletions bindings/go/scip/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package scip

import (
"bytes"
"compress/gzip"
"os"
"regexp"
"testing"

Expand Down Expand Up @@ -46,3 +48,31 @@ func TestFuzz(t *testing.T) {
}
}
}

func TestLargeDocuments(t *testing.T) {
// Copied from the Sourcegraph monorepo, which triggered a bug
// where Reader.read() didn't actually fill a buffer completely,
// due to the presence of large documents.
gzipped, err := os.Open("./testdata/index1.scip.gz")
if err != nil {
t.Fatalf("unexpected error reading test file: %s", err)
}
reader, err := gzip.NewReader(gzipped)
if err != nil {
t.Fatalf("unexpected error unzipping test file: %s", err)
}

parsedIndex := Index{}

indexVisitor := IndexVisitor{func(metadata *Metadata) {
parsedIndex.Metadata = metadata
}, func(document *Document) {
parsedIndex.Documents = append(parsedIndex.Documents, document)
}, func(extSym *SymbolInformation) {
parsedIndex.ExternalSymbols = append(parsedIndex.ExternalSymbols, extSym)
}}

if err := indexVisitor.ParseStreaming(reader); err != nil {
t.Fatalf("got error parsing index %v", err)
}
}
Binary file added bindings/go/scip/testdata/index1.scip.gz
Binary file not shown.

0 comments on commit 5ce20e1

Please sign in to comment.