]> Cypherpunks repositories - gostls13.git/commitdiff
archive/tar: bug fixes.
authorChristopher Wedgwood <cw@f00f.org>
Mon, 14 Dec 2009 19:35:02 +0000 (11:35 -0800)
committerRuss Cox <rsc@golang.org>
Mon, 14 Dec 2009 19:35:02 +0000 (11:35 -0800)
1. If all data is exhausted using Read then a following Next will
   fail as if it saw EOF.  (Test case added.)
2. Seeking isn't always possible (i.e. sockets and pipes).  Fallback
   to read.  (Test case added.)
3. Fix to readHeader (cleaner fix pointed out by rsc).
   (TestReader modified.)
4. When Read has consumed all the data, don't try to read 0 bytes from reader.
   In cases where tr.nb is zero we attempt to read zero bytes and thus
   never see an EOF (this is most easily seen when the 'tar source' is
   something like bytes.Buffer{} as opposed to os.File).
5. If write is used to the point of ErrWriteTooLong, allow additional file entries.
6. Make close work as expected.  That is any further Write or
   WriteHeader attempts will result in ErrWriteAfterClose.
Fixes #419.

R=rsc, dsymonds1
https://golang.org/cl/162062

src/pkg/archive/tar/reader.go
src/pkg/archive/tar/reader_test.go
src/pkg/archive/tar/writer.go

index cc2d89909f6e41f243efe5d3172a5ec5c4302a06..50cda624bdf12c1540daf43123cd28d1a37e8f11 100644 (file)
@@ -93,13 +93,13 @@ func (ignoreWriter) Write(b []byte) (n int, err os.Error) {
 // Skip any unread bytes in the existing file entry, as well as any alignment padding.
 func (tr *Reader) skipUnread() {
        nr := tr.nb + tr.pad;   // number of bytes to skip
-
+       tr.nb, tr.pad = 0, 0;
        if sr, ok := tr.r.(io.Seeker); ok {
-               _, tr.err = sr.Seek(nr, 1)
-       } else {
-               _, tr.err = io.Copyn(ignoreWriter{}, tr.r, nr)
+               if _, err := sr.Seek(nr, 1); err == nil {
+                       return
+               }
        }
-       tr.nb, tr.pad = 0, 0;
+       _, tr.err = io.Copyn(ignoreWriter{}, tr.r, nr);
 }
 
 func (tr *Reader) verifyChecksum(header []byte) bool {
@@ -123,8 +123,10 @@ func (tr *Reader) readHeader() *Header {
                if _, tr.err = io.ReadFull(tr.r, header); tr.err != nil {
                        return nil
                }
-               if !bytes.Equal(header, zeroBlock[0:blockSize]) {
-                       tr.err = HeaderError
+               if bytes.Equal(header, zeroBlock[0:blockSize]) {
+                       tr.err = os.EOF
+               } else {
+                       tr.err = HeaderError    // zero block and then non-zero block
                }
                return nil;
        }
@@ -202,14 +204,23 @@ func (tr *Reader) readHeader() *Header {
 }
 
 // Read reads from the current entry in the tar archive.
-// It returns 0, nil when it reaches the end of that entry,
+// It returns 0, os.EOF when it reaches the end of that entry,
 // until Next is called to advance to the next entry.
 func (tr *Reader) Read(b []byte) (n int, err os.Error) {
+       if tr.nb == 0 {
+               // file consumed
+               return 0, os.EOF
+       }
+
        if int64(len(b)) > tr.nb {
                b = b[0:tr.nb]
        }
        n, err = tr.r.Read(b);
        tr.nb -= int64(n);
+
+       if err == os.EOF && tr.nb > 0 {
+               err = io.ErrUnexpectedEOF
+       }
        tr.err = err;
        return;
 }
index 0e6f40082bd5b7a33436b159c939797dd167781d..f5a77dd385056b705292e87132fd273bf71bdf23 100644 (file)
@@ -6,6 +6,8 @@ package tar
 
 import (
        "bytes";
+       "crypto/md5";
+       "fmt";
        "io";
        "os";
        "reflect";
@@ -16,36 +18,43 @@ import (
 type untarTest struct {
        file    string;
        headers []*Header;
+       cksums  []string;
 }
 
-var untarTests = []*untarTest{
-       &untarTest{
-               file: "testdata/gnu.tar",
-               headers: []*Header{
-                       &Header{
-                               Name: "small.txt",
-                               Mode: 0640,
-                               Uid: 73025,
-                               Gid: 5000,
-                               Size: 5,
-                               Mtime: 1244428340,
-                               Typeflag: '0',
-                               Uname: "dsymonds",
-                               Gname: "eng",
-                       },
-                       &Header{
-                               Name: "small2.txt",
-                               Mode: 0640,
-                               Uid: 73025,
-                               Gid: 5000,
-                               Size: 11,
-                               Mtime: 1244436044,
-                               Typeflag: '0',
-                               Uname: "dsymonds",
-                               Gname: "eng",
-                       },
+var gnuTarTest = &untarTest{
+       file: "testdata/gnu.tar",
+       headers: []*Header{
+               &Header{
+                       Name: "small.txt",
+                       Mode: 0640,
+                       Uid: 73025,
+                       Gid: 5000,
+                       Size: 5,
+                       Mtime: 1244428340,
+                       Typeflag: '0',
+                       Uname: "dsymonds",
+                       Gname: "eng",
                },
+               &Header{
+                       Name: "small2.txt",
+                       Mode: 0640,
+                       Uid: 73025,
+                       Gid: 5000,
+                       Size: 11,
+                       Mtime: 1244436044,
+                       Typeflag: '0',
+                       Uname: "dsymonds",
+                       Gname: "eng",
+               },
+       },
+       cksums: []string{
+               "e38b27eaccb4391bdec553a7f3ae6b2f",
+               "c65bd2e50a56a2138bf1716f2fd56fe9",
        },
+}
+
+var untarTests = []*untarTest{
+       gnuTarTest,
        &untarTest{
                file: "testdata/star.tar",
                headers: []*Header{
@@ -124,6 +133,9 @@ testLoop:
                        }
                }
                hdr, err := tr.Next();
+               if err == os.EOF {
+                       break
+               }
                if hdr != nil || err != nil {
                        t.Errorf("test %d: Unexpected entry or error: hdr=%v err=%v", i, err)
                }
@@ -166,3 +178,98 @@ func TestPartialRead(t *testing.T) {
                t.Errorf("Contents = %v, want %v", buf, expected)
        }
 }
+
+
+func TestIncrementalRead(t *testing.T) {
+       test := gnuTarTest;
+       f, err := os.Open(test.file, os.O_RDONLY, 0444);
+       if err != nil {
+               t.Fatalf("Unexpected error: %v", err)
+       }
+       defer f.Close();
+
+       tr := NewReader(f);
+
+       headers := test.headers;
+       cksums := test.cksums;
+       nread := 0;
+
+       // loop over all files
+       for ; ; nread++ {
+               hdr, err := tr.Next();
+               if hdr == nil || err == os.EOF {
+                       break
+               }
+
+               // check the header
+               if !reflect.DeepEqual(hdr, headers[nread]) {
+                       t.Errorf("Incorrect header:\nhave %+v\nwant %+v",
+                               *hdr, headers[nread])
+               }
+
+               // read file contents in little chunks EOF,
+               // checksumming all the way
+               h := md5.New();
+               rdbuf := make([]uint8, 8);
+               for {
+                       nr, err := tr.Read(rdbuf);
+                       if err == os.EOF {
+                               break
+                       }
+                       if err != nil {
+                               t.Errorf("Read: unexpected error %v\n", err);
+                               break;
+                       }
+                       h.Write(rdbuf[0:nr]);
+               }
+               // verify checksum
+               have := fmt.Sprintf("%x", h.Sum());
+               want := cksums[nread];
+               if want != have {
+                       t.Errorf("Bad checksum on file %s:\nhave %+v\nwant %+v", hdr.Name, have, want)
+               }
+       }
+       if nread != len(headers) {
+               t.Errorf("Didn't process all files\nexpected: %d\nprocessed %d\n", len(headers), nread)
+       }
+}
+
+func TestNonSeekable(t *testing.T) {
+       test := gnuTarTest;
+       f, err := os.Open(test.file, os.O_RDONLY, 0444);
+       if err != nil {
+               t.Fatalf("Unexpected error: %v", err)
+       }
+       defer f.Close();
+
+       // pipe the data in
+       r, w, err := os.Pipe();
+       if err != nil {
+               t.Fatalf("Unexpected error %s", err)
+       }
+       go func() {
+               rdbuf := make([]uint8, 1<<16);
+               for {
+                       nr, err := f.Read(rdbuf);
+                       w.Write(rdbuf[0:nr]);
+                       if err == os.EOF {
+                               break
+                       }
+               }
+               w.Close();
+       }();
+
+       tr := NewReader(r);
+       nread := 0;
+
+       for ; ; nread++ {
+               hdr, err := tr.Next();
+               if hdr == nil || err == os.EOF {
+                       break
+               }
+       }
+
+       if nread != len(test.headers) {
+               t.Errorf("Didn't process all files\nexpected: %d\nprocessed %d\n", len(test.headers), nread)
+       }
+}
index 6bb4acdf4031b2340e5ac14be6a05b0e3910aba3..f3ce84afa413a9ba8bb0f5a5996f427812c8881e 100644 (file)
@@ -15,8 +15,9 @@ import (
 )
 
 var (
-       ErrWriteTooLong = os.NewError("write too long");
-       ErrFieldTooLong = os.NewError("header field too long");
+       ErrWriteTooLong         = os.NewError("write too long");
+       ErrFieldTooLong         = os.NewError("header field too long");
+       ErrWriteAfterClose      = os.NewError("write after close");
 )
 
 // A Writer provides sequential writing of a tar archive in POSIX.1 format.
@@ -108,7 +109,11 @@ func (tw *Writer) numeric(b []byte, x int64) {
 
 // WriteHeader writes hdr and prepares to accept the file's contents.
 // WriteHeader calls Flush if it is not the first header.
+// Calling after a Close will return ErrWriteAfterClose.
 func (tw *Writer) WriteHeader(hdr *Header) os.Error {
+       if tw.closed {
+               return ErrWriteAfterClose
+       }
        if tw.err == nil {
                tw.Flush()
        }
@@ -164,6 +169,10 @@ func (tw *Writer) WriteHeader(hdr *Header) os.Error {
 // Write returns the error ErrWriteTooLong if more than
 // hdr.Size bytes are written after WriteHeader.
 func (tw *Writer) Write(b []byte) (n int, err os.Error) {
+       if tw.closed {
+               err = ErrWriteTooLong;
+               return;
+       }
        overwrite := false;
        if int64(len(b)) > tw.nb {
                b = b[0:tw.nb];
@@ -172,12 +181,15 @@ func (tw *Writer) Write(b []byte) (n int, err os.Error) {
        n, err = tw.w.Write(b);
        tw.nb -= int64(n);
        if err == nil && overwrite {
-               err = ErrWriteTooLong
+               err = ErrWriteTooLong;
+               return;
        }
        tw.err = err;
        return;
 }
 
+// Close closes the tar archive, flushing any unwritten
+// data to the underlying writer.
 func (tw *Writer) Close() os.Error {
        if tw.err != nil || tw.closed {
                return tw.err