Re: Need sponsor to fix Javadoc warnings
Hi Pavel,
On 8/04/2020 11:23 pm, Pavel Rappo wrote:
Hey David,
Where exactly? In the files affected by this changeset? If so, then we will
introduce inconsistency. Otherwise it's a huge change. From what I can see there
are some 250 occurrences of `@exception` in
src/java.base/share/classes/com/sun/{crypto, security}
and some 7,300 in src.
Okay as I said I didn't examine in detail to see the size of the change
- and its obviously too big.
Personally, out of all tag renovations, changing `@exception` to `@throws`
probably gives the least bang for the buck. If nothing else, it gives you 3
extra characters on the same line to fill with something more useful.
There was an effort to do this conversion a while back, at least while
touching affected files. It's not a big deal to me.
Cheers,
David
I would be more inclined to change `...` to `{@code ...}`, but
given how error-prone that can be, I still wouldn't do it in this changeset.
-Pavel
On 8 Apr 2020, at 13:56, David Holmes wrote:
Hi Pavel,
Not a review ...
On 8/04/2020 9:50 pm, Pavel Rappo wrote:
Vipin, here you go:
https://bugs.openjdk.java.net/browse/JDK-8242366
http://cr.openjdk.java.net/~prappo/8242366/webrev.00/
I took the liberty of additionally fixing a couple of parameters' names,
a typo, and `@exception` tags for checked exceptions that were neither thrown
nor imported.
While you are in there is it worth changing @exception to @throws? (I didn't
look to see how big that change would be.)
Cheers,
David
The bulk of the change is in Security. Some changes are in Networking. The
appropriate mailing lists are in CC for this email. We should wait for their
feedback.
Changes in core area look good to me and I'd be surprised if there are any
problems with the remaining portion of the changeset.
-Pavel
On 7 Apr 2020, at 19:50, Vipin Sharma wrote:
Hi Pavel,
On Apr 7, 2020, at 11:11 PM, Pavel Rappo wrote:
I assume you have signed the OCA [1]. If not and you want to continue, please
do it. If you've already done so, which is probably the case [2], please attach
your patch as text to this thread with the next email. Do not use zip or the
like. I will take it from there and sponsor that for you.
Yes I have signed OCA.
-Pavel
[1] https://www.oracle.com/technetwork/community/oca-486395.html
[2] changeset: 58344:65f30e209890
user:clanger
date:Wed Mar 11 13:50:13 2020 +0100
files: test/jdk/java/lang/Boolean/GetBoolean.java
test/jdk/java/lang/Boolean/MakeBooleanComparable.java
test/jdk/java/lang/Boolean/ParseBoolean.java
description:
8240524: Remove explicit type argument in test
jdk/java/lang/Boolean/MakeBooleanComparable.java
Reviewed-by: clanger, vtewari
Contributed-by: vipinsharma85 at gmail.com
Yes this is my first contribution.
Patch text:
--- old/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java
2020-04-06 00:19:10.546117441 +0530
+++ new/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java
2020-04-06 00:19:10.130115855 +0530
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2002, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -202,7 +202,7 @@
/**
* Sets the padding mechanism of this cipher.
*
- * @param padding the padding mechanism
+ * @param paddingScheme the padding mechanism
*
* @exception NoSuchPaddingException if the requested padding mechanism
* does not exist
--- old/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java
2020-04-06 00:19:11.526121179 +0530
+++ new/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java
2020-04-06 00:19:11.118119622 +0530
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2004, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -313,10 +313,10 @@
* current Cipher.engineInit(...) implementation,
* IllegalStateException will always be thrown upon invocation.
*
- * @param in the input buffer
- * @param inOffset the offset in in where the input
+ * @param input the input buffer
+ * @param inputOffset the offset in in where the input
* starts
- * @param inLen the input length.
+ * @param inputLen the input length.
*
* @return n/a.
*
--- old/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java
2020-04-06 00:19:12.462124749 +0530
+++ new/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java
2020-04-06 00:19:12.054123193 +0530
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1998, 2007, Oracle and/
Re: Need sponsor to fix Javadoc warnings
If your new patch addresses a similar type of problem, please send it in reply to this email, so that I could merge it with the existing patch. Let's try to minimize process overhead if possible. > On 8 Apr 2020, at 17:35, Vipin Sharma wrote: > > > >> On Apr 8, 2020, at 6:57 PM, Pavel Rappo wrote: >> >> Why assume something that sophisticated where it can be adequately explained >> by >> a simpler thing? :) I bet it was an IDE inspection. >> >> -Pavel > > Yes, it was IDE inspection in java.base, it looked like the best way to start > contributing and understand the process. > If it helps and not creating noise for you all, I see an opportunity for one > more such patch. What do you think? > >> >>> On 8 Apr 2020, at 14:14, Alan Bateman wrote: >>> >>> On 08/04/2020 14:07, Daniel Fuchs wrote: Hi Pavel, On 08/04/2020 13:56, David Holmes wrote: > and `@exception` tags for checked exceptions that were neither thrown > nor imported Hopefully that's only on internal classes. >>> From a quick scan, the changes are to internal classes and a few non-public >>> elements of public API classes. So I don't think anything should impact the >>> javadoc. I'm guessing this patch is motivated by something creating that is >>> running the javadoc tool with different options to what the "docs" target >>> uses. >>> >>> -Alan >> > > Regards, > Vipin
Re: Need sponsor to fix Javadoc warnings
> On Apr 8, 2020, at 6:57 PM, Pavel Rappo wrote: > > Why assume something that sophisticated where it can be adequately explained > by > a simpler thing? :) I bet it was an IDE inspection. > > -Pavel Yes, it was IDE inspection in java.base, it looked like the best way to start contributing and understand the process. If it helps and not creating noise for you all, I see an opportunity for one more such patch. What do you think? > >> On 8 Apr 2020, at 14:14, Alan Bateman wrote: >> >> On 08/04/2020 14:07, Daniel Fuchs wrote: >>> Hi Pavel, >>> >>> On 08/04/2020 13:56, David Holmes wrote: and `@exception` tags for checked exceptions that were neither thrown nor imported >>> >>> Hopefully that's only on internal classes. >> From a quick scan, the changes are to internal classes and a few non-public >> elements of public API classes. So I don't think anything should impact the >> javadoc. I'm guessing this patch is motivated by something creating that is >> running the javadoc tool with different options to what the "docs" target >> uses. >> >> -Alan > Regards, Vipin
Re: RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected
Good work Rahul! I am not sure whether that deserves a CSR (probably not) but we may want to create some release note to explain that the HttpClient is no longer overriding the default protocols selected by the SSLContext. So HTTP 1.1 over TLSv1.1 might now get negotiated where previously an handshake failure would have occurred. It might be worth mentioning in a release note. best regards, -- daniel On 08/04/2020 10:13, Rahul wrote: Updated patch after considering the impact of returning default parameters on the http client. TLS versions earlier limited to 1.2 and above by client, now will support all versions(wrt the scenarios for this bug). Issue:https://bugs.openjdk.java.net/browse/JDK-8239595 Issue:https://bugs.openjdk.java.net/browse/JDK-8239594 Webrev:http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.01/ -- Rahul
Re: Need sponsor to fix Javadoc warnings
On 08/04/2020 12:50, Pavel Rappo wrote: Vipin, here you go: https://bugs.openjdk.java.net/browse/JDK-8242366 http://cr.openjdk.java.net/~prappo/8242366/webrev.00/ Hi Pavel, That looks good to me. WRT to the @exception changes I'll leave that responsibility to Sean ;-) best regards, -- daniel I took the liberty of additionally fixing a couple of parameters' names, a typo, and `@exception` tags for checked exceptions that were neither thrown nor imported.
Re: Need sponsor to fix Javadoc warnings
Why assume something that sophisticated where it can be adequately explained by a simpler thing? :) I bet it was an IDE inspection. -Pavel > On 8 Apr 2020, at 14:14, Alan Bateman wrote: > > On 08/04/2020 14:07, Daniel Fuchs wrote: >> Hi Pavel, >> >> On 08/04/2020 13:56, David Holmes wrote: >>> and `@exception` tags for checked exceptions that were neither thrown >>> nor imported >> >> Hopefully that's only on internal classes. > From a quick scan, the changes are to internal classes and a few non-public > elements of public API classes. So I don't think anything should impact the > javadoc. I'm guessing this patch is motivated by something creating that is > running the javadoc tool with different options to what the "docs" target > uses. > > -Alan
Re: Need sponsor to fix Javadoc warnings
Hey David,
Where exactly? In the files affected by this changeset? If so, then we will
introduce inconsistency. Otherwise it's a huge change. From what I can see there
are some 250 occurrences of `@exception` in
src/java.base/share/classes/com/sun/{crypto, security}
and some 7,300 in src.
Personally, out of all tag renovations, changing `@exception` to `@throws`
probably gives the least bang for the buck. If nothing else, it gives you 3
extra characters on the same line to fill with something more useful.
I would be more inclined to change `...` to `{@code ...}`, but
given how error-prone that can be, I still wouldn't do it in this changeset.
-Pavel
> On 8 Apr 2020, at 13:56, David Holmes wrote:
>
> Hi Pavel,
>
> Not a review ...
>
> On 8/04/2020 9:50 pm, Pavel Rappo wrote:
>> Vipin, here you go:
>> https://bugs.openjdk.java.net/browse/JDK-8242366
>> http://cr.openjdk.java.net/~prappo/8242366/webrev.00/
>> I took the liberty of additionally fixing a couple of parameters' names,
>> a typo, and `@exception` tags for checked exceptions that were neither thrown
>> nor imported.
>
> While you are in there is it worth changing @exception to @throws? (I didn't
> look to see how big that change would be.)
>
> Cheers,
> David
>
>> The bulk of the change is in Security. Some changes are in Networking. The
>> appropriate mailing lists are in CC for this email. We should wait for their
>> feedback.
>> Changes in core area look good to me and I'd be surprised if there are any
>> problems with the remaining portion of the changeset.
>> -Pavel
>>> On 7 Apr 2020, at 19:50, Vipin Sharma wrote:
>>>
>>> Hi Pavel,
>>>
On Apr 7, 2020, at 11:11 PM, Pavel Rappo wrote:
I assume you have signed the OCA [1]. If not and you want to continue,
please do it. If you've already done so, which is probably the case [2],
please attach your patch as text to this thread with the next email. Do
not use zip or the like. I will take it from there and sponsor that for
you.
>>> Yes I have signed OCA.
-Pavel
[1] https://www.oracle.com/technetwork/community/oca-486395.html
[2] changeset: 58344:65f30e209890
user:clanger
date:Wed Mar 11 13:50:13 2020 +0100
files: test/jdk/java/lang/Boolean/GetBoolean.java
test/jdk/java/lang/Boolean/MakeBooleanComparable.java
test/jdk/java/lang/Boolean/ParseBoolean.java
description:
8240524: Remove explicit type argument in test
jdk/java/lang/Boolean/MakeBooleanComparable.java
Reviewed-by: clanger, vtewari
Contributed-by: vipinsharma85 at gmail.com
>>> Yes this is my first contribution.
>>>
>>> Patch text:
>>>
>>> --- old/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java
>>> 2020-04-06 00:19:10.546117441 +0530
>>> +++ new/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java
>>> 2020-04-06 00:19:10.130115855 +0530
>>> @@ -1,5 +1,5 @@
>>> /*
>>> - * Copyright (c) 2002, 2017, Oracle and/or its affiliates. All rights
>>> reserved.
>>> + * Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights
>>> reserved.
>>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>> *
>>> * This code is free software; you can redistribute it and/or modify it
>>> @@ -202,7 +202,7 @@
>>> /**
>>> * Sets the padding mechanism of this cipher.
>>> *
>>> - * @param padding the padding mechanism
>>> + * @param paddingScheme the padding mechanism
>>> *
>>> * @exception NoSuchPaddingException if the requested padding mechanism
>>> * does not exist
>>> ---
>>> old/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java
>>> 2020-04-06 00:19:11.526121179 +0530
>>> +++
>>> new/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java
>>> 2020-04-06 00:19:11.118119622 +0530
>>> @@ -1,5 +1,5 @@
>>> /*
>>> - * Copyright (c) 2004, 2017, Oracle and/or its affiliates. All rights
>>> reserved.
>>> + * Copyright (c) 2004, 2020, Oracle and/or its affiliates. All rights
>>> reserved.
>>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>> *
>>> * This code is free software; you can redistribute it and/or modify it
>>> @@ -313,10 +313,10 @@
>>> * current Cipher.engineInit(...) implementation,
>>> * IllegalStateException will always be thrown upon invocation.
>>> *
>>> - * @param in the input buffer
>>> - * @param inOffset the offset in in where the input
>>> + * @param input the input buffer
>>> + * @param inputOffset the offset in in where the input
>>> * starts
>>> - * @param inLen the input length.
>>> + * @param inputLen the input length.
>>> *
>>> * @return n/a.
>>> *
>>> ---
>>> old/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java
>>> 2020-04-06 00:19:12.462124749 +0530
>>> +++
>>> new/src/java.base/share/classes/com/sun/crypto/provider/
Re: Need sponsor to fix Javadoc warnings
On 08/04/2020 14:07, Daniel Fuchs wrote: Hi Pavel, On 08/04/2020 13:56, David Holmes wrote: and `@exception` tags for checked exceptions that were neither thrown nor imported Hopefully that's only on internal classes. From a quick scan, the changes are to internal classes and a few non-public elements of public API classes. So I don't think anything should impact the javadoc. I'm guessing this patch is motivated by something creating that is running the javadoc tool with different options to what the "docs" target uses. -Alan
Re: Need sponsor to fix Javadoc warnings
The security changes look fine to me. --Sean On 4/8/20 7:50 AM, Pavel Rappo wrote: Vipin, here you go: https://bugs.openjdk.java.net/browse/JDK-8242366 http://cr.openjdk.java.net/~prappo/8242366/webrev.00/ I took the liberty of additionally fixing a couple of parameters' names, a typo, and `@exception` tags for checked exceptions that were neither thrown nor imported. The bulk of the change is in Security. Some changes are in Networking. The appropriate mailing lists are in CC for this email. We should wait for their feedback. Changes in core area look good to me and I'd be surprised if there are any problems with the remaining portion of the changeset. -Pavel On 7 Apr 2020, at 19:50, Vipin Sharma wrote: Hi Pavel, On Apr 7, 2020, at 11:11 PM, Pavel Rappo wrote: I assume you have signed the OCA [1]. If not and you want to continue, please do it. If you've already done so, which is probably the case [2], please attach your patch as text to this thread with the next email. Do not use zip or the like. I will take it from there and sponsor that for you. Yes I have signed OCA. -Pavel [1] https://www.oracle.com/technetwork/community/oca-486395.html [2] changeset: 58344:65f30e209890 user:clanger date:Wed Mar 11 13:50:13 2020 +0100 files: test/jdk/java/lang/Boolean/GetBoolean.java test/jdk/java/lang/Boolean/MakeBooleanComparable.java test/jdk/java/lang/Boolean/ParseBoolean.java description: 8240524: Remove explicit type argument in test jdk/java/lang/Boolean/MakeBooleanComparable.java Reviewed-by: clanger, vtewari Contributed-by: vipinsharma85 at gmail.com Yes this is my first contribution. Patch text: --- old/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java 2020-04-06 00:19:10.546117441 +0530 +++ new/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java 2020-04-06 00:19:10.130115855 +0530 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -202,7 +202,7 @@ /** * Sets the padding mechanism of this cipher. * - * @param padding the padding mechanism + * @param paddingScheme the padding mechanism * * @exception NoSuchPaddingException if the requested padding mechanism * does not exist --- old/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java 2020-04-06 00:19:11.526121179 +0530 +++ new/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java 2020-04-06 00:19:11.118119622 +0530 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2004, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2004, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -313,10 +313,10 @@ * current Cipher.engineInit(...) implementation, * IllegalStateException will always be thrown upon invocation. * - * @param in the input buffer - * @param inOffset the offset in in where the input + * @param input the input buffer + * @param inputOffset the offset in in where the input * starts - * @param inLen the input length. + * @param inputLen the input length. * * @return n/a. * --- old/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java 2020-04-06 00:19:12.462124749 +0530 +++ new/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java 2020-04-06 00:19:12.054123193 +0530 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2007, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -130,7 +130,6 @@ * * @param plain the buffer with the input data to be encrypted * @param plainOffset the offset in plain - * @param plainLen the length of the input data * @param cipher the buffer for the result * @param cipherOffset the offset in cipher */ @@ -154,7 +153,6 @@ * * @param cipher the buffer with the input data to be decrypted * @param cipherOffset the offset in cipherOffset - * @param cipherLen the length of the input data * @param plain the buffer for the result * @param plainOffset the offset in plain */ --- old/src/java.base/share/classes/com/sun/crypto/provider/DESCrypt.java 2020-04-06 00:19:13.414128382 +0530 +++ new/src/java.base/share/classes/com/sun/crypto/provider/DESCrypt.java 2020-04-06 00:19:12.998126795 +0530 @@ -1,5 +1,5 @@ /* - * Co
Re: Need sponsor to fix Javadoc warnings
Hi Pavel, On 08/04/2020 13:56, David Holmes wrote: and `@exception` tags for checked exceptions that were neither thrown nor imported Hopefully that's only on internal classes. It might be prudent to separate this out in a different changeset. It's not always obvious where an exception is thrown. best regards, -- daniel
Re: Need sponsor to fix Javadoc warnings
Hi Pavel, Not a review ... On 8/04/2020 9:50 pm, Pavel Rappo wrote: Vipin, here you go: https://bugs.openjdk.java.net/browse/JDK-8242366 http://cr.openjdk.java.net/~prappo/8242366/webrev.00/ I took the liberty of additionally fixing a couple of parameters' names, a typo, and `@exception` tags for checked exceptions that were neither thrown nor imported. While you are in there is it worth changing @exception to @throws? (I didn't look to see how big that change would be.) Cheers, David The bulk of the change is in Security. Some changes are in Networking. The appropriate mailing lists are in CC for this email. We should wait for their feedback. Changes in core area look good to me and I'd be surprised if there are any problems with the remaining portion of the changeset. -Pavel On 7 Apr 2020, at 19:50, Vipin Sharma wrote: Hi Pavel, On Apr 7, 2020, at 11:11 PM, Pavel Rappo wrote: I assume you have signed the OCA [1]. If not and you want to continue, please do it. If you've already done so, which is probably the case [2], please attach your patch as text to this thread with the next email. Do not use zip or the like. I will take it from there and sponsor that for you. Yes I have signed OCA. -Pavel [1] https://www.oracle.com/technetwork/community/oca-486395.html [2] changeset: 58344:65f30e209890 user:clanger date:Wed Mar 11 13:50:13 2020 +0100 files: test/jdk/java/lang/Boolean/GetBoolean.java test/jdk/java/lang/Boolean/MakeBooleanComparable.java test/jdk/java/lang/Boolean/ParseBoolean.java description: 8240524: Remove explicit type argument in test jdk/java/lang/Boolean/MakeBooleanComparable.java Reviewed-by: clanger, vtewari Contributed-by: vipinsharma85 at gmail.com Yes this is my first contribution. Patch text: --- old/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java 2020-04-06 00:19:10.546117441 +0530 +++ new/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java 2020-04-06 00:19:10.130115855 +0530 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -202,7 +202,7 @@ /** * Sets the padding mechanism of this cipher. * - * @param padding the padding mechanism + * @param paddingScheme the padding mechanism * * @exception NoSuchPaddingException if the requested padding mechanism * does not exist --- old/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java 2020-04-06 00:19:11.526121179 +0530 +++ new/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java 2020-04-06 00:19:11.118119622 +0530 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2004, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2004, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -313,10 +313,10 @@ * current Cipher.engineInit(...) implementation, * IllegalStateException will always be thrown upon invocation. * - * @param in the input buffer - * @param inOffset the offset in in where the input + * @param input the input buffer + * @param inputOffset the offset in in where the input * starts - * @param inLen the input length. + * @param inputLen the input length. * * @return n/a. * --- old/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java 2020-04-06 00:19:12.462124749 +0530 +++ new/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java 2020-04-06 00:19:12.054123193 +0530 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2007, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -130,7 +130,6 @@ * * @param plain the buffer with the input data to be encrypted * @param plainOffset the offset in plain - * @param plainLen the length of the input data * @param cipher the buffer for the result * @param cipherOffset the offset in cipher */ @@ -154,7 +153,6 @@ * * @param cipher the buffer with the input data to be decrypted * @param cipherOffset the offset in cipherOffset - * @param cipherLen the length of the input data * @param plain the buffer for the result * @param plainOffset the offset in plain */ --- old/src/java.base/share/classes/com/sun/crypto/provider/DESCrypt.java 2020-04-06 00:19:13.414128382 +0530 +++ new/src/java.
Re: Need sponsor to fix Javadoc warnings
Vipin, here you go: https://bugs.openjdk.java.net/browse/JDK-8242366 http://cr.openjdk.java.net/~prappo/8242366/webrev.00/ I took the liberty of additionally fixing a couple of parameters' names, a typo, and `@exception` tags for checked exceptions that were neither thrown nor imported. The bulk of the change is in Security. Some changes are in Networking. The appropriate mailing lists are in CC for this email. We should wait for their feedback. Changes in core area look good to me and I'd be surprised if there are any problems with the remaining portion of the changeset. -Pavel > On 7 Apr 2020, at 19:50, Vipin Sharma wrote: > > Hi Pavel, > >> On Apr 7, 2020, at 11:11 PM, Pavel Rappo wrote: >> >> I assume you have signed the OCA [1]. If not and you want to continue, >> please do it. If you've already done so, which is probably the case [2], >> please attach your patch as text to this thread with the next email. Do not >> use zip or the like. I will take it from there and sponsor that for you. > Yes I have signed OCA. >> >> -Pavel >> >> [1] https://www.oracle.com/technetwork/community/oca-486395.html >> [2] changeset: 58344:65f30e209890 >> user:clanger >> date:Wed Mar 11 13:50:13 2020 +0100 >> files: test/jdk/java/lang/Boolean/GetBoolean.java >> test/jdk/java/lang/Boolean/MakeBooleanComparable.java >> test/jdk/java/lang/Boolean/ParseBoolean.java >> description: >> 8240524: Remove explicit type argument in test >> jdk/java/lang/Boolean/MakeBooleanComparable.java >> Reviewed-by: clanger, vtewari >> Contributed-by: vipinsharma85 at gmail.com >> > Yes this is my first contribution. > > Patch text: > > --- old/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java > 2020-04-06 00:19:10.546117441 +0530 > +++ new/src/java.base/share/classes/com/sun/crypto/provider/AESCipher.java > 2020-04-06 00:19:10.130115855 +0530 > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2002, 2017, Oracle and/or its affiliates. All rights > reserved. > + * Copyright (c) 2002, 2020, Oracle and/or its affiliates. All rights > reserved. > * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. > * > * This code is free software; you can redistribute it and/or modify it > @@ -202,7 +202,7 @@ > /** > * Sets the padding mechanism of this cipher. > * > - * @param padding the padding mechanism > + * @param paddingScheme the padding mechanism > * > * @exception NoSuchPaddingException if the requested padding mechanism > * does not exist > --- > old/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java > 2020-04-06 00:19:11.526121179 +0530 > +++ > new/src/java.base/share/classes/com/sun/crypto/provider/AESWrapCipher.java > 2020-04-06 00:19:11.118119622 +0530 > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2004, 2017, Oracle and/or its affiliates. All rights > reserved. > + * Copyright (c) 2004, 2020, Oracle and/or its affiliates. All rights > reserved. > * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. > * > * This code is free software; you can redistribute it and/or modify it > @@ -313,10 +313,10 @@ > * current Cipher.engineInit(...) implementation, > * IllegalStateException will always be thrown upon invocation. > * > - * @param in the input buffer > - * @param inOffset the offset in in where the input > + * @param input the input buffer > + * @param inputOffset the offset in in where the input > * starts > - * @param inLen the input length. > + * @param inputLen the input length. > * > * @return n/a. > * > --- > old/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java > 2020-04-06 00:19:12.462124749 +0530 > +++ > new/src/java.base/share/classes/com/sun/crypto/provider/BlowfishCrypt.java > 2020-04-06 00:19:12.054123193 +0530 > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 1998, 2007, Oracle and/or its affiliates. All rights > reserved. > + * Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights > reserved. > * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. > * > * This code is free software; you can redistribute it and/or modify it > @@ -130,7 +130,6 @@ > * > * @param plain the buffer with the input data to be encrypted > * @param plainOffset the offset in plain > - * @param plainLen the length of the input data > * @param cipher the buffer for the result > * @param cipherOffset the offset in cipher > */ > @@ -154,7 +153,6 @@ > * > * @param cipher the buffer with the input data to be decrypted > * @param cipherOffset the offset in cipherOffset > - * @param cipherLen the length of the input data > * @param plain the buffer for the result > * @param plainOffset the offset in plain > */ > --- old/src/java.base/share/classes/com/sun/crypto/provider/DESCrypt.java > 2020-04-06 00:19:13.414128
Re: RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected
> On 8 Apr 2020, at 10:13, Rahul wrote: > > Updated patch after considering the impact of returning default parameters on > the http client. > TLS versions earlier limited to 1.2 and above by client, now will support all > versions(wrt the scenarios for this bug). > >Issue: https://bugs.openjdk.java.net/browse/JDK-8239595 >Issue: https://bugs.openjdk.java.net/browse/JDK-8239594 > >Webrev: > http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.01/ Thanks for persevering with this. The changes and test look good to me. -Chris.
Re: RFR JDK-8239595/JDK-8239594 : ssl context version is not respected/jdk.tls.client.protocols is not respected
Updated patch after considering the impact of returning default parameters on
the http client.
TLS versions earlier limited to 1.2 and above by client, now will support all
versions(wrt the scenarios for this bug).
Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
Issue: https://bugs.openjdk.java.net/browse/JDK-8239594
Webrev:
http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.01/
-- Rahul
On 27/03/2020, 16:23, "net-dev on behalf of Xuelei Fan"
wrote:
On 3/27/2020 5:52 AM, Chris Hegarty wrote:
> Xuelei,
>
> Before commenting further on the interaction of the HTTP Client with
various contorted configurations, I would like to get a better understanding of
the `jdk.tls.client.protocols` property.
>
> Is there a specification or other documentation describing
`jdk.tls.client.protocols` ?
>
See the jdk.tls.client.protocols line in table 'Table 8-3 System
Properties and Customized Items" in JSSE Reference Guides:
"https://docs.oracle.com/en/java/javase/14/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-A41282C3-19A3-400A-A40F-86F4DA22ABA9
For your quick reference, I copied the note here:
---
Customized Item:
Default handshaking protocols for TLS/DTLS clients.
Notes:
To enable specific SunJSSE protocols on the client, specify them in a
comma-separated list within quotation marks; all other supported
protocols are not enabled on the client
For example,
If jdk.tls.client.protocols="TLSv1,TLSv1.1", then the default
protocol settings on the client for TLSv1 and TLSv1.1 are enabled, while
SSLv3, TLSv1.2, TLSv1.3, and SSLv2Hello are not enabled
If jdk.tls.client.protocols="DTLSv1.2" , then the protocol setting
on the client for DTLS1.2 is enabled, while DTLS1.0 is not enabled
---
> It is my understanding that the property only affects the *default*
protocol’s ( not the supported protocols ) of the *default* context. That is,
the context returned by `SSLContext.getInstance("Default”)`,
It is correct that the property impact the default SSLContext only. The
default SSLContext instance could get from:
SSLContext.getInstance("Default");
SSLContext.getInstance("TLS");
SSLContext.getInstance("DTLS");
> and the protocol values returned by the following invocation on that
context `getDefaultSSLParameters().getProtocols()`. Is this correct? If not,
what does it do?
Yes.
Xuelei
> -Chris.
>
>> On 26 Mar 2020, at 16:58, Xuelei Fan wrote:
>>
>> With this update, the logic looks like: if TLSv1.3 is not enabled in the
SSLContext, use TLSv1.2 instead; Otherwise, use TLSv1.3 and TLSv1.2.
>>
>> There may be a couple of issues:
>> 1. TLSv1.2 may be not enabled, although TLSv1.3 is enabled.
>> For example:
>>System.setProperty("jdk.tls.client.protocols", "TLSv1.3")
>>System.setProperty("jdk.tls.client.protocols", "TLSv1.1, TLSv1.0")
>>
>> 2. TLSv1.2 may be not supported in the SSLContext.
>> For example:
>>SSLContext context = SSLContext.getInstance("DTLS");
>>HttpClient.newBuilder().sslContext(context)...
>>
>> 3. The application may not want to use TLS 1.2.
>> For example:
>>System.setProperty("jdk.tls.client.protocols", "TLSv1.1, TLSv1.0")
>>
>> The System property may be shared by code other than httpclient. So the
setting may not consider the impact on httpclient.
>>
>> I may use enabled protocols only. If no TLSv1.2/TLSv1.3, I may use an
empty protocol array, and test to see what happens in the httpclient
implementation stack.
>>
>> Xuelei
>>
>> On 3/26/2020 9:28 AM, Sean Mullan wrote:
>>> Cross-posting to security-dev as this involves TLS/SSL configuration.
>>> --Sean
>>> On 3/26/20 10:02 AM, rahul.r.ya...@oracle.com wrote:
Hello,
Request to have my fix reviewed for issues:
JDK-8239595 : ssl context version is not respected
JDK-8239594 : jdk.tls.client.protocols is not respected
The fix updates
jdk.internal.net.http.HttpClientImpl.getDefaultParams(SSLContext ctx)
to use ctx.getDefaultSSLParameters()instead of
ctx.getSupportedSSLParameters(),
as the latter does not respect the context parameters set by the user.
Issue: https://bugs.openjdk.java.net/browse/JDK-8239595
Issue: https://bugs.openjdk.java.net/browse/JDK-8239594
Webrev:
http://cr.openjdk.java.net/~jboes/rayayada/webrevs/8239595/webrev.00/
-- Rahul
>
