RFR: 8215788: Clarify JarInputStream Manifest access [v6]
Lance Andersen
lancea at openjdk.org
Sun Sep 18 20:37:43 UTC 2022
On Sun, 18 Sep 2022 19:49:51 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> Lance Andersen has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Incorporated latest round of input
>
> src/java.base/share/classes/java/util/jar/JarInputStream.java line 36:
>
>> 34: * The {@code JarInputStream} class, which extends {@link ZipInputStream},
>> 35: * is used to read the contents of a JAR file from an input stream.
>> 36: * It provides support for reading an optional {@link JarFile#MANIFEST_NAME Manifest}
>
> What would you think about linking this to {@docRoot}/../specs/jar/jar.html#jar-manifest rather tan JarFile#MANIFEST_NAME?
Sure if that is your preference.
> src/java.base/share/classes/java/util/jar/JarInputStream.java line 60:
>
>> 58: * {@link JarEntry#getAttributes()} will return the {@code Manifest}'s
>> 59: * attributes for the current JAR file entry, if any, providing
>> 60: * {@code getManifest()} returns a {@code Manifest} for the JAR file.
>
> Per-entry attributes is an advanced feature to attempt to bring into the class description. I think it would be simpler to just drop this paragraph. If you really want something on this topic then it would require first describing main vs. per entry attributes and then explaining that the per-entry attributes are obtained with JarEntry::getAttributes when the manifest is at the beginning of the stream.
I can remove, but I am not sure I agree we need to describe main vs attribute here given we are pointing to the Jar spec and if there is any discussion of Pre-entry attributes, it should be in JarEntry IMHO. I guess the clarification I was trying to make, apparently unsuccessfully is that `JarEntry` will not have access to the attributes if `getManifest` does not return the Manifest.
> src/java.base/share/classes/java/util/jar/JarInputStream.java line 157:
>
>> 155: *
>> 156: * @return the {@code Manifest} for this JAR file when accessible, or
>> 157: * {@code null} otherwise.
>
> The word "accessible" suggests there is access control in the picture so I think drop that word. Maybe just drop "if none" from the original return description?
Will change as you suggest
-------------
PR: https://git.openjdk.org/jdk/pull/10045
More information about the security-dev
mailing list