On Fri, 20 Mar 2026 21:22:48 GMT, Eirik Bjørsnøs <[email protected]> wrote:
>> test/jdk/java/util/zip/ZipOutputStream/UnmappableZipEntryNameOrComment.java
>> line 59:
>>
>>> 57: try (var out = new ZipOutputStream(nullOutputStream(),
>>> CHARSET)) {
>>> 58: assertThrows(ZipException.class, () -> out.putNextEntry(e));
>>> 59: }
>>
>> One thing to think about which is minor
>>
>>
>> try (var out = new ZipOutputStream(nullOutputStream(), US_ASCII)) {
>> // assertThrows(ZipException.class, () -> {
>> // ZipEntry e = new ZipEntry("\u00f8"); // 'ø' unmappable in
>> US_ASCII
>> // out.putNextEntry(e);
>> // });
>> ZipEntry e = new ZipEntry("\u00f8"); // 'ø' unmappable in
>> US_ASCII
>> out.putNextEntry(e);
>> } catch (Exception e) {
>> e.printStackTrace();
>> }
>>
>> The IAE is thrown from writeLOC vs writeCEN (due to closing the Zip)
>>
>> Question is do we want to remove the use of try/with/resources or leave as
>> is?
>>
>> With your change it might not matter though but wanted to throw it out there
>
>> The IAE is thrown from writeLOC vs writeCEN (due to closing the Zip)
>>
>> Question is do we want to remove the use of try/with/resources or leave as
>> is?
>>
>> With your change it might not matter though but wanted to throw it out there
>
> With the try-with-resources in place, we excercise the close and verify that
> any validation exception is throw from `putNextEntry` and only then.
>
> If we don't call close (explicitly or implicitly) then we won't neccessarily
> catch a regression where decoding the name fails first in putNextEntry, then
> in close.
>
> Do you think this makes sense?
I am OK with how the test is, especially with the change. With the original
code we could have failed earlier via the exception thrown via WriteLOC vs at
the end when the file was closed. With the try block, we caught the failure
during writeCEN
Again not a big deal, I was just pointing it out given the difference on when
the failure occurs
All good
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30319#discussion_r2968100365