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: possibly useless test #28700
Comments
It seems that fix applied for #10956 (that I reviewed and approved) wasn't actually a fix; maybe in theory, but it was never tested in practice. As you say, the example that @dvykov provided doesn't seem to get past the readDirectoryEnd function, so the additional check added to the init method is never reached. I sent a change. |
Change https://golang.org/cl/179757 mentions this issue: |
This removes a special case that was added to fix issue #10956, but that was never actually effective. The code in the test case still fails to read, so perhaps the zip64 support added in CL 6463050 inadvertently caught this particular case. It's possible that the original theorized bug still exists, but I'm not convinced it was ever fixed. Update #28700 Change-Id: I4854de616364510f64a6def30b308686563f8dbb Reviewed-on: https://go-review.googlesource.com/c/go/+/179757 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This removes a special case that was added to fix issue golang#10956, but that was never actually effective. The code in the test case still fails to read, so perhaps the zip64 support added in CL 6463050 inadvertently caught this particular case. It's possible that the original theorized bug still exists, but I'm not convinced it was ever fixed. Update golang#28700 Change-Id: I4854de616364510f64a6def30b308686563f8dbb Reviewed-on: https://go-review.googlesource.com/c/go/+/179757 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This removes a special case that was added to fix issue golang#10956, but that was never actually effective. The code in the test case still fails to read, so perhaps the zip64 support added in CL 6463050 inadvertently caught this particular case. It's possible that the original theorized bug still exists, but I'm not convinced it was ever fixed. Update golang#28700 Change-Id: I4854de616364510f64a6def30b308686563f8dbb Reviewed-on: https://go-review.googlesource.com/c/go/+/179757 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
While playing around with the new vet and its nilness check, I have found an interesting bug. In
src/archive/zip/reader_test.go:985
we have a test for #10956, but the test doesn't actually check anything:It should obviously be
if err != nil && …
here. But when I changed it, the test failed, because it returnedErrFormat
instead of the custom Errorf'd error that was expected. I ran all of the package zip's tests with -cover and the coverage profile shows that the branch in(*Reader).init
is never taken.The text was updated successfully, but these errors were encountered: