Hi,
Please review this simple code cleanup.
The following checking for key in the makeSessionKey() method is redundant as
it the same checking has been performance before calling the method.
if (k == null) {
throw new InvalidKeyException("Empty key");
}
if (
On Wed, 7 Sep 2022 01:41:14 GMT, Weijun Wang wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> comments from Max and Sean
>
> src/java.base/share/classes/sun/security/ssl/ECDHClientKeyExchange.java line
> 289:
>
>>
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 Wed, 7 Sep 2022 19:49:53 GMT, Mark Powers wrote:
> https://bugs.openjdk.org/browse/JDK-8291974
I would write a test which serializes the data (before your change) and
deserializes it after. There should be some regression tests that already do
that.
-
PR: https://git.openjdk.o
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
On Tue, 6 Sep 2022 22:48:00 GMT, Weijun Wang wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> white space
>
> src/java.base/share/classes/sun/security/ssl/CertificateMessage.java line 610:
>
>> 608: X509
On Tue, 13 Sep 2022 14:58:11 GMT, Sean Coffey wrote:
> Ensure that security properties are only overridden if the override security
> properties file exists.
> Refactored some of the code in Security class initialization also.
> Extra test coverage for security properties file options.
Is it a
Ensure that security properties are only overridden if the override security
properties file exists.
Refactored some of the code in Security class initialization also.
Extra test coverage for security properties file options.
-
Commit messages:
- Merge branch 'master' into 8292297-
23 matches
Mail list logo
