RFR: 8345431: Detect duplicate entries in jar files with jar --validate [v8]

[Lance Andersen]

On Thu, 15 May 2025 21:57:17 GMT, Henry Jen <henryjen at openjdk.org> wrote:

>> This PR check the jar file to ensure entries are consistent from the central directory and local file header. Also check there is no duplicate entry names that could override the desired content by accident.
>
> Henry Jen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adjust message based on review feedback

Hi Henry,

A few comments based on an initial pass through last update

src/jdk.jartool/share/classes/sun/tools/jar/Validator.java line 65:

> 63:      * - does not contain a backslash, '\'
> 64:      * - does not contain a drive letter
> 65:      * - path element does not include '..'

change to 
 >    * - path element does not include '.' or '..'

src/jdk.jartool/share/classes/sun/tools/jar/Validator.java line 117:

> 115:      * and UNIX file systems etc.
> 116:      * Also validate that the file name is not "." and that any name element is
> 117:      * not equal to ".."

Also validate that the file name is not "." or ".." and that any name element is
     * not equal to ".." or "."

src/jdk.jartool/share/classes/sun/tools/jar/Validator.java line 132:

> 130:      * - CEN and LOC should have same entries, in the same order
> 131:      * NOTE: This implementation assumes CEN entries are to be added before
> 132:      *       add any LOC entries.

I think you should probably expand this comment a bit with the expected work flow as it takes a while to grasp your intent otherwise

src/jdk.jartool/share/classes/sun/tools/jar/Validator.java line 155:

> 153:             }
> 154: 
> 155:             boolean isPlaceHolder() {

A comment would be helpful here for future maintainers

src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 287:

> 285: \                             or invalid file names that could lead to unintended effects.\n\
> 286: \                             Check with the developer to ensure the jar archive integrity\n\
> 287: \                             when warnings observed after using this option.

I  think this could be improved perhaps something like:


main.help.opt.main.validate=\
\      --validate             Validate the contents of the jar archive. This option will:\n\
\                             + validate that the API exported by a multi-release\n\
\                                 jar archive is consistent across all different release\n\
\                                versions.\n\
\                             + warn if there are duplicate or invalid file names\n\


I think you can remove the verbiage WRT "Check with..."  from the help message

src/jdk.jartool/share/man/jar.md line 111:

> 109: `--validate`
> 110: :   Validate the contents of the jar archive.
> 111:     Check with the developer to ensure the jar archive integrity

You can remove here and add a suggestion in the "integrity of a Jar Archive" section as to what to do.

src/jdk.jartool/share/man/jar.md line 223:

> 221: 
> 222: ## Integrity of a jar Archive
> 223: As a jar archive is based on ZIP format, it is possible to manufacture a jar archive using tools

manufacture -> create

src/jdk.jartool/share/man/jar.md line 224:

> 222: ## Integrity of a jar Archive
> 223: As a jar archive is based on ZIP format, it is possible to manufacture a jar archive using tools
> 224: other than the `jar` command. The `--validate` options checks a jar archive for some integrity

`The '--validate' option performs the following integrity checks:`

src/jdk.jartool/share/man/jar.md line 237:

> 235: -   The API exported by a multi-release jar archive is consistent across all different release
> 236:     versions.
> 237: 

I would consider something similar to:


 - That there are no duplicate Zip Entry file names
 - Verify that the Zip Entry file name:
      - is not an absolute path
      - the file name is not '.' or '..'
      - does not contain a backslash, ''
      - does not contain a drive letter
      - path element does not include '.' or '..
  - The API exported by a multi-release jar archive is consistent across all different release versions.

 The jar tool will return a status code of 0 if there were  no integrity issues encountered and a status code of 1 an issue was found.
  
When an integrity issue is reported, it will often require that the jar file is re-created by the original source of the jar file
```.

test/jdk/tools/jar/ValidatorTest.java line 325:

> 323:     }
> 324: 
> 325:     private void rm(String cmdline) {

Maybe I missed it, but is this method used?

-------------

PR Review: https://git.openjdk.org/jdk/pull/24430#pullrequestreview-2847026334
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093296698
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093300176
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093421481
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093422620
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093445110
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093448844
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093450314
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093452262
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093475451
PR Review Comment: https://git.openjdk.org/jdk/pull/24430#discussion_r2093537089