On Wed, 26 Mar 2025 23:26:12 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Hai-May Chao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update with comments from Sean and Weijun
>
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java line
> 1215:
>
>> 1213: if (!cenEntries2.equals(locEntries)) {
>> 1214: crossChkWarnings.add(rb.getString(
>> 1215:
>> "entries.mismatch.when.comparing.jarfile.and.jarinputstream"));
>
> Do we still need this warning? The meaning is not clear to me. Since we have
> already compared in both ways, does this only mean the orders are different?
This step checks content and order. As the order does matter, I have this step
to explicitly warn about ordering issue.
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/resources/jarsigner.properties
> line 219:
>
>> 217: entry.1.present.in.jarfile.but.unreadable=Entry %s is present in
>> JarFile but unreadable
>> 218:
>> codesigners.different.for.entry.1.when.reading.jarfile.and.jarinputstream=Code
>> signers are different for entry %s when reading from JarFile and
>> JarInputStream
>> 219: entry.1.has.codesigners.in.jarfile.but.not.in.jarinputstream=Entry %s
>> has codesigners in JarFile but not in JarInputStream
>
> Usually we don't say "has codesigners" or "has no codesigners", we say "is
> signed" and "is not signed". Same for the next one.
Change them to:
Entry %s is signed in JarFile but is not signed in JarInputStream
Entry %s is signed in JarInputStream but is not signed in JarFile
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/resources/jarsigner.properties
> line 222:
>
>> 220: entry.1.has.codesigners.in.jarinputstream.but.not.in.jarfile=Entry %s
>> has codesigners in JarInputStream but not in JarFile
>> 221: entries.mismatch.when.comparing.jarfile.and.jarinputstream=Entries
>> mismatch when comparing JarFile and JarInputStream
>> 222:
>> jar.contains.internal.inconsistencies.may.result.in.different.contents.when.reading.via.jarfile.and.jarinputstream=This
>> JAR file contains internal inconsistencies that may result in different
>> contents when reading via JarFile and JarInputStream
>
> Do you think it makes sense to add a ":" at the end of this header line?
Yes.
> src/jdk.jartool/share/classes/sun/security/tools/jarsigner/resources/jarsigner.properties
> line 224:
>
>> 222:
>> jar.contains.internal.inconsistencies.may.result.in.different.contents.when.reading.via.jarfile.and.jarinputstream=This
>> JAR file contains internal inconsistencies that may result in different
>> contents when reading via JarFile and JarInputStream
>> 223:
>> signature.verification.failed.on.entry.1.when.reading.via.jarinputstream=Signature
>> verification failed on entry %s when reading via JarInputStream
>> 224:
>> signature.verification.failed.on.entry.1.when.reading.via.jarfile.inputstream=Signature
>> verification failed on entry %s when reading via JarFile InputStream
>
> I don't think you need to mention `InputStream` for the "JarFile" case.
The entry is read using an `InputStream` from `JarFile`, and is not directly
via `JarFile`. So I added `InputStream`. Remove it to be more consistent with
other warnings.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015407996
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015408034
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015408114
PR Review Comment: https://git.openjdk.org/jdk/pull/23532#discussion_r2015408205
