diff --git a/crypt/cipher.go b/crypt/cipher.go index 85ee101f1..cd3cd9d3a 100644 --- a/crypt/cipher.go +++ b/crypt/cipher.go @@ -449,14 +449,13 @@ func (fh *encrypter) Read(p []byte) (n int, err error) { // FIXME should overlap the reads with a go-routine and 2 buffers? readBuf := fh.readBuf[:blockDataSize] n, err = io.ReadFull(fh.in, readBuf) - if err == io.EOF { - // ReadFull only returns n=0 and EOF - return fh.finish(io.EOF) - } else if err == io.ErrUnexpectedEOF { - // Next read will return EOF - } else if err != nil { + if n == 0 { + // err can't be nil since: + // n == len(buf) if and only if err == nil. return fh.finish(err) } + // possibly err != nil here, but we will process the + // data and the next call to ReadFull will return 0, err // Write nonce to start of block copy(fh.buf, fh.nonce[:]) // Encrypt the block using the nonce @@ -563,26 +562,27 @@ func (fh *decrypter) fillBuffer() (err error) { // FIXME should overlap the reads with a go-routine and 2 buffers? readBuf := fh.readBuf n, err := io.ReadFull(fh.rc, readBuf) - if err == io.EOF { - // ReadFull only returns n=0 and EOF - return io.EOF - } else if err == io.ErrUnexpectedEOF { - // Next read will return EOF - } else if err != nil { + if n == 0 { + // err can't be nil since: + // n == len(buf) if and only if err == nil. return err } + // possibly err != nil here, but we will process the data and + // the next call to ReadFull will return 0, err + // Check header + 1 byte exists if n <= blockHeaderSize { + if err != nil { + return err // return pending error as it is likely more accurate + } return ErrorEncryptedFileBadHeader } // Decrypt the block using the nonce block := fh.buf _, ok := secretbox.Open(block[:0], readBuf[:n], fh.nonce.pointer(), &fh.c.dataKey) if !ok { - // if block wouldn't decode and got unexpected EOF - // then return that as it is probably a better error if err != nil { - return err + return err // return pending error as it is likely more accurate } return ErrorEncryptedBadBlock } diff --git a/crypt/cipher_test.go b/crypt/cipher_test.go index 0d4414bdd..2face09fa 100644 --- a/crypt/cipher_test.go +++ b/crypt/cipher_test.go @@ -788,6 +788,21 @@ func TestNewEncrypter(t *testing.T) { } +// Test the stream returning 0, io.ErrUnexpectedEOF - this used to +// cause a fatal loop +func TestNewEncrypterErrUnexpectedEOF(t *testing.T) { + c, err := newCipher(NameEncryptionStandard, "", "") + assert.NoError(t, err) + + in := &errorReader{io.ErrUnexpectedEOF} + fh, err := c.newEncrypter(in) + assert.NoError(t, err) + + n, err := io.CopyN(ioutil.Discard, fh, 1E6) + assert.Equal(t, io.ErrUnexpectedEOF, err) + assert.Equal(t, int64(32), n) +} + type errorReader struct { err error } @@ -854,6 +869,23 @@ func TestNewDecrypter(t *testing.T) { } } +// Test the stream returning 0, io.ErrUnexpectedEOF +func TestNewDecrypterErrUnexpectedEOF(t *testing.T) { + c, err := newCipher(NameEncryptionStandard, "", "") + assert.NoError(t, err) + + in2 := &errorReader{io.ErrUnexpectedEOF} + in1 := bytes.NewBuffer(file16) + in := ioutil.NopCloser(io.MultiReader(in1, in2)) + + fh, err := c.newDecrypter(in) + assert.NoError(t, err) + + n, err := io.CopyN(ioutil.Discard, fh, 1E6) + assert.Equal(t, io.ErrUnexpectedEOF, err) + assert.Equal(t, int64(16), n) +} + func TestNewDecrypterSeek(t *testing.T) { c, err := newCipher(NameEncryptionStandard, "", "") assert.NoError(t, err) @@ -936,8 +968,6 @@ func TestDecrypterRead(t *testing.T) { case i == fileHeaderSize: // This would normally produce an error *except* on the first block expectedErr = nil - case i <= fileHeaderSize+blockHeaderSize: - expectedErr = ErrorEncryptedFileBadHeader default: expectedErr = io.ErrUnexpectedEOF }