Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

archive/zip: Error zip reader test #7292

Closed
gopherbot opened this issue Feb 9, 2014 · 4 comments
Closed

archive/zip: Error zip reader test #7292

gopherbot opened this issue Feb 9, 2014 · 4 comments
Assignees

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 9, 2014

by iacob.campia:

By curiosity I was looking over the source code and spotted an error in the
/archive/zip/reader_test.go that when fixed, the test fails.

https://code.google.com/p/go/source/browse/src/pkg/archive/zip/reader_test.go#358


340:  func readTestFile(t *testing.T, zt ZipTest, ft ZipTestFile, f *File) {
       // ....

358:    size0 := f.UncompressedSize

        var b bytes.Buffer
        r, err := f.Open()
        if err != nil {
                t.Errorf("%s: %v", zt.Name, err)
                return
        }

367:    if size1 := f.UncompressedSize; size0 != size1 {
                t.Errorf("file %q changed f.UncompressedSize from %d to %d", f.Name, size0, size1)
        }

As seen in there, first it saves UncompresedSize value in size0 then compares it with
itself as size1 and not to the length of the uncompressed bytes.

If you fix it, the test will fail with:

--- FAIL: TestReader (0.01 seconds)
        reader_test.go:367: file "README" changed f.UncompressedSize from 4294967295 to 36

Yhe value of UncompresedSize is actually 4294967295 bytes and of the uncompressed data
36 bytes.
@adg
Copy link
Contributor

@adg adg commented Feb 9, 2014

Comment 1:

I don't see the error; the purpose of the test is to check that the UncompressedSize
field does not change after f.Open is called. This is because first UncompressedSize is
first populated by the directory header (at the end of the zip file), and then updated
as Open reads the actual file header (present just before the file body).
What's your "fix"?

Owner changed to @adg.

Status changed to WaitingForReply.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Feb 10, 2014

Comment 2 by iacob.campia:

f.Open() doesn't touch anything, it just calls flate decompress on the data.
I thought the purpose of the test was that the value of the UncompressedSize as written
in the headers is the same as the actual size of the uncompressed data.
The fix would be to move the check after the _, err = io.Copy(&b, r) and compare len(b)
with f.size() witch returns UncompresedSize64 if > 0 or UncompresedSize if not. and not
UncompressedSize which is useless for 64 archives because it contains 0xFFFFFF. This
test fails on zip64.zip
@adg
Copy link
Contributor

@adg adg commented Feb 11, 2014

Comment 3:

Ah. Originally, f.Open did re-read the header, but it seems it doesn't do that now.
The failure message you're seeing is for tests of ZIP64 files, which populates the
UncompressedSize64 field instead.
Fix: https://golang.org/cl/61650046

Status changed to Started.

@adg
Copy link
Contributor

@adg adg commented Feb 11, 2014

Comment 4:

This issue was closed by revision 413e28d.

Status changed to Fixed.

@gopherbot gopherbot added the fixed label Feb 11, 2014
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.