From f1deee0e8c5f31d86301fe3ec6554a93da0d7e42 Mon Sep 17 00:00:00 2001 From: Travis Bischel Date: Sat, 31 Mar 2018 21:51:00 -0700 Subject: [PATCH] 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 Run-TryBot: Joe Tsai TryBot-Result: Gobot Gobot --- src/compress/gzip/gzip.go | 10 ++++----- src/compress/gzip/gzip_test.go | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/compress/gzip/gzip.go b/src/compress/gzip/gzip.go index db9750dee2..eaeb185795 100644 --- 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 865c529f55..e16aba1572 100644 --- 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)) + } + } +} -- 2.48.1