On Wed, 30 Oct 2024 16:52:16 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> Hello Claes, >> >>> if there's an encoding error we'd have thrown earlier when hashing over the >>> full name. >> >> If I understand correctly, then I think you are referring to the >> `ZipCoder.checkedHash()` which throws that `Exception`. >> >> Before the change in this PR, there was (and continues to exist) one such >> call in the `checkAndAddEntry()` private method of this `Source` class. In >> that place we catch the `Exception` and throw a `ZipException` (which is an >> `IOException`) >> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/ZipFile.java#L1264. >> >> I think we should be doing the same here and throw a `ZipException` instead >> of an `IllegalArgumentException` > > What I mean is this IAE cant be thrown and is more of an assert. But sure, it > should throw a ZipException to be more robust and obviously correct. Yes, this was a "this will never happen" situation so I thought of it as an assert. But that's not locally obvious and I agree it's better to be consistent and use `ZipException` here. See #21791 for a fix. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21489#discussion_r1823065479
