On Fri, 23 Sep 2022 17:45:26 GMT, Sean Mullan wrote:
> Do you also want to say "Otherwise, the method returns `null`."
The method description has that already, it might be too much detail to include
in the class description,.
-
PR: https://git.openjdk.org/jdk/pull/10045
On Tue, 20 Sep 2022 18:08:23 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Tue, 20 Sep 2022 18:08:23 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Fri, 23 Sep 2022 17:38:47 GMT, Sean Mullan wrote:
> As a side comment, I notice that `JarInputStream` capitalizes "JAR", whereas
> `JarFile` does not ("jar"). We should really be consistent in our javadocs. I
> think "JAR" is more correct, mainly because that is what the Jar File
> specific
On Tue, 20 Sep 2022 18:08:23 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Wed, 21 Sep 2022 09:23:31 GMT, Alan Bateman wrote:
> > Assuming we are set with the other changes, did you want to add the
> > following paragraph (or similar) at line 58 to make it clear that if the
> > Manifest is not found, then the JarEntry attributes will not be available:
>
> This is
On Tue, 20 Sep 2022 18:08:21 GMT, Lance Andersen wrote:
> Assuming we are set with the other changes, did you want to add the following
> paragraph (or similar) at line 58 to make it clear that if the Manifest is
> not found, then the JarEntry attributes will not be available:
This is somethin
On Tue, 20 Sep 2022 17:47:03 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Tue, 20 Sep 2022 17:53:27 GMT, Sean Mullan wrote:
> Now that this API has a section about signed JARs, I think it is very
> important to include the following sentences which are copied from `JarFile`:
>
> "Please note that the verification process does not include validating the
> signer's
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Tue, 20 Sep 2022 17:47:03 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Tue, 20 Sep 2022 17:37:50 GMT, Alan Bateman wrote:
>>> > OK, will make another pass at this today
>>>
>>> I looked at the latest draft
>>> ([2bafc00](https://github.com/openjdk/jdk/commit/2bafc00cc462b7af3f724371ac1bef5fd99c989c)).
>>> I think it would help if the section "Verifying a JarIn
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Tue, 20 Sep 2022 10:49:59 GMT, Lance Andersen wrote:
>>> OK, will make another pass at this today
>>
>> I looked at the latest draft (2bafc00c). I think it would help if the
>> section "Verifying a JarInputStream" were renamed to "Signed JAR files".
>> The link to getManifest makes the rea
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Mon, 19 Sep 2022 20:30:48 GMT, Weijun Wang wrote:
> Does this mean that the "Verifying a JarInputStream" should also avoid
> mentioning "getManifest method returns the manifest"? I understand precisely
> it should be "getManifest method is able to return the manifest if you call
> it".
See
On Tue, 20 Sep 2022 06:56:49 GMT, Alan Bateman wrote:
>>> I realise you've had a few iterations with Max on this section but I'm
>>> concerned that the text is telling the reader that they should use the
>>> 2-arg constructor to verify the signatures when a JAR is signed. The
>>> default is to
On Mon, 19 Sep 2022 10:21:30 GMT, Lance Andersen wrote:
> OK, will make another pass at this today
I looked at the latest draft (2bafc00c). I think it would help if the section
"Verifying a JarInputStream" were renamed to "Signed JAR files". The link to
getManifest makes the reader wonder if
On Mon, 19 Sep 2022 17:53:51 GMT, Lance Andersen wrote:
>>> 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 cl
On Mon, 19 Sep 2022 19:26:53 GMT, Sean Mullan wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Minor clarification for verifying an JarInputStream
>
> src/java.base/share/classes/java/util/jar/JarInputStream.java
On Mon, 19 Sep 2022 20:04:18 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Mon, 19 Sep 2022 17:25:53 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Mon, 19 Sep 2022 06:34:00 GMT, Alan Bateman wrote:
> > 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 clar
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Mon, 19 Sep 2022 06:45:13 GMT, Alan Bateman wrote:
> I realise you've had a few iterations with Max on this section but I'm
> concerned that the text is telling the reader that they should use the 2-arg
> constructor to verify the signatures when a JAR is signed. The default is to
> verify
On Sun, 18 Sep 2022 20:43:25 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Sun, 18 Sep 2022 20:33:55 GMT, Lance Andersen wrote:
> 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
> clarificat
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Sun, 18 Sep 2022 19:49:51 GMT, Alan Bateman 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:
>
>> 3
On Sat, 17 Sep 2022 14:35:46 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Sat, 17 Sep 2022 08:05:37 GMT, Alan Bateman wrote:
> It's a bit better but I think we can make it clearer and also link the JAR
> Manifest section of the JAR file spec. Can you try this:
>
> ```
> * The {@link #getManifest() getManifest} method is used to read the
> * JAR Manifest
> * fr
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Fri, 16 Sep 2022 18:33:46 GMT, Lance Andersen wrote:
>> Okay, in which case what would you think about just saying that the
>> getNextEntry/getNextJarEntry method do not return the Manifest when it's at
>> the start of the stream, and it's unspecified whether they return the
>> Manifest whe
On Fri, 16 Sep 2022 18:01:40 GMT, Alan Bateman wrote:
> Okay, in which case what would you think about just saying that the
> getNextEntry/getNextJarEntry method do not return the Manifest when it's at
> the start of the stream, and it's unspecified whether they return the
> Manifest when it l
On Fri, 16 Sep 2022 16:15:44 GMT, Lance Andersen wrote:
>> src/java.base/share/classes/java/util/jar/JarInputStream.java line 51:
>>
>>> 49: * If {@code META-INF/} is the first entry in the input stream it will
>>> be
>>> 50: * also not be returned by {@link #getNextEntry()} and
>>> 51: * {@
On Fri, 16 Sep 2022 13:06:28 GMT, Alan Bateman wrote:
>> Lance Andersen has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address latest input regarding wording
>
> src/java.base/share/classes/java/util/jar/JarInputStream.java line 44:
>
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Wed, 14 Sep 2022 16:43:43 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Wed, 14 Sep 2022 15:53:41 GMT, Weijun Wang wrote:
> I have no more comment.
Thank you Max for your time and input
-
PR: https://git.openjdk.org/jdk/pull/10045
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Wed, 14 Sep 2022 12:53:12 GMT, Alan Bateman wrote:
> I think you can insert a comma after "when it is the first entry in the
> stream"? I think that would make it a bit clearer that there are two cases.
Done
>
> Also I'm wondering if the paragraph should be split into two, meaning "When
>
On Wed, 14 Sep 2022 10:31:41 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Wed, 14 Sep 2022 10:31:41 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Wed, 14 Sep 2022 02:22:17 GMT, Weijun Wang wrote:
> Only tiny comments for the last paragraph.
Thank you Max, I addressed the above
>
> That said, I have some questions on the other parts of this file:
>
> 1. In `getNextEntry`, the method spec says "If verification has been
> enabled,
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Wed, 14 Sep 2022 01:42:20 GMT, Lance Andersen wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this updat
On Wed, 14 Sep 2022 00:39:37 GMT, Weijun Wang wrote:
> On lines 36 and 37, there are two "Manifest". The first uses `linkplain` so
> it's using normal font style, the 2nd uses `code` so it's fixed-width. I
> would like them to be the same. In fact, I would not use `linkplain` at all.
> I only
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we are finally documenting
> behavior that dates back to w
On Tue, 13 Sep 2022 20:58:04 GMT, Lance Andersen wrote:
>> src/java.base/share/classes/java/util/jar/JarInputStream.java line 36:
>>
>>> 34: * The {@code JarInputStream} class, which extends {@linkplain
>>> ZipInputStream},
>>> 35: * is used to read the contents of a JAR file from an input st
On Fri, 26 Aug 2022 15:48:55 GMT, Lance Andersen wrote:
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we
On Tue, 13 Sep 2022 20:39:31 GMT, Weijun Wang wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this update,
On Fri, 26 Aug 2022 15:48:55 GMT, Lance Andersen wrote:
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we
On Fri, 2 Sep 2022 14:50:32 GMT, Alan Bateman wrote:
>> I could do tweak further to say:
>>
>> _`getManifest()` will return the Manifest if it is the first entry or
>> META-INF/ is the first entry and the Manifest is the second entry within the
>> Jar file. When the Manifest is returned by `
On Thu, 1 Sep 2022 17:56:03 GMT, Lance Andersen wrote:
>>> It's better. Do you need to explicitly say "For all other cases"?
>>
>> I thought it is worth being specific, but happy to leave it out if you and
>> others prefer
>>>
>>> My original comment was more about explaining `getManifest()`
On Thu, 1 Sep 2022 13:53:05 GMT, Weijun Wang wrote:
> It's better. Do you need to explicitly say "For all other cases"?
I thought it is worth being specific, but happy to leave it out if you and
others prefer
>
> My original comment was more about explaining `getManifest()` and
> `getNextEnt
On Thu, 1 Sep 2022 17:27:23 GMT, Lance Andersen wrote:
>> It's better. Do you need to explicitly say "For all other cases"?
>>
>> My original comment was more about explaining `getManifest()` and
>> `getNextEntry()` in the same if. It's still doable.
>
>> It's better. Do you need to explicitly
On Thu, 1 Sep 2022 11:06:41 GMT, Lance Andersen wrote:
>> That's right. But I think we care about the MANIFEST more. It's not that
>> important whether META-INF is there.
>
> True we do care more about the manifest, but was also trying to address the
> difference between ZipInputStream which wi
On Wed, 31 Aug 2022 19:13:05 GMT, Weijun Wang wrote:
>> The challenge I had with the wording is due to the fact that if "META-INF/"
>> is the first entry in the Zip file, it will not be returned regardless of
>> whether there is a manifest. So open to suggestions.
>
> That's right. But I think
On Wed, 31 Aug 2022 18:31:13 GMT, Lance Andersen wrote:
>> src/java.base/share/classes/java/util/jar/JarInputStream.java line 62:
>>
>>> 60: * is the second jar entry
>>> 61: *
>>> 62: *
>>
>> I wonder if it's necessary to duplicate these lines. How about something
>> like "I
On Tue, 30 Aug 2022 21:47:03 GMT, Weijun Wang wrote:
>> Please review this PR which updates the JarInputStream class description to
>> clarify when the Manifest is accessible via JarInputStream::getManifest and
>> JarInputStream::get[Jar]Entry.
>>
>> It is worth noting that with this update,
Please review this PR which updates the JarInputStream class description to
clarify when the Manifest is accessible via JarInputStream::getManifest and
JarInputStream::get[Jar]Entry.
It is worth noting that with this update, we are finally documenting behavior
that dates back to when this cla
On Fri, 26 Aug 2022 15:48:55 GMT, Lance Andersen wrote:
> Please review this PR which updates the JarInputStream class description to
> clarify when the Manifest is accessible via JarInputStream::getManifest and
> JarInputStream::get[Jar]Entry.
>
> It is worth noting that with this update, we
63 matches
Mail list logo
