On Fri, 17 Feb 2023 15:30:21 GMT, Jaikiran Pai <[email protected]> wrote:
>> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line
>> 1970:
>>
>>> 1968: OutputStream os = null;
>>> 1969: try {
>>> 1970: os = new BufferedOutputStream(new
>>> FileOutputStream(signedJarFile));
>>
>> Can we make this change right at
>> https://github.com/openjdk/jdk/blob/5dfc4ec7d94af9fe39fdee9d83b06101b827a3c6/src/jdk.jartool/share/classes/jdk/security/jarsigner/JarSigner.java#L691
>> or at least inside that class?
>
> Hello Max, what you suggest sounds fine to me. I've updated the PR to follow
> this suggestion. The only difference now would be that in the previous
> proposed patch if any exception happened during the jarsiging the
> `BufferedOutputStream` would be (flushed and) closed from the finally block
> of the main method. However, in the newer version, there can be a case that
> if an exception occurs then the underlying BufferedOutputStream, constructed
> in the `JarSigner` class may not be flushed/closed, but that I think should
> be fine, because we anyway delete the temporary signed jar file, in the main
> method, if any exception occurs.
>
> With this new change, I again ran the test against a 3GB and 6GB file and the
> numbers continue to show the improvement. I'll trigger the tier testing too.
Correct, `jarsigner` the tool won't damage the original JAR file if an error
occurs. For people directly calling the `JarSigner` API, I think they should
not have any expectation on what is written into the output stream when there
is an error.
-------------
PR: https://git.openjdk.org/jdk/pull/12588
