Skip to content

Commit

Permalink
[bug] - Handle empty reader case in newFileReader (#2854)
Browse files Browse the repository at this point in the history
* Correclty handle empty files

* fix

* fix test
  • Loading branch information
ahrav committed May 16, 2024
1 parent ead9dd5 commit 2db06f0
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 11 deletions.
13 changes: 11 additions & 2 deletions pkg/handlers/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const (

var (
maxDepth = 5
maxSize = 250 * 1024 * 1024 // 250 MB
maxSize = 2 << 30 // 2 GB
maxTimeout = time.Duration(30) * time.Second
)

Expand Down Expand Up @@ -101,6 +101,10 @@ func (h *archiveHandler) openArchive(ctx logContext.Context, depth int, reader f

rdr, err := newFileReader(compReader)
if err != nil {
if errors.Is(err, ErrEmptyReader) {
ctx.Logger().V(5).Info("empty reader, skipping file")
return nil
}
return fmt.Errorf("error creating custom reader: %w", err)
}
defer rdr.Close()
Expand Down Expand Up @@ -146,7 +150,8 @@ func (h *archiveHandler) extractorHandler(archiveChan chan []byte) func(context.

fileSize := file.Size()
if int(fileSize) > maxSize {
lCtx.Logger().V(3).Info("skipping file due to size")
lCtx.Logger().V(3).Info("skipping file due to size", "size", fileSize)
h.metrics.incFilesSkipped()
return nil
}

Expand Down Expand Up @@ -180,6 +185,10 @@ func (h *archiveHandler) extractorHandler(archiveChan chan []byte) func(context.

rdr, err := newFileReader(f)
if err != nil {
if errors.Is(err, ErrEmptyReader) {
lCtx.Logger().V(5).Info("empty reader, skipping file")
return nil
}
return fmt.Errorf("error creating custom reader: %w", err)
}
defer rdr.Close()
Expand Down
11 changes: 11 additions & 0 deletions pkg/handlers/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ type fileReader struct {
isGenericArchive bool
}

var ErrEmptyReader = errors.New("reader is empty")

func newFileReader(r io.ReadCloser) (fileReader, error) {
defer r.Close()

Expand All @@ -57,6 +59,11 @@ func newFileReader(r io.ReadCloser) (fileReader, error) {
}
}()

// Check if the reader is empty.
if rdr.Size() == 0 {
return reader, ErrEmptyReader
}

format, arReader, err := archiver.Identify("", rdr)
switch {
case err == nil: // Archive detected
Expand Down Expand Up @@ -172,6 +179,10 @@ func HandleFile(

rdr, err := newFileReader(reader)
if err != nil {
if errors.Is(err, ErrEmptyReader) {
ctx.Logger().V(5).Info("empty reader, skipping file")
return nil
}
return fmt.Errorf("error creating custom reader: %w", err)
}
defer rdr.Close()
Expand Down
19 changes: 19 additions & 0 deletions pkg/handlers/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,22 @@ func TestHandleFileNonArchive(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, wantChunkCount, len(reporter.Ch))
}

func TestExtractTarContentWithEmptyFile(t *testing.T) {
file, err := os.Open("testdata/testdir.zip")
assert.Nil(t, err)

chunkCh := make(chan *sources.Chunk)
go func() {
defer close(chunkCh)
err := HandleFile(logContext.Background(), file, &sources.Chunk{}, sources.ChanReporter{Ch: chunkCh})
assert.NoError(t, err)
}()

wantCount := 4
count := 0
for range chunkCh {
count++
}
assert.Equal(t, wantCount, count)
}
Binary file added pkg/handlers/testdata/testdir.zip
Binary file not shown.
3 changes: 3 additions & 0 deletions pkg/readers/bufferedfilereader.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,6 @@ func (b *BufferedFileReader) ReadAt(p []byte, off int64) (n int, err error) {
}
return b.reader.Read(p)
}

// Size returns the total size of the data stored in the BufferedFileReader.
func (b *BufferedFileReader) Size() int { return b.bufWriter.Len() }
8 changes: 2 additions & 6 deletions pkg/writers/buffered_file_writer/bufferedfilewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,10 @@ func New(opts ...Option) *BufferedFileWriter {
// NewFromReader creates a new instance of BufferedFileWriter and writes the content from the provided reader to the writer.
func NewFromReader(r io.Reader, opts ...Option) (*BufferedFileWriter, error) {
writer := New(opts...)
if _, err := io.Copy(writer, r); err != nil {
if _, err := io.Copy(writer, r); err != nil && !errors.Is(err, io.EOF) {
return nil, fmt.Errorf("error writing to buffered file writer: %w", err)
}

if writer.buf == nil {
return nil, fmt.Errorf("reader was empty; no data written to buffered file writer")
}

return writer, nil
}

Expand Down Expand Up @@ -271,7 +267,7 @@ func (w *BufferedFileWriter) ReadSeekCloser() (io.ReadSeekCloser, error) {
}

if w.buf == nil {
return nil, fmt.Errorf("BufferedFileWriter has not buffer data to read")
return nil, nil
}

// Data is in memory.
Expand Down
9 changes: 6 additions & 3 deletions pkg/writers/buffered_file_writer/bufferedfilewriter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,8 @@ func TestNewFromReader(t *testing.T) {
wantData: "hello world",
},
{
name: "Empty reader",
reader: strings.NewReader(""),
wantErr: true,
name: "Empty reader",
reader: strings.NewReader(""),
},
{
name: "Error reader",
Expand Down Expand Up @@ -550,6 +549,10 @@ func TestNewFromReader(t *testing.T) {
return
}
assert.NoError(t, err)

if rdr == nil {
return
}
defer rdr.Close()

_, err = b.ReadFrom(rdr)
Expand Down

0 comments on commit 2db06f0

Please sign in to comment.