compress/gzip: do not count header bytes written in Write

Before, if an underlying writer errored within 10 bytes (plus any gzip
header metadata), a gzip.Write would erroneously report up to 10 bytes
written that were not actually written of the input slice. This is
especially problematic when the input slice is less than 10 bytes.

The error came from counting the 10 header byte write. If writing the
header is completely successful, the 10 bytes written is overridden by
the flate write with the input slice.

This removes counting the 10 required header bytes, and also changes the
return to use zero until the slice is used.

The old Write could return one byte written when it actually was not.
This is difficult to verify because the smallest input slice is one
byte; a test checking that the input slice was the byte written would be
quite involved. Thankfully, gzip's minimum header write is 10 bytes. If
we test that two bytes are not falsely written, we indirectly cover the
one byte case.

Fixes #24625

Change-Id: I1c1f8cd791e0c4cffc22aa8acd95186582c832ba
Reviewed-on: https://go-review.googlesource.com/103861
Reviewed-by: Joe Tsai <joetsai@google.com>
Run-TryBot: Joe Tsai <joetsai@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/compress/gzip/gzip.go b/src/compress/gzip/gzip.go
index db9750dee2278f1a080a792c9b3baebf7be719eb..eaeb1857954282ffd85070f9cea64c43850a5fc3 100644 (file)
--- a/src/compress/gzip/gzip.go
+++ b/src/compress/gzip/gzip.go
@@ -165,26 +165,26 @@ func (z *Writer) Write(p []byte) (int, error) {
                        z.buf[8] = 4
                }
                z.buf[9] = z.OS
-               n, z.err = z.w.Write(z.buf[:10])
+               _, z.err = z.w.Write(z.buf[:10])
                if z.err != nil {
-                       return n, z.err
+                       return 0, z.err
                }
                if z.Extra != nil {
                        z.err = z.writeBytes(z.Extra)
                        if z.err != nil {
-                               return n, z.err
+                               return 0, z.err
                        }
                }
                if z.Name != "" {
                        z.err = z.writeString(z.Name)
                        if z.err != nil {
-                               return n, z.err
+                               return 0, z.err
                        }
                }
                if z.Comment != "" {
                        z.err = z.writeString(z.Comment)
                        if z.err != nil {
-                               return n, z.err
+                               return 0, z.err
                        }
                }
                if z.compressor == nil {

diff --git a/src/compress/gzip/gzip_test.go b/src/compress/gzip/gzip_test.go
index 865c529f55c240bf46238fa03690c38bb8033b93..e16aba1572d190e05c2e38b55c99f504e99904a1 100644 (file)
--- a/src/compress/gzip/gzip_test.go
+++ b/src/compress/gzip/gzip_test.go
@@ -7,6 +7,7 @@ package gzip
 import (
        "bufio"
        "bytes"
+       "io"
        "io/ioutil"
        "reflect"
        "testing"
@@ -233,3 +234,40 @@ func TestWriterReset(t *testing.T) {
                t.Errorf("buf2 %q != original buf of %q", buf2.String(), buf.String())
        }
 }
+
+type limitedWriter struct {
+       N int
+}
+
+func (l *limitedWriter) Write(p []byte) (n int, err error) {
+       if n := l.N; n < len(p) {
+               l.N = 0
+               return n, io.ErrShortWrite
+       }
+       l.N -= len(p)
+       return len(p), nil
+}
+
+// Write should never return more bytes than the input slice.
+func TestLimitedWrite(t *testing.T) {
+       msg := []byte("a")
+
+       for lim := 2; lim < 20; lim++ {
+               z := NewWriter(&limitedWriter{lim})
+               if n, _ := z.Write(msg); n > len(msg) {
+                       t.Errorf("Write() = %d, want %d or less", n, len(msg))
+               }
+
+               z.Reset(&limitedWriter{lim})
+               z.Header = Header{
+                       Comment: "comment",
+                       Extra:   []byte("extra"),
+                       ModTime: time.Now(),
+                       Name:    "name",
+                       OS:      1,
+               }
+               if n, _ := z.Write(msg); n > len(msg) {
+                       t.Errorf("Write() = %d, want %d or less", n, len(msg))
+               }
+       }
+}