-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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: FileInfoHeader does not handle directories #24082
Comments
@dsnet Please update milestone. Thanks. |
Change https://golang.org/cl/124155 mentions this issue: |
We should fix this. I found several cases where users are explicitly doing something like this: fh, err := zip.FileInfoHeader(fi)
if err != nil {
return err
}
if fi.IsDir() {
fh.Name += "/"
} Also, per CL/124155, the function should avoid setting the size for directories. EDIT: Will have to think about what happens with existing code, since this will result in "/" appended twice. |
Several adjustments: 1) When encoding the FileHeader for a directory, explicitly set all of the sizes to zero regardless of their prior values. These values are currently populated by FileInfoHeader as it calls os.FileInfo.Size regardless of whether the file is a directory or not. We avoid fixing FileInfoHeader now as it is too late in the release cycle (see #24082). We silently adjust slightly wrong FileHeader fields as opposed to returning an error because the CreateHeader method already does such mutations (e.g., for UTF-8 detection, data descriptor, etc). 2) Have dirWriter.Write only return an error if some number of bytes are written. Some code still call Write for both normal files and directories, but just pass an empty []byte to Write for directories. Change-Id: I85492a31356107fcf76dc89ceb00a28853754289 Reviewed-on: https://go-review.googlesource.com/124155 Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@dsnet Is anything going to happen here for 1.12? |
This change has a risk of breaking people. Too late for 1.12. We'll submit early 1.13. |
Consider the following:
This currently prints:
However, ZIP treats directories as files with a trailing "/" in the name, which
FileInfoHeader
should automatically append ifos.FileInfo.IsDir
reports true.This difference is minor since users are usually expected to replace the
FileHeader.Name
shortly afterwards anyways.The text was updated successfully, but these errors were encountered: