archive/zip: use base offset 0 if it has a valid entry

In CL 408734 we introduced a fall back to base offset 0 if reading a
directory entry at the computed base offset failed. We have now found
a file in the wild for which the computed base offset is incorrect,
but happens to refer to a valid directory entry. In this CL, we change
the fallback such that if the first directory header relative to base
offset 0 is valid, we just use base offset 0.

Change-Id: Ia9ace20c1065d1f651035f16f7d91d741ab1dbf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/461598
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>

diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go
index c29837836b7cf6cc26b6c22badf994bd2b171cf4..3e96d0ecc9bd8cf963b417e6a1ca1eba2404ad33 100644 (file)
--- a/src/archive/zip/reader.go
+++ b/src/archive/zip/reader.go
@@ -151,20 +151,6 @@ func (z *Reader) init(r io.ReaderAt, size int64) error {
        for {
                f := &File{zip: z, zipr: r}
                err = readDirectoryHeader(f, buf)
-
-               // For compatibility with other zip programs,
-               // if we have a non-zero base offset and can't read
-               // the first directory header, try again with a zero
-               // base offset.
-               if err == ErrFormat && z.baseOffset != 0 && len(z.File) == 0 {
-                       z.baseOffset = 0
-                       if _, err = rs.Seek(int64(end.directoryOffset), io.SeekStart); err != nil {
-                               return err
-                       }
-                       buf.Reset(rs)
-                       continue
-               }
-
                if err == ErrFormat || err == io.ErrUnexpectedEOF {
                        break
                }
@@ -627,6 +613,20 @@ func readDirectoryEnd(r io.ReaderAt, size int64) (dir *directoryEnd, baseOffset
        if o := baseOffset + int64(d.directoryOffset); o < 0 || o >= size {
                return nil, 0, ErrFormat
        }
+
+       // If the directory end data tells us to use a non-zero baseOffset,
+       // but we would find a valid directory entry if we assume that the
+       // baseOffset is 0, then just use a baseOffset of 0.
+       // We've seen files in which the directory end data gives us
+       // an incorrect baseOffset.
+       if baseOffset > 0 {
+               off := int64(d.directoryOffset)
+               rs := io.NewSectionReader(r, off, size-off)
+               if readDirectoryHeader(&File{}, rs) == nil {
+                       baseOffset = 0
+               }
+       }
+
        return d, baseOffset, nil
 }
 

diff --git a/src/archive/zip/reader_test.go b/src/archive/zip/reader_test.go
index 1594b2648ef2d87eb343681f34e38a36d1017d92..70ad260cc5e0ed976238aec235ce5db0c6930024 100644 (file)
--- a/src/archive/zip/reader_test.go
+++ b/src/archive/zip/reader_test.go
@@ -126,6 +126,24 @@ var tests = []ZipTest{
                        },
                },
        },
+       {
+               Name:    "test-badbase.zip",
+               Comment: "This is a zipfile comment.",
+               File: []ZipTestFile{
+                       {
+                               Name:     "test.txt",
+                               Content:  []byte("This is a test text file.\n"),
+                               Modified: time.Date(2010, 9, 5, 12, 12, 1, 0, timeZone(+10*time.Hour)),
+                               Mode:     0644,
+                       },
+                       {
+                               Name:     "gophercolor16x16.png",
+                               File:     "gophercolor16x16.png",
+                               Modified: time.Date(2010, 9, 5, 15, 52, 58, 0, timeZone(+10*time.Hour)),
+                               Mode:     0644,
+                       },
+               },
+       },
        {
                Name:   "r.zip",
                Source: returnRecursiveZip,

diff --git a/src/archive/zip/testdata/test-badbase.zip b/src/archive/zip/testdata/test-badbase.zip
new file mode 100644 (file)
index 0000000..245a62c
Binary files /dev/null and b/src/archive/zip/testdata/test-badbase.zip differ