Usually Gerrit changesets also contain test(s) for the fix.

@av86743 ok. I will add it tomorrow morning. Thanks

Thank you for filing this bug @dgrr, and @av86743 for encouraging the test in the patchset!

@dgrr could you please perhaps explain the underlying problem and perhaps describe why the current behavior is wrong?

@odeke-em

archive/zip: zip.OpenReader panics when opened .zip file does not exist

as shown by provided example https://play.golang.org/p/JvKpGINHHBY

Panics similarly on /dev/null and /dev/zero as well.

Hello @odeke-em

The underlying problem is that the Read function in a zip.File returns the io.EOF error in the last read call, when readed bytes > 0.

I think that the current behavior is wrong based on how other Readers works. Also, the io.Reader doc says:

When Read encounters an error or end-of-file condition after successfully reading n > 0 bytes, it returns the number of bytes read. It may return the (non-nil) error from the same call or return the error (and n == 0) from a subsequent call.

Perhaps @odeke-em was going to suggest to add verbal description of a problem and/or solution to Gerrit issue, to facilitate the process of review?

@dgrr I'm sorry, I don't understand. The docs for io.Reader that you quote permit a Reader to return a non-zero number of bytes with io.EOF. Code that calls Read must always handle the bytes first, then consider the error.

The docs for io.Reader say

Callers should always process the n > 0 bytes returned before considering the error err. Doing so correctly handles I/O errors that happen after reading some bytes and also both of the allowed EOF behaviors.

It's not clear to me that there is any bug here.

Yes, you are right. But it's kind of weird to see how this Reader works as other readers works. From my point of view, I see that maybe the best option should be to return the io.EOF error in the subsequent call. But it's just a suggestion here.