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: (*File).Open is inconsistent with (*Reader).Open for directory entries #58165

Open
bcmills opened this issue Jan 31, 2023 · 4 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Jan 31, 2023

What version of Go are you using (go version)?

https://pkg.go.dev/archive/zip@go1.20rc3

Does this issue reproduce with the latest release?

Yes

What did you do?

(https://go.dev/play/p/NFjouX0xrQk?v=gotip)

  1. Create a zip archive containing a directory entry.
  2. Open a zip.Reader for that archive.
  3. Open the zip.File for the directory entry using (*zip.Reader).Open and Read it.
  4. Open the zip.File for the directory by calling (*zip.File).Open and Read it.

What did you expect to see?

The same contents (and same error code) regardless of how the directory entry was opened.

What did you see instead?

The reader opened via (*zip.Reader).Open returns a non-nil error like read dir: is a directory.
The reader opened via (*zip.File).Open returns 0, io.EOF.

(attn @dsnet @bradfitz; CC @neild)

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 31, 2023
@bcmills bcmills added this to the Backlog milestone Jan 31, 2023
@bcmills
Copy link
Contributor Author

bcmills commented Jan 31, 2023

(Found while investigating #53448.)

@bcmills
Copy link
Contributor Author

bcmills commented Jan 31, 2023

I'm not sure whether this can be addressed without breaking something.

(Arguably the lack of error return from the Read call from (*zip.File).Open is a bug, but is the bug too entrenched at this point to fix?)

@rsc
Copy link
Contributor

rsc commented Jan 31, 2023

It's a clear bug for the zip.File.Open result to return io.EOF from a read on a directory. We should fix that. This is the kind of thing that can come with a GODEBUG to ease any transition (#56986).

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 31, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 31, 2023
@rsc
Copy link
Contributor

rsc commented Jan 31, 2023

Note that we should update HashZip (#53448) to avoid the error before or at the same time as making this change, so that module archives that work today keep hashing to the same value rather than failing to hash at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants