x/mod/zip: verify file list without creating zip

[jayconrod]

golang.org/x/mod/zip provides a Create function that takes a list of files ([]File). On success, it creates a zip file with some of those files. It ignores files in submodules and vendor directories and reports errors for files with invalid names or types (symbolic links). On failure, it reports an error on the first file that caused a problem.

This is not the most useful API for clients creating zip files or verifying that a set of files can be used to create a zip file. Some improvements are possible without making incompatible changes to the API:

• We should provide a Verify function that accepts []File and reports whether each file in that list will be included, ignored, or rejected. Verify would not create a zip file, and it would not read files (just stat to get sizes and types), so it should be faster than Create.
• When Create encounters an error due to the []File argument, it should return a structured error with information about each file. This should be similar to what Verify returns (perhaps whatever Verify returns can satisfy the error interface).
• Unzip should also return a structured error so that issues with multiple zip entries can be reported.
• We may also want a VerifyZip function which checks a zip file for errors without extracting it. Currently, Unzip always needs a directory to write files.
• We should document more clearly when files are included, ignored, or rejected.

cc @dmitshur

[bcmills]

Some improvements are possible without making incompatible changes to the API

Incompatible changes would be fine too, I think: the latest tagged version of that repo is still v0.1.0 and the zip package is not included in that tag, so even with our more conservative approach to breaking changes, most users should be unaffected.

[dmitshur]

Issue #37397 is related.

It's worth considering that depending on the exact goal of a user, they may want to reproduce that vendor bug in order to ensure consistent behavior with cmd/go, or they want to resolve that bug and avoid stripping some valid packages unneccesarily.

A good high-level way of asking that question is whether the user goal is to stay within the rules of the module proxy protocol, or additionally within the cmd/go implementation restrictions.

[jayconrod]

A good high-level way of asking that question is whether the user goal is to stay within the rules of the module proxy protocol, or additionally within the cmd/go implementation restrictions.

That's a good way to phrase it. Perhaps zip.Create could be a lower-level API that zips all files unless they would cause an error in zip.Unzip (e.g., too big, invalid file name). Or maybe we would introduce zip.CreateRaw to avoid an incompatible change. zip.CreateFromDir would be the convenience function that ignores vendor directories.

Verify would still be useful for clients to understand whether a file should be included.

[gopherbot]

Change https://golang.org/cl/235597 mentions this issue: zip: add CheckFiles, CheckDir, and CheckZip