On Thu, 16 Nov 2023 20:52:08 GMT, Eirik Bjorsnos <d...@openjdk.org> wrote:
> > Regarding you comment about checking whether or not to check if the > > combined length of the CEN header + name length + comment length + extra > > length > 65K bytes, I chose to add this given the strong wording given this > > is a really old spec. That being said, I do not object to removing the > > validation if that is the overall preference. > > I can't claim to have a particularly strong opinion on this, the following is > mostly me thinking aloud: > > * Given Hyrum's Law, it is conceivable that someone is currently using the > extra or comment fields to attach up to 65535+65535 bytes of metadata for > entires. The proposed validation will break such schemes. Does Oracle have a > ZIP file corpus which could be used to identify files currently exceeding the > combined length clause, just to get a sense of how rare or common this is? > * The actual benefits of adding this validation after all these years is not > quite clear to me. I don't see how this improves security, robustness, > compatibility, maintainability or other ilities (apart from > strictly-following-the-spec-ility :-) > * I created a ZIP file with an entry with an extra field of the maximal > expressable length of 0xFFFF. Info-ZIP's `unzip` command on MacOS did not > output any warning or error when processing this file. Yes we have a corpus search available and have exercised this patch (along with your ZipInputStream patch) without any regressions. Given where we are in the JDK 22 cycle, going to hold off on finalizing the PR until we fork for JDK 23 and look to move this forward early on allowing for additional time to bake ------------- PR Comment: https://git.openjdk.org/jdk/pull/16570#issuecomment-1830639162
