On Thu, 1 Oct 2026 17:51:16 GMT, Alan Bateman <[email protected]> wrote:
>> Hello Alan, I had a more detailed look and ran some experiments. The >> presence of the end-of-central-dir signature (the `ENDSIG`) is already >> guaranteed (through the `find_positions(...)` call) even before we start >> looping over the central directory entries in this `find_file()` function. >> If the `ENDSIG` is missing in the ZIP, then `find_file()` will return `-1` >> even before starting the while loop, and thus the ZIP will be considered >> invalid/corrupt. So on that front, this implementation is fine. >> >> The second aspect of this is that when the central directory entries end, >> and we break out of the while loop, then the comment says that the ZIP file >> must contain the `ENDSIG` after the central directory entries end. However >> it doesn't say that `ENDSIG` must immediately start at the next byte. I went >> back and ran some experiments and although the `ENDSIG` is expected to >> immediately follow the last central directory entry, the ZIP structure >> itself is flexible to allow for additional (arbitrary) bytes between the >> last central directory entry and the `ENDSIG`. In fact, some prominent ZIP >> tools allow for such ZIP files to be functional (you can list and extract >> files from such ZIP files). They do print out a message about this oddity >> when working on those files. >> >> Given this, I think adding a check for `ENDSIG` and returning `-1` if that >> check fails would end up considering such ZIP files as corrupt/invalid and >> that may not be ideal. I can adjust the comment at the start of the while >> loop to be a bit more clear if that helps. >> >> Do you think we should add the tighter check here? > > I looked at it again and I think you are right, it's the comment that put me > off and maybe we should clarify it. Hello Alan, I've now pushed an update to the PR to adjust that comment. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33064#discussion_r4181559295
