NewBlob(): Reader.Read() can return both data and error.

Fixes #264

types/blob.go

@@ -33,7 +33,7 @@ func NewBlob(r io.Reader) (Blob, error) {
 	for {
 		buf := bytes.Buffer{}
 		n, err := copyChunk(&buf, r)
-		if err != nil {
+		if err != nil && err != io.EOF {
 			return nil, err
 		}
 
@@ -46,6 +46,10 @@ func NewBlob(r io.Reader) (Blob, error) {
 		offsets = append(offsets, length)
 		blob = newBlobLeaf(buf.Bytes())
 		blobs = append(blobs, futureFromValue(blob))
+
+		if err == io.EOF {
+			break
+		}
 	}
 
 	if length == 0 {
@@ -73,23 +77,23 @@ func copyChunk(dst io.Writer, src io.Reader) (n uint64, err error) {
 	p := []byte{0}
 
 	for {
-		_, err = src.Read(p)
-		if err != nil {
-			if err == io.EOF {
-				return n, nil
-			}
-			return
+		l, rerr := src.Read(p)
+		n += uint64(l)
+
+		// io.Reader can return data and error at the same time, so we need to write before considering the error.
+		h.Write(p[:l])
+		_, werr := dst.Write(p[:l])
+
+		if rerr != nil {
+			return n, rerr
 		}
 
-		h.Write(p)
-		_, err = dst.Write(p)
-		if err != nil {
-			return
+		if werr != nil {
+			return n, werr
 		}
-		n++
 
 		if h.Sum32()&blobPattern == blobPattern {
-			return
+			return n, nil
 		}
 	}
 }

types/blob_test.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	"bytes"
+	"io"
 	"testing"
 
 	"github.com/attic-labs/noms/Godeps/_workspace/src/github.com/stretchr/testify/assert"
@@ -40,3 +41,43 @@ func TestBlobEquals(t *testing.T) {
 	AssertSymNe(assert, b2, b3)
 	AssertSymNe(assert, b1, Int32(1))
 }
+
+type testReader struct {
+	readCount int
+	buf       *bytes.Buffer
+}
+
+func (r *testReader) Read(p []byte) (n int, err error) {
+	r.readCount++
+
+	switch r.readCount {
+	case 1:
+		for i := 0; i < len(p); i++ {
+			p[i] = 0x01
+		}
+		io.Copy(r.buf, bytes.NewReader(p))
+		return len(p), nil
+	case 2:
+		p[0] = 0x02
+		r.buf.WriteByte(p[0])
+		return 1, io.EOF
+	default:
+		return 0, io.EOF
+	}
+}
+
+func TestBlobFromReaderThatReturnsDataAndError(t *testing.T) {
+	// See issue #264.
+	// This tests the case of building a Blob from a reader who returns both data and an error for the final Read() call.
+	assert := assert.New(t)
+	tr := &testReader{buf: &bytes.Buffer{}}
+
+	b, err := NewBlob(tr)
+	assert.NoError(err)
+
+	actual := &bytes.Buffer{}
+	io.Copy(actual, b.Reader())
+
+	assert.True(bytes.Equal(actual.Bytes(), tr.buf.Bytes()))
+	assert.Equal(byte(2), actual.Bytes()[len(actual.Bytes())-1])
+}