kaitai_struct_go_runtime icon indicating copy to clipboard operation
kaitai_struct_go_runtime copied to clipboard

Potential bug in `Stream.Size()`

Open chavacava opened this issue 4 years ago • 2 comments

The current implementation of Stream.Size() is:

// Size returns the number of bytes of the stream.
func (k *Stream) Size() (int64, error) {
	// Go has no internal ReadSeeker function to get current ReadSeeker size,
	// thus we use the following trick.
	// Remember our current position
	curPos, err := k.Pos()
	if err != nil {
		return 0, err
	}
	// Seek to the end of the File object
	_, err = k.Seek(0, io.SeekEnd)
	if err != nil {
		return 0, err
	}
	// Remember position, which is equal to the full length
	fullSize, err := k.Pos()
	if err != nil {
		return fullSize, err
	}
	// Seek back to the current position
	_, err = k.Seek(curPos, io.SeekStart)
	return fullSize, err
}

It seems that if the assignment fullSize, err := k.Pos() returns an error the current position pointer is never set to its original value.

A potential mitigation of the problem could be:

// Size returns the number of bytes of the stream.
func (k *Stream) Size() (int64, error) {
	// Go has no internal ReadSeeker function to get current ReadSeeker size,
	// thus we use the following trick.
	// Remember our current position
	curPos, err := k.Pos()
	if err != nil {
		return 0, err
	}

        // Deferred seek back to the current position
	defer k.Seek(curPos, io.SeekStart)

	// Seek to the end of the File object
	_, err = k.Seek(0, io.SeekEnd)
	if err != nil {
		return 0, err
	}

	// return position, which is equal to the full length
	return k.Pos()
}

chavacava avatar Jan 17 '21 18:01 chavacava

@chavacava Good catch! Can you please open a pull request with the fix?

generalmimon avatar Jan 17 '21 21:01 generalmimon

@generalmimon please check PR #27

chavacava avatar Jan 19 '21 20:01 chavacava