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.

Sign up for GitHub

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

Jump to bottom

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
Labels
FrozenDueToAge

Comments

@gopherbot
Copy link

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 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

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 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 commented Feb 11, 2014

Comment 4:

This issue was closed by revision 413e28d.

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
@rsc rsc unassigned adg Jun 23, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@adg @gopherbot