12247: Propagate write errors, don't hide them with "bad checksum".
authorTom Clegg <tclegg@veritasgenetics.com>
Tue, 19 Sep 2017 14:37:43 +0000 (10:37 -0400)
committerTom Clegg <tclegg@veritasgenetics.com>
Tue, 19 Sep 2017 14:37:43 +0000 (10:37 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

sdk/go/keepclient/hashcheck.go
sdk/go/keepclient/hashcheck_test.go

index 4a5c09ba793cdfb9d37609d6b9f4392d7f1c18db..9b13eda771afcf9a3ac523454a7f2163a67864e4 100644 (file)
@@ -43,8 +43,9 @@ func (this HashCheckingReader) Read(p []byte) (n int, err error) {
        return n, err
 }
 
-// WriteTo writes the entire contents of this.Reader to dest.  Returns
-// BadChecksum if the checksum doesn't match.
+// WriteTo writes the entire contents of this.Reader to dest. Returns
+// BadChecksum if writing is successful but the checksum doesn't
+// match.
 func (this HashCheckingReader) WriteTo(dest io.Writer) (written int64, err error) {
        if writeto, ok := this.Reader.(io.WriterTo); ok {
                written, err = writeto.WriteTo(io.MultiWriter(dest, this.Hash))
@@ -52,13 +53,16 @@ func (this HashCheckingReader) WriteTo(dest io.Writer) (written int64, err error
                written, err = io.Copy(io.MultiWriter(dest, this.Hash), this.Reader)
        }
 
-       sum := this.Hash.Sum(make([]byte, 0, this.Hash.Size()))
+       if err != nil {
+               return written, err
+       }
 
+       sum := this.Hash.Sum(make([]byte, 0, this.Hash.Size()))
        if fmt.Sprintf("%x", sum) != this.Check {
-               err = BadChecksum
+               return written, BadChecksum
        }
 
-       return written, err
+       return written, nil
 }
 
 // Close reads all remaining data from the underlying Reader and
index db748ee98ed2b247d37f21b931b03283f5e70bb1..44345afda61c5535ee1bfa1232c3369827f00be5 100644 (file)
@@ -8,9 +8,10 @@ import (
        "bytes"
        "crypto/md5"
        "fmt"
-       . "gopkg.in/check.v1"
        "io"
        "io/ioutil"
+
+       . "gopkg.in/check.v1"
 )
 
 type HashcheckSuiteSuite struct{}
@@ -86,4 +87,17 @@ func (h *HashcheckSuiteSuite) TestWriteTo(c *C) {
                c.Check(err, Equals, BadChecksum)
                <-done
        }
+
+       // If WriteTo stops early due to a write error, return the
+       // write error (not "bad checksum").
+       {
+               input := bytes.NewBuffer(make([]byte, 1<<26))
+               hcr := HashCheckingReader{input, md5.New(), hash}
+               r, w := io.Pipe()
+               r.Close()
+               n, err := hcr.WriteTo(w)
+               c.Check(n, Equals, int64(0))
+               c.Check(err, NotNil)
+               c.Check(err, Not(Equals), BadChecksum)
+       }
 }