On Wed, 26 Mar 2025 12:34:13 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Updated with Sean's comments
>
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line
> 1138:
>
>> 1136: private void crossCheckEntries(String jarName) throws Exception {
>> 1137: List<String> locEntries = new ArrayList<>();
>> 1138: List<String> cenEntries;
>
> You can move this declaration to where you first use it (line 1177).
Moved.
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line
> 1163:
>
>> 1161: }
>> 1162:
>> 1163: readEntry(jis);
>
> I think this method is only needed in order to get the code signers, right?
> If so, it can be moved to just before line 1174.
Both `readEntry()` are needed to ensure that the entry is fully processed
before calling `compareSigners()`.
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line
> 1168:
>
>> 1166:
>> crossChkWarnings.add(String.format(rb.getString(
>> 1167:
>> "entry.1.present.in.jarfile.but.unreadable"),
>> 1168: entryName));
>
> I think we should continue after this, since the JarInputStream entry doesn't
> exist, so no need to compare code signers.
Fixed.
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/resources/jarsigner.properties
> line 215:
>
>> 213:
>> manifest.attribute.1.present.when.reading.jarinputstream.but.missing.via.jarfile=Manifest
>> attribute %s is present when reading via JarInputStream but missing when
>> reading via JarFile
>> 214:
>> manifest.attribute.1.differs.jarfile.value.2.jarinputstream.value.3=Manifest
>> attribute %1$s differs: JarFile value = %2$s, JarInputStream value = %3$s
>> 215:
>> Entry.1.present.when.reading.jarinputstream.but.missing.via.JarFile=Entry %s
>> is present when reading via JarInputStream but missing when reading via
>> JarFile
>
> Lower case the name ("entry.1")
>
> (to be consistent with other property names)
Done.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015092387
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015092416
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015092577
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015092456
