Re: RFR: 8285890: Fix some @param tags

[Joe Darcy]

On Fri, 29 Apr 2022 09:03:58 GMT, Pavel Rappo  wrote:

> * Syntactically improves a few cases from 8285676 
> (https://github.com/openjdk/jdk/pull/8410)
> * Fixes a few misspelled `@param` tags for elements that, although are not 
> included in the API Documentation, are visible in and processed by IDEs

Thanks for fixing these Pavel.

-

Marked as reviewed by darcy (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8465

Integrated: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces

[Joe Darcy]

On Tue, 26 Apr 2022 22:24:26 GMT, Joe Darcy  wrote:

> To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
> review this PR to add type-level @param tags where they are missing.
> 
> To the maintainers of java.util.concurrent, those changes could be separated 
> out in another bug if that would ease maintenance of that code.
> 
> Making these library fixes is a blocker for correcting and expanding the 
> doclint checks (JDK-8285496).
> 
> I'll update copyright years before pushing.

This pull request has now been integrated.

Changeset: bba456a8
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/bba456a8dbf9027e4b015567c17a79fc7441aa08
Stats: 102 lines in 40 files changed: 76 ins; 0 del; 26 mod

8285676: Add missing @param tags for type parameters on classes and interfaces

Reviewed-by: wetmore, smarks, dfuchs, prr, alanb, mchung

-

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v5]

[Joe Darcy]

> To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
> review this PR to add type-level @param tags where they are missing.
> 
> To the maintainers of java.util.concurrent, those changes could be separated 
> out in another bug if that would ease maintenance of that code.
> 
> Making these library fixes is a blocker for correcting and expanding the 
> doclint checks (JDK-8285496).
> 
> I'll update copyright years before pushing.

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains seven additional commits since the 
last revision:

 - Update copyright years.
 - Merge branch 'master' into JDK-8285676
 - Respond to more review feedback.
 - Respond to more review feedback.
 - Respond to review feedback.
 - Merge branch 'master' into JDK-8285676
 - JDK-8285676: Add missing @param tags for type parameters on classes and 
interfaces

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8410/files
  - new: https://git.openjdk.java.net/jdk/pull/8410/files/aaa8f828..cb1fe1c2

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8410=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8410=03-04

  Stats: 687 lines in 59 files changed: 610 ins; 8 del; 69 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8410.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]

[Joe Darcy]

On Thu, 28 Apr 2022 08:10:38 GMT, Alan Bateman  wrote:

>> Joe Darcy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Respond to more review feedback.
>
> src/java.base/share/classes/java/nio/file/WatchEvent.java line 51:
> 
>> 49: /**
>> 50:  * An event kind, for the purposes of identification.
>> 51:  * @param  the type of the context value
> 
> This is okay but the it differs slightly to the type parameters specified 
> further up. I think the issue here is that it was just wasn't copied down to 
> WatchEvent.Kind.

Okay -- I'll for the T type parameter of the Kind interface I'll reuse the 
wording of the T type parameter of the enclosing WatchEvent interface. (The 
type variable on Kind could be renamed to show that the two type parameters are 
distinct.)

-

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v4]

[Joe Darcy]

> To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
> review this PR to add type-level @param tags where they are missing.
> 
> To the maintainers of java.util.concurrent, those changes could be separated 
> out in another bug if that would ease maintenance of that code.
> 
> Making these library fixes is a blocker for correcting and expanding the 
> doclint checks (JDK-8285496).
> 
> I'll update copyright years before pushing.

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Respond to more review feedback.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8410/files
  - new: https://git.openjdk.java.net/jdk/pull/8410/files/db4919a9..aaa8f828

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8410=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8410=02-03

  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8410.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]

[Joe Darcy]

On Thu, 28 Apr 2022 08:08:37 GMT, Alan Bateman  wrote:

>> Joe Darcy has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Respond to more review feedback.
>
> src/java.base/share/classes/java/nio/file/SecureDirectoryStream.java line 55:
> 
>> 53:  * against the original path of the directory (irrespective of if 
>> the
>> 54:  * directory is moved since it was opened).
>> 55:  * @param  the type of path
> 
> It may not be a path. The type parameter is specified in the super 
> interfaces, can you copy that down instead?

Will change in the next push.

-

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]

[Joe Darcy]

On Wed, 27 Apr 2022 23:24:57 GMT, Stuart Marks  wrote:

>> I said "keys maintained", omitting "by this map" to finesse the question of 
>> if the SimpleEntry class *is* a map, or is used to implement a map, etc. I 
>> can change it to include "by this map" if the map/entry distinction is okay 
>> to be blurred.
>
> Whoops, sorry, this is `SimpleEntry`, which is _not_ a `Map`. In this case 
> I'd follow `Map.Entry` which says "the type of the key" and "the type of the 
> map".

Will change to the Map.Entry wording "the type of key" and "the type of the 
value", respectively.

-

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v3]

[Joe Darcy]

> To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
> review this PR to add type-level @param tags where they are missing.
> 
> To the maintainers of java.util.concurrent, those changes could be separated 
> out in another bug if that would ease maintenance of that code.
> 
> Making these library fixes is a blocker for correcting and expanding the 
> doclint checks (JDK-8285496).
> 
> I'll update copyright years before pushing.

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Respond to more review feedback.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8410/files
  - new: https://git.openjdk.java.net/jdk/pull/8410/files/e0ac5dcb..db4919a9

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8410=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8410=01-02

  Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8410.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v2]

[Joe Darcy]

On Wed, 27 Apr 2022 10:55:22 GMT, Daniel Fuchs  wrote:

>> Joe Darcy has updated the pull request with a new target base due to a merge 
>> or a rebase. The incremental webrev excludes the unrelated changes brought 
>> in by the merge/rebase. The pull request contains three additional commits 
>> since the last revision:
>> 
>>  - Respond to review feedback.
>>  - Merge branch 'master' into JDK-8285676
>>  - JDK-8285676: Add missing @param tags for type parameters on classes and 
>> interfaces
>
> src/java.management/share/classes/javax/management/openmbean/SimpleType.java 
> line 60:
> 
>> (failed to retrieve contents of file, check the PR for context)
> I would suggest to say "that values described by this type"...

Will change to "the Java type that values described by this SimpleType must 
have."

-

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces [v2]

[Joe Darcy]

> To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
> review this PR to add type-level @param tags where they are missing.
> 
> To the maintainers of java.util.concurrent, those changes could be separated 
> out in another bug if that would ease maintenance of that code.
> 
> Making these library fixes is a blocker for correcting and expanding the 
> doclint checks (JDK-8285496).
> 
> I'll update copyright years before pushing.

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains three additional commits since the 
last revision:

 - Respond to review feedback.
 - Merge branch 'master' into JDK-8285676
 - JDK-8285676: Add missing @param tags for type parameters on classes and 
interfaces

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8410/files
  - new: https://git.openjdk.java.net/jdk/pull/8410/files/fe47dd2f..e0ac5dcb

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8410=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8410=00-01

  Stats: 5958 lines in 128 files changed: 4827 ins; 485 del; 646 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8410.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces

[Joe Darcy]

On Wed, 27 Apr 2022 10:54:00 GMT, Daniel Fuchs  wrote:

>> To enable more complete doclint checking (courtesy @jonathan-gibbons), 
>> please review this PR to add type-level @param tags where they are missing.
>> 
>> To the maintainers of java.util.concurrent, those changes could be separated 
>> out in another bug if that would ease maintenance of that code.
>> 
>> Making these library fixes is a blocker for correcting and expanding the 
>> doclint checks (JDK-8285496).
>> 
>> I'll update copyright years before pushing.
>
> src/java.management/share/classes/javax/management/openmbean/ArrayType.java 
> line 96:
> 
>> 94:  *
>> 95:  * @param  the Java type that instances described by this type must
>> 96:  * have.
> 
> Instead of "instances" - would it be more correct to say "array elements"?

Will change to "the Java component type that arrays described by ArrayType must 
have"

-

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces

[Joe Darcy]

On Wed, 27 Apr 2022 01:39:27 GMT, Stuart Marks  wrote:

>> To enable more complete doclint checking (courtesy @jonathan-gibbons), 
>> please review this PR to add type-level @param tags where they are missing.
>> 
>> To the maintainers of java.util.concurrent, those changes could be separated 
>> out in another bug if that would ease maintenance of that code.
>> 
>> Making these library fixes is a blocker for correcting and expanding the 
>> doclint checks (JDK-8285496).
>> 
>> I'll update copyright years before pushing.
>
> src/java.base/share/classes/java/util/AbstractMap.java line 601:
> 
>> 599:  * {@code Map.entrySet().toArray}.
>> 600:  *
>> 601:  * @param  the type of keys maintained
> 
> Please update to match java.util.Map, which says "the type of keys maintained 
> by this map"

I said "keys maintained", omitting "by this map" to finesse the question of if 
the SimpleEntry class *is* a map, or is used to implement a map, etc. I can 
change it to include "by this map" if the map/entry distinction is okay to be 
blurred.

> src/java.base/share/classes/java/util/Dictionary.java line 44:
> 
>> 42:  * @param  the type of keys
>> 43:  * @param  the type of mapped values
>> 44:  *
> 
> Urgh. This class is obsolete, but it was retrofitted to implement Map and was 
> subsequently generified, so I'd update these to match java.util.Map.

The javadoc of Dictionary states "The Dictionary class [...] maps keys to 
values." which was my guide for the wording, but I don't mind changing it.

-

PR: https://git.openjdk.java.net/jdk/pull/8410

RFR: JDK-8285676: Add missing @param tags for type parameters on classes and interfaces

[Joe Darcy]

To enable more complete doclint checking (courtesy @jonathan-gibbons), please 
review this PR to add type-level @param tags where they are missing.

To the maintainers of java.util.concurrent, those changes could be separated 
out in another bug if that would ease maintenance of that code.

Making these library fixes is a blocker for correcting and expanding the 
doclint checks (JDK-8285496).

I'll update copyright years before pushing.

-

Commit messages:
 - JDK-8285676: Add missing @param tags for type parameters on classes and 
interfaces

Changes: https://git.openjdk.java.net/jdk/pull/8410/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8410=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8285676
  Stats: 76 lines in 40 files changed: 76 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8410.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8410/head:pull/8410

PR: https://git.openjdk.java.net/jdk/pull/8410

Re: RFR: 8284105: Update security libraries to use sealed classes

[Joe Darcy]

On Fri, 8 Apr 2022 13:40:37 GMT, Sean Mullan  wrote:

> Please review these changes to update the security libraries to use sealed 
> classes. See JEP 409 for more details on sealed classes.
> 
> No CSR is required as all the changes are to internal classes. A few classes 
> that did not have subclasses were simply marked final instead of sealed.

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/8165

Integrated: JDK-8282686: Add constructors taking a cause to SocketException

[Joe Darcy]

On Fri, 4 Mar 2022 21:17:01 GMT, Joe Darcy  wrote:

> Please review this small API enhancement to add the usual constructors taking 
> a cause to SocketException and then update uses of initiCause on creating 
> SocketException to instead pass the cause via the constructor.
> 
> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688

This pull request has now been integrated.

Changeset: 1faa5c80
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/1faa5c8092f8baec3ca08ed059653876ec46db46
Stats: 102 lines in 7 files changed: 77 ins; 12 del; 13 mod

8282686: Add constructors taking a cause to SocketException

Reviewed-by: alanb, xuelei, lancea, dfuchs

-

PR: https://git.openjdk.java.net/jdk/pull/7705

Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException [v3]

[Joe Darcy]

> Please review this small API enhancement to add the usual constructors taking 
> a cause to SocketException and then update uses of initiCause on creating 
> SocketException to instead pass the cause via the constructor.
> 
> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Improve test.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7705/files
  - new: https://git.openjdk.java.net/jdk/pull/7705/files/978b379d..da863692

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7705=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7705=01-02

  Stats: 11 lines in 1 file changed: 8 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7705.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7705/head:pull/7705

PR: https://git.openjdk.java.net/jdk/pull/7705

Re: RFR: JDK-8282686: Add constructors taking a cause to SocketException [v2]

[Joe Darcy]

> Please review this small API enhancement to add the usual constructors taking 
> a cause to SocketException and then update uses of initiCause on creating 
> SocketException to instead pass the cause via the constructor.
> 
> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains three additional commits since the 
last revision:

 - Add regression test.
 - Merge branch 'master' into JDK-8282686
 - JDK-8282686: Add constructors take a cause to SocketException

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7705/files
  - new: https://git.openjdk.java.net/jdk/pull/7705/files/e65f9ae5..978b379d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7705=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7705=00-01

  Stats: 8268 lines in 175 files changed: 6072 ins; 1565 del; 631 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7705.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7705/head:pull/7705

PR: https://git.openjdk.java.net/jdk/pull/7705

Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines

[Joe Darcy]

On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan  wrote:

> Hi
> 
> I have reviewed the code for removing double semicolons at the end of lines
> 
> all the best
> matteo

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7268

RFR: JDK-8282686: Add constructors taking a cause to SocketException

[Joe Darcy]

Please review this small API enhancement to add the usual constructors taking a 
cause to SocketException and then update uses of initiCause on creating 
SocketException to instead pass the cause via the constructor.

Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282688

-

Commit messages:
 - JDK-8282686: Add constructors take a cause to SocketException

Changes: https://git.openjdk.java.net/jdk/pull/7705/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7705=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8282686
  Stats: 48 lines in 6 files changed: 23 ins; 12 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7705.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7705/head:pull/7705

PR: https://git.openjdk.java.net/jdk/pull/7705

Integrated: JDK-8281082: Improve javadoc references to JOSS

[Joe Darcy]

On Tue, 1 Feb 2022 21:11:39 GMT, Joe Darcy  wrote:

> The references to JOSS, the Java Object Serialization Specification, are not 
> done consistently in the API javadoc. This should be improved.
> 
> I'll update copyright years before pushing.

This pull request has now been integrated.

Changeset: 9ca7ff3e
Author:    Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/9ca7ff3e4f0a944bacf38da7e5426082d64c52bd
Stats: 16 lines in 6 files changed: 3 ins; 0 del; 13 mod

8281082: Improve javadoc references to JOSS

Reviewed-by: iris, rriggs, naoto, lancea

-

PR: https://git.openjdk.java.net/jdk/pull/7315

Re: RFR: JDK-8281082: Improve javadoc references to JOSS [v2]

[Joe Darcy]

> The references to JOSS, the Java Object Serialization Specification, are not 
> done consistently in the API javadoc. This should be improved.
> 
> I'll update copyright years before pushing.

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Update copyrights.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7315/files
  - new: https://git.openjdk.java.net/jdk/pull/7315/files/3eb97175..a4e1eb79

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7315=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7315=00-01

  Stats: 5 lines in 5 files changed: 0 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7315.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7315/head:pull/7315

PR: https://git.openjdk.java.net/jdk/pull/7315

RFR: JDK-8281082: Improve javadoc references to JOSS

[Joe Darcy]

The references to JOSS, the Java Object Serialization Specification, are not 
done consistently in the API javadoc. This should be improved.

I'll update copyright years before pushing.

-

Commit messages:
 - JDK-8281082: Improve javadoc references to JOSS

Changes: https://git.openjdk.java.net/jdk/pull/7315/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7315=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8281082
  Stats: 11 lines in 6 files changed: 3 ins; 0 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7315.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7315/head:pull/7315

PR: https://git.openjdk.java.net/jdk/pull/7315

Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v13]

[Joe Darcy]

On Fri, 28 Jan 2022 16:58:55 GMT, Michael McMahon  wrote:

>> Hi,
>> 
>> This change adds Channel Binding Token (CBT) support to HTTPS 
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, 
>> Kerberos) authentication scheme. When enabled, the implementation 
>> preemptively includes a CBT with authentication requests over Kerberos. The 
>> feature is enabled as follows:
>> 
>> A system property "jdk.spnego.cbt" is defined which can have the values 
>> "never" (default), which means the feature is disabled, "always", which 
>> means the CBT is included for all https Negotiate authentications, or it can 
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of 
>> domains/hosts where the feature is enabled, and disabled everywhere else. In 
>> the given example, the CBT would be included in authentication requests for 
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
>> sub-domains.
>> 
>> A test will be added separately to the implementation.
>> 
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>> 
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   spec update from CSR

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7065

Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v26]

[Joe Darcy]

On Mon, 22 Nov 2021 12:09:30 GMT, Maurizio Cimadamore  
wrote:

>> This PR contains the API and implementation changes for JEP-419 [1]. A more 
>> detailed description of such changes, to avoid repetitions during the review 
>> process, is included as a separate comment.
>> 
>> [1] - https://openjdk.java.net/jeps/419
>
> Maurizio Cimadamore has updated the pull request with a new target base due 
> to a merge or a rebase. The pull request now contains 35 commits:
> 
>  - Merge branch 'master' into JEP-419
>  - Fix javadoc issues found in CSR review
>  - Adopt blessed modofier order
>  - Merge branch 'master' into JEP-419
>  - Revert removal of upcall MH customization
>(This change caused spurious VM crashes, so reverting to baseline)
>  - Further tweak upcall safety considerations
>  - Clarify safety considerations for upcalls
>  - Rename MemorySegment::ofAddressNative to MemorySegment::ofAddress
>(which is consistent with other restricted factories in VaList and 
> NativeSymbol)
>  - Streamline javadoc for package-info
>  - * Add two new CLinker static methods to compute upcall/downcall method 
> types
>* Clarify section on CLinker downcall type
>* Add section on CLinker safety guarantees
>  - ... and 25 more: 
> https://git.openjdk.java.net/jdk/compare/d427c79d...29cc6c60

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5907

Re: RFR: JDK-8276447 Deprecate finalization-related methods for removal

[Joe Darcy]

On Thu, 18 Nov 2021 21:51:30 GMT, Brent Christian  wrote:

> Here are the code changes for the "Deprecate finalizers in the standard Java 
> API" portion of JEP 421 ("Deprecate Finalization for Removal") for code 
> review.
> 
> This change makes the indicated deprecations, and updates the API spec for 
> JEP 421. It also updates the relevant @SuppressWarning annotations.
> 
> The CSR has been approved.
> An automated test build+test run passes cleanly (FWIW :D ).

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6465

Re: RFR: 8276348: Use blessed modifier order in java.base

[Joe Darcy]

On Tue, 2 Nov 2021 16:30:56 GMT, Pavel Rappo  wrote:

> This PR follows up one of the recent PRs, where I used a non-canonical 
> modifier order. Since the problem was noticed [^1], why not to address it at 
> mass?
> 
> As far as I remember, the first mass-canonicalization of modifiers took place 
> in JDK-8136583 in 2015 [^2]. That change affected 1780 lines spanning 453 
> files. Since then modifiers have become a bit lose, and it makes sense to 
> re-bless (using the JDK-8136583 terminology) them.
> 
> This change was produced by running the below command followed by updating 
> the copyright years on the affected files where necessary:
> 
> $ sh ./bin/blessed-modifier-order.sh src/java.base
> 
> The resulting change is much smaller than that of 2015: 39 lines spanning 21 
> files.
> 
> [^1]: 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/082987.html
>  (or https://github.com/openjdk/jdk/pull/6191#pullrequestreview-79465)
> [^2]: 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035217.html

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6213

Integrated: 8275003: Suppress warnings on non-serializable non-transient instance fields in windows mscapi

[Joe Darcy]

On Sat, 9 Oct 2021 19:41:51 GMT, Joe Darcy  wrote:

> Analogous to other recent cleanups like JDK-8274393, suppress warnings for 
> serialization-related issues in the windows mscapi code.

This pull request has now been integrated.

Changeset: 926966be
Author:    Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/926966be7ad91d2b4a750583c78721b2cdb26981
Stats: 5 lines in 2 files changed: 2 ins; 0 del; 3 mod

8275003: Suppress warnings on non-serializable non-transient instance fields in 
windows mscapi

Reviewed-by: valeriep

-

PR: https://git.openjdk.java.net/jdk/pull/5879

RFR: 8275003: Suppress warnings on non-serializable non-transient instance fields in windows mscapi

[Joe Darcy]

Analogous to other recent cleanups like JDK-8274393, suppress warnings for 
serialization-related issues in the windows mscapi code.

-

Commit messages:
 - Applease jcheck.
 - 8275003: Suppress warnings on non-serializable non-transient instance fields 
in windows mscapi

Changes: https://git.openjdk.java.net/jdk/pull/5879/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5879=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8275003
  Stats: 5 lines in 2 files changed: 2 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5879.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5879/head:pull/5879

PR: https://git.openjdk.java.net/jdk/pull/5879

Re: RFR: 8274835: Remove unnecessary castings in java.base

[Joe Darcy]

On Thu, 9 Sep 2021 20:12:47 GMT, Andrey Turbanov 
 wrote:

> Redundant castings make code harder to read.
> Found them by IntelliJ IDEA.
> I tried to select only casts which are definitely safe to remove. Also didn't 
> touch primitive types casts.

Curious. The JDK build is done with javac -Xlint:cast warning enabled 
(JDK-8032734) which is intended to catch issues like this. Perhaps IntelliJ is 
using a different (or sharper) analysis.

-

PR: https://git.openjdk.java.net/jdk/pull/5454

Integrated: 8274393: Suppress more warnings on non-serializable non-transient instance fields in security libs

[Joe Darcy]

On Mon, 27 Sep 2021 19:24:39 GMT, Joe Darcy  wrote:

> Follow-up change to JDK-8231262, augmentations to javac's Xlint:serial 
> checking are out for review (#5709) and various security libraries would need 
> some changes to pass under the expanded checks.
> 
> The changes are to suppress warnings where non-transient fields in 
> serializable types are not declared with a type statically known to be 
> serializable. That isn't necessarily a correctness issues, but it does merit 
> further scrutiny.

This pull request has now been integrated.

Changeset: 73264811
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/7326481143c1321700cbf2caa9e068c5077e22c4
Stats: 41 lines in 12 files changed: 38 ins; 0 del; 3 mod

8274393: Suppress more warnings on non-serializable non-transient instance 
fields in security libs

Reviewed-by: weijun

-

PR: https://git.openjdk.java.net/jdk/pull/5720

Re: RFR: 8274393: Suppress more warnings on non-serializable non-transient instance fields in security libs [v3]

[Joe Darcy]

> Follow-up change to JDK-8231262, augmentations to javac's Xlint:serial 
> checking are out for review (#5709) and various security libraries would need 
> some changes to pass under the expanded checks.
> 
> The changes are to suppress warnings where non-transient fields in 
> serializable types are not declared with a type statically known to be 
> serializable. That isn't necessarily a correctness issues, but it does merit 
> further scrutiny.

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains five additional commits since the 
last revision:

 - Update copyrigths.
 - Merge branch 'master' into JDK-8274393
 - Add explanatory comments for SuppressWarnings annotations.
 - Merge branch 'master' into JDK-8274393
 - 8274393: Suppress more warnings on non-serializable non-transient instance 
fields in security libs

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5720/files
  - new: https://git.openjdk.java.net/jdk/pull/5720/files/08718270..dcb48a0a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5720=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5720=01-02

  Stats: 815 lines in 52 files changed: 604 ins; 55 del; 156 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5720.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5720/head:pull/5720

PR: https://git.openjdk.java.net/jdk/pull/5720

Re: RFR: 8274393: Suppress more warnings on non-serializable non-transient instance fields in security libs [v2]

[Joe Darcy]

> Follow-up change to JDK-8231262, augmentations to javac's Xlint:serial 
> checking are out for review (#5709) and various security libraries would need 
> some changes to pass under the expanded checks.
> 
> The changes are to suppress warnings where non-transient fields in 
> serializable types are not declared with a type statically known to be 
> serializable. That isn't necessarily a correctness issues, but it does merit 
> further scrutiny.

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains three additional commits since the 
last revision:

 - Add explanatory comments for SuppressWarnings annotations.
 - Merge branch 'master' into JDK-8274393
 - 8274393: Suppress more warnings on non-serializable non-transient instance 
fields in security libs

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5720/files
  - new: https://git.openjdk.java.net/jdk/pull/5720/files/5d95dea7..08718270

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5720=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5720=00-01

  Stats: 4821 lines in 185 files changed: 3414 ins; 967 del; 440 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5720.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5720/head:pull/5720

PR: https://git.openjdk.java.net/jdk/pull/5720

Re: RFR: 8274393: Suppress more warnings on non-serializable non-transient instance fields in security libs [v2]

[Joe Darcy]

On Wed, 29 Sep 2021 18:13:14 GMT, Joe Darcy  wrote:

>> Follow-up change to JDK-8231262, augmentations to javac's Xlint:serial 
>> checking are out for review (#5709) and various security libraries would 
>> need some changes to pass under the expanded checks.
>> 
>> The changes are to suppress warnings where non-transient fields in 
>> serializable types are not declared with a type statically known to be 
>> serializable. That isn't necessarily a correctness issues, but it does merit 
>> further scrutiny.
>
> Joe Darcy has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains three additional commits since 
> the last revision:
> 
>  - Add explanatory comments for SuppressWarnings annotations.
>  - Merge branch 'master' into JDK-8274393
>  - 8274393: Suppress more warnings on non-serializable non-transient instance 
> fields in security libs

I added explanatory comments alongside the SuppressWarnings annotations to 
explain their presence.

-

PR: https://git.openjdk.java.net/jdk/pull/5720

RFR: 8274393: Suppress more warnings on non-serializable non-transient instance fields in security libs

[Joe Darcy]

Follow-up change to JDK-8231262, augmentations to javac's Xlint:serial checking 
are out for review (#5709) and various security libraries would need some 
changes to pass under the expanded checks.

The changes are to suppress warnings where non-transient fields in serializable 
types are not declared with a type statically known to be serializable. That 
isn't necessarily a correctness issues, but it does merit further scrutiny.

-

Commit messages:
 - 8274393: Suppress more warnings on non-serializable non-transient instance 
fields in security libs

Changes: https://git.openjdk.java.net/jdk/pull/5720/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5720=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8274393
  Stats: 35 lines in 12 files changed: 35 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5720.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5720/head:pull/5720

PR: https://git.openjdk.java.net/jdk/pull/5720

RFR: 8202056: Expand serial warning to check for bad overloads of serial-related methods and ineffectual fields

[Joe Darcy]

This is an initial PR for expanded lint warnings done under two bugs:

8202056: Expand serial warning to check for bad overloads of serial-related 
methods and ineffectual fields
8160675: Issue lint warning for non-serializable non-transient instance fields 
in serializable type

to get feedback on the general approach and test strategy before further 
polishing the implementation.

The implementation initially started as an annotation processor I wrote several 
years ago. The refined version being incorporated into Attr has been 
refactored, had its checks expanded, and been partially ported to idiomatic 
javac coding style rather than using the javax.lang.model API from annotation 
processing.

Subsequent versions of this PR are expected to move the implementation closer 
to idiomatic javac, in particular to use javac flags rather than 
javax.lang.model.Modifier's. Additional resources keys will be defined for the 
serialization-related fields and methods not having the expected modifiers, 
types, etc. The resource keys for the existing checks related to 
serialVersionUID and reused.

Please also review the corresponding CSRs:

https://bugs.openjdk.java.net/browse/JDK-8274335
https://bugs.openjdk.java.net/browse/JDK-8274336

Informative serialization-related warning messages must take into account 
whether a class, interface, annotation, record, and enum is being analyzed. 
Enum classes and record classes have special handling in serialization. This 
implementation under review has been augmented with checks for interface types 
recommended by Chris Hegarty in an attachment on 8202056.

The JDK build has the Xlint:serial check enabled. The build did not pass with 
the augmented checks. For most modules, this PR contains the library changes 
necessary for the build to pass. I will start separate PRs in those library 
areas to get the needed SuppressWarning("serial") or other changes in place. 
For one module, I temporarily disabled the Xlint:serial check.

In terms of performance, I have not done benchmarks of the JDK build with and 
without these changes, but informally the build seems to take about as long as 
before.

-

Commit messages:
 - Appease jcheck
 - Implement checks chegar recommended for interfaces.
 - Update comment.
 - Add tests for instance field checks.
 - Clean build with instance field checks in place.
 - Merge branch 'master' into JDK-8202056
 - Put Externalizable checks last.
 - Add checks for constructor access in Serializable classes.
 - Add no-arg ctor check for Externalizable classes.
 - Improve Externalization warnings.
 - ... and 19 more: https://git.openjdk.java.net/jdk/compare/5756385c...d498ff5f

Changes: https://git.openjdk.java.net/jdk/pull/5709/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5709=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8202056
  Stats: 1519 lines in 79 files changed: 1511 ins; 1 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5709.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5709/head:pull/5709

PR: https://git.openjdk.java.net/jdk/pull/5709

Re: RFR: 8274075: Fix miscellaneous typos in java.base [v2]

[Joe Darcy]

On Tue, 21 Sep 2021 13:16:02 GMT, Pavel Rappo  wrote:

>> 8274075: Fix miscellaneous typos in java.base
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Tweak wording for Throwable constructor parameters

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5610

Re: RFR: 8272626: Avoid C-style array declarations in java.*

[Joe Darcy]

On 8/18/2021 6:20 AM, Roger Riggs wrote:

On Wed, 18 Aug 2021 10:07:35 GMT, Claes Redestad  wrote:


C-style array declarations generate noisy warnings in IDEs, et.c. This patch 
cleans up all java.* packages.

(Copyrights intentionally not updated due the triviality of most changes)

34 Minutes from proposed to integrated!
Its hard to argue with the efficiency, but only 1 timezone of developers had a 
chance to review or even be aware of the change.



I don't think removing use of this archaic language feature, which 
doesn't change semantics, should be in any way controversial and is long 
overdue.


-Joe

Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v6]

[Joe Darcy]

On 6/21/2021 2:02 PM, Paul Sandoz wrote:

On Mon, 21 Jun 2021 05:17:09 GMT, Yi Yang  wrote:


After JDK-8265518(#3615), it's possible to replace all variants of checkIndex 
by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in 
the whole JDK codebase.

Yi Yang has updated the pull request incrementally with one additional commit 
since the last revision:

   more replacement 2

All the updates to the check* methods look ok (requires some careful looking!).

I cannot recall what others said about the change in exception messages. 
@jddarcy any advice here?


Generally, the JDK does not have the text of exception message as a 
supported interface meant to be relied on by users. This doesn't stop 
developers from occasionally parsing those messages, but that is usually 
a sign of a missing API which we try to rectify more directly.


HTH,

-Joe

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

[Joe Darcy]

On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/4073

Integrated: 8264148: Update spec for exceptions retrofitted for exception chaining

[Joe Darcy]

On Wed, 24 Mar 2021 23:17:46 GMT, Joe Darcy  wrote:

> 8264148: Update spec for exceptions retrofitted for exception chaining

This pull request has now been integrated.

Changeset: 815248ab
Author:    Joe Darcy 
URL:   https://git.openjdk.java.net/jdk/commit/815248ab
Stats: 84 lines in 22 files changed: 8 ins; 44 del; 32 mod

8264148: Update spec for exceptions retrofitted for exception chaining

Reviewed-by: rriggs, smarks

-

PR: https://git.openjdk.java.net/jdk/pull/3182

Re: RFR: 8264148: Update spec for exceptions retrofitted for exception chaining

[Joe Darcy]

On 3/30/2021 6:29 AM, Roger Riggs wrote:

On Wed, 24 Mar 2021 23:17:46 GMT, Joe Darcy  wrote:


8264148: Update spec for exceptions retrofitted for exception chaining

I agree that the public field in WriteAbortedException could be remediated.
But it is also mostly harmless.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/VMObjectFactory.java
 line 62:


60: catch (java.lang.reflect.InvocationTargetException ite) {
61: if (ite.getCause() instanceof RuntimeException) {
62: throw (RuntimeException)ite.getCause();

This might be a place to use the new instanceof pattern form:
`if (ite.getCause() instanceof RuntimeException rex)
 throw rex.getCause();
`

src/jdk.jconsole/share/classes/sun/tools/jconsole/inspector/Utils.java line 293:


291: Throwable t = e.getCause();
292: if (t instanceof Exception) {
293: throw (Exception) t;

Ditto:
  ` if (t instanceof Exception ex) throw ex`



I think the use of the new instanceof form would be better left for a 
follow-up refactoring.


Thanks,

-Joe

Re: RFR: 8264148: Update spec for exceptions retrofitted for exception chaining

[Joe Darcy]

On 3/30/2021 6:43 AM, jmehrens wrote:

On Wed, 24 Mar 2021 23:17:46 GMT, Joe Darcy  wrote:


8264148: Update spec for exceptions retrofitted for exception chaining

src/java.base/share/classes/java/io/WriteAbortedException.java line 86:


84: @Override
85: public Throwable getCause() {
86: return detail;

Use SuppressWarnings??



There is no warning to suppress since the detail field is defined in the 
same class.


-Joe

Re: RFR: 8264148: Update spec for exceptions retrofitted for exception chaining

[Joe Darcy]

On Thu, 25 Mar 2021 18:52:54 GMT, Stuart Marks  wrote:

>> 8264148: Update spec for exceptions retrofitted for exception chaining
>
> The removal of the obsolescent "As of release 1.4, this exception has been 
> retrofitted..." is good. Changing the calls from the other exception-getting 
> methods to `getCause()` is also good. I'm less sure of the utility of 
> deprecating these older methods. The deprecation will issue warning messages; 
> is there any benefit to the calling code to migrating from the older methods 
> to `getCause()`? If they're exactly equivalent, then I think the benefits are 
> small, compared to the cost of dealing with warnings. Thus for most of these 
> cases I think that not deprecating the older methods is reasonable, and 
> perhaps the explanation should be converted to an `@apiNote`.
> 
> (The considerations for the JDK itself are different, though, which is why I 
> support changing the call sites.)
> 
> One special case is the **public field** in `WriteAbortedException`. This is 
> really bad and something ought to be done about this, including deprecation, 
> and maybe more. This implies that the exception is mutable, right? Hrrmph. 
> Isn't there a general rule that once the cause has been set (either via a 
> constructor or via initCause) the exception is immutable? Maybe the field 
> should be deprecated, and `getCause()` should return the cause from the 
> superclass. That's a behavior change of course, and I don't know how to 
> assess the compatibility impact. But the current situation just seems wrong.

Some discussion of the proposed changes here. While the history with respect to 
the introduction of the chained exception mechanism may have been relevant in 
times past, those comments have little utility today. Per guidance from Dr. 
Deprecator, I've removed deprecation of the redundant methods and only kept 
deprecation of the mutable field in WriteAbortedException.

The legacy wrapped exception accessing methods in ClassNotFoundException, 
ExceptionInInitializerError, and UndeclaredThrowableException are equivalent to 
getCause. The legacy method in PrivilegedActionException returns the same 
value, but narrowed to Exception rather than Throwable.

Once the rest of the change is agree to, I'll update copyrights and do a CSR 
for the deprecation and other spec changes.

-

PR: https://git.openjdk.java.net/jdk/pull/3182

RFR: 8264148: Update spec for exceptions retrofitted for exception chaining

[Joe Darcy]

8264148: Update spec for exceptions retrofitted for exception chaining

-

Commit messages:
 - Respond to review feedback.
 - Respond to review feedback.
 - Merge branch 'master' into 8264148
 - Merge branch 'master' into 8264148
 - 8264148: Update spec for exceptions retrofitted for exception chaining

Changes: https://git.openjdk.java.net/jdk/pull/3182/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3182=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264148
  Stats: 84 lines in 22 files changed: 8 ins; 44 del; 32 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3182.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3182/head:pull/3182

PR: https://git.openjdk.java.net/jdk/pull/3182

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v37]

[Joe Darcy]

On Fri, 26 Mar 2021 12:25:43 GMT, Jim Laskey  wrote:

>> This PR is to introduce a new random number API for the JDK. The primary API 
>> is found in RandomGenerator and RandomGeneratorFactory. Further description 
>> can be found in the JEP https://openjdk.java.net/jeps/356 .
>> 
>> javadoc can be found at 
>> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html
>> 
>> old PR:  https://github.com/openjdk/jdk/pull/1273
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   CSR review revisions

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/1292

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v32]

[Joe Darcy]

On Thu, 18 Mar 2021 15:08:56 GMT, Jim Laskey  wrote:

>> This PR is to introduce a new random number API for the JDK. The primary API 
>> is found in RandomGenerator and RandomGeneratorFactory. Further description 
>> can be found in the JEP https://openjdk.java.net/jeps/356 .
>> 
>> javadoc can be found at 
>> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html
>> 
>> old PR:  https://github.com/openjdk/jdk/pull/1273
>
> Jim Laskey has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 67 commits:
> 
>  - Merge branch 'master' into 8248862
>  - Review revisions
>  - Missing @since
>  - Review revisions
>  - Review requested changes
>  - Merge branch 'master' into 8248862
>  - Remove conflicts
>  - Use isAnnotationPresent
>  - Introduce isDeprecated
>  - Update javadoc
>  - ... and 57 more: 
> https://git.openjdk.java.net/jdk/compare/8c8d1b31...63094f9d

Changes requested by darcy (Reviewer).

src/java.base/share/classes/java/util/random/RandomGenerator.java line 1290:

> 1288:  * @return a new object that is a copy of this generator
> 1289:  */
> 1290: LeapableGenerator copy();

Suggest adding an Override annotation here and possibly to inheritDoc the text 
from Jumpable.copy.

src/java.base/share/classes/java/util/random/RandomGenerator.java line 1455:

> 1453:  *  period of this generator
> 1454:  */
> 1455: void jump(double distance);

Suggest Override and inheritDoc combo.

src/java.base/share/classes/java/util/random/RandomGenerator.java line 1465:

> 1463:  * @implSpec The default implementation invokes 
> jump(jumpDistance()).
> 1464:  */
> 1465: default void jump() { jump(jumpDistance()); }

Should be able to avoid defining the jump method in this subinterface.

src/java.base/share/classes/java/util/random/RandomGenerator.java line 1486:

> 1484:  * ({@link Long#MAX_VALUE Long.MAX_VALUE}).
> 1485:  */
> 1486: default Stream jumps(double 
> distance) {

Suggest adding an Override annotation.

src/java.base/share/classes/java/util/random/RandomGenerator.java line 1521:

> 1519:  * {@link ArbitrarilyJumpableGenerator#leapDistance() 
> leapDistance}().
> 1520:  */
> 1521: default void leap() { jump(leapDistance()); }

Should need to define this method in the subinterface of Leapable.

src/java.base/share/classes/java/util/random/RandomGenerator.java line 1538:

> 1536:  * returns the copy.
> 1537:  */
> 1538: default ArbitrarilyJumpableGenerator copyAndJump(double 
> distance) {

Suggest Override and inheritDoc.

-

PR: https://git.openjdk.java.net/jdk/pull/1292

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v31]

[Joe Darcy]

On Mon, 15 Mar 2021 12:54:32 GMT, Jim Laskey  wrote:

>> This PR is to introduce a new random number API for the JDK. The primary API 
>> is found in RandomGenerator and RandomGeneratorFactory. Further description 
>> can be found in the JEP https://openjdk.java.net/jeps/356 .
>> 
>> javadoc can be found at 
>> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html
>> 
>> old PR:  https://github.com/openjdk/jdk/pull/1273
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Missing @since

src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line 62:

> 60: @Retention(RetentionPolicy.RUNTIME)
> 61: @Target(ElementType.TYPE)
> 62: public @interface RandomGeneratorProperties {

Should the is-deprecated information be stored here?

-

PR: https://git.openjdk.java.net/jdk/pull/1292

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v31]

[Joe Darcy]

On Mon, 15 Mar 2021 12:54:32 GMT, Jim Laskey  wrote:

>> This PR is to introduce a new random number API for the JDK. The primary API 
>> is found in RandomGenerator and RandomGeneratorFactory. Further description 
>> can be found in the JEP https://openjdk.java.net/jeps/356 .
>> 
>> javadoc can be found at 
>> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html
>> 
>> old PR:  https://github.com/openjdk/jdk/pull/1273
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Missing @since

src/java.base/share/classes/java/util/Random.java line 135:

> 133:  * number generator which is maintained by method {@link #next}.
> 134:  *
> 135:  * @implSpec The invocation {@code new Random(seed)} is 
> equivalent to:

This is not conventional formatting for an implSpec section. I suggest putting 
implSpec on its own line and then left-justifying the text as normal. A new 
paragraph tag is no needed at the beginning of implSpec.

-

PR: https://git.openjdk.java.net/jdk/pull/1292

Re: RFR: 8263105: security-libs doclint cleanup [v2]

[Joe Darcy]

On Tue, 9 Mar 2021 00:56:25 GMT, Bradford Wetmore  wrote:

>> Fix various things pointed out by the most recent doclint run in the 
>> security-libs area.
>> 
>> This is docs only:  I will be checking doccheck/doclint, and will be running 
>> tier1/tier2 tests.  Minor spot checks on generated files.
>
> Bradford Wetmore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Codereview Comment

Looks fine.

-

Marked as reviewed by darcy (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2856

Re: RFR: 8263190: Update java.io, java.math, and java.text to use instanceof pattern variable

[Joe Darcy]

On Mon, 8 Mar 2021 18:48:30 GMT, Patrick Concannon  
wrote:

> Hi,
> 
> Could someone please review my code for updating the code in the `java.io`, 
> `java.math`, and `java.text` packages to make use of the `instanceof` pattern 
> variable?
> 
> Kind regards,
> Patrick

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/2879

Re: RFR: JDK-8262875: doccheck: empty paragraphs, etc in java.base module

[Joe Darcy]

On Tue, 2 Mar 2021 19:35:47 GMT, Jonathan Gibbons  wrote:

> Please review some minor doc fixes, for issues found by _doccheck_.There 
> are two kinds of errors that are addressed.
> 
> 1. Incorrect use of ``. In HTML, `` marks the *beginning* of a 
> paragraph. It is not a terminator, to mark the end of a paragraph, or a 
> separator to mark the boundary between paragraphs.  In particular, it should 
> not be used at the end of a description before a javadoc block tag, such as 
> `@param` or before other HTML block tags, like `` or ``.
> 
> 2. References to the id `package-description`, following the recent 
> standardization of all ids generated by javadoc,

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/2795

Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v21]

[Joe Darcy]

On Fri, 19 Feb 2021 12:48:05 GMT, Jim Laskey  wrote:

>> This PR is to introduce a new random number API for the JDK. The primary API 
>> is found in RandomGenerator and RandomGeneratorFactory. Further description 
>> can be found in the JEP https://openjdk.java.net/jeps/356 .
>> 
>> javadoc can be found at 
>> http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html
>> 
>> old PR:  https://github.com/openjdk/jdk/pull/1273
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Remove tabs from random/package-info.java

src/java.base/share/classes/java/util/random/package-info.java line 193:

> 191:  *
> 192:  *
> 193:  * Random Number Generator Algorithms Available 
> in Java SE

Some comments and questions on the spec status of this table: is the 
intentional to require all of these algorithms in all compliant implementation 
of Java SE or just in the JDK reference implementation? Is the list intended to 
be exhaustive, meaning no other algorithms should be findable?

I recommend clarifying the intended Java SE vs JDK status here. Also, I 
recommend including wording along the lines of "this list may change in the 
future" and "an implementation may provide additional algorithms" etc.

Also, to aid future evolution of the set of algorithms, was there consideration 
of an "isDeprecated" predicate so that algorithms could be so-marked for a 
while before being dropped?

-

PR: https://git.openjdk.java.net/jdk/pull/1292

Integrated: 8250564: Remove terminally deprecated constructor in GSSUtil

[Joe Darcy]

On Tue, 5 Jan 2021 21:02:21 GMT, Joe Darcy  wrote:

> Back in JDK 16, two unintended default constructors were identified and 
> deprecated for removal. The time has come to remove them.
> 
> Please also review the corresponding CSRs:
> 
> JDK-8258521 Remove terminally deprecated constructor in GSSUtil 
> https://bugs.openjdk.java.net/browse/JDK-8258521
> 
>  JDK-8258522 Remove terminally deprecated constructor in java.net.URLDecoder 
> https://bugs.openjdk.java.net/browse/JDK-8258522
> 
> Calling a static method using an instance of a class, as done in the test 
> B6463990.java, is a coding anti-pattern that generates a lint warning; that 
> warning in enabled in the JDK build.
> 
> Thanks,

This pull request has now been integrated.

Changeset: 80544e4d
Author:Joe Darcy 
URL:   https://git.openjdk.java.net/jdk/commit/80544e4d
Stats: 7 lines in 3 files changed: 0 ins; 3 del; 4 mod

8250564: Remove terminally deprecated constructor in GSSUtil
8250565: Remove terminally deprecated constructor in java.net.URLDecoder

Reviewed-by: bpb, smarks, alanb, mullan

-

PR: https://git.openjdk.java.net/jdk/pull/1948

RFR: 8250564: Remove terminally deprecated constructor in GSSUtil

[Joe Darcy]

Back in JDK 16, two unintended default constructors were identified and 
deprecated for removal. The time has come to remove them.

Please also review the corresponding CSRs:

JDK-8258521 Remove terminally deprecated constructor in GSSUtil 
https://bugs.openjdk.java.net/browse/JDK-8258521

 JDK-8258522 Remove terminally deprecated constructor in java.net.URLDecoder 
https://bugs.openjdk.java.net/browse/JDK-8258522

Calling a static method using an instance of a class, as done in the test 
B6463990.java, is a coding anti-pattern that generates a lint warning; that 
warning in enabled in the JDK build.

Thanks,

-

Commit messages:
 - 8250564: Remove terminally deprecated constructor in GSSUti

Changes: https://git.openjdk.java.net/jdk/pull/1948/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1948=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8250564
  Stats: 7 lines in 3 files changed: 0 ins; 3 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1948.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1948/head:pull/1948

PR: https://git.openjdk.java.net/jdk/pull/1948

Re: RFR: JDK-8255262: Remove use of legacy custom @spec tag

[Joe Darcy]

On Thu, 22 Oct 2020 17:16:23 GMT, Jonathan Gibbons  wrote:

> The change is (just) to remove legacy usages of a JDK-private custom tag.

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/814

Re: JDK 16 RFR of JDK-8250246: Address reliance on default constructors in security libs

[Joe Darcy]

On 7/24/2020 10:32 AM, Sean Mullan wrote:

On 7/24/20 1:18 PM, Joe Darcy wrote:

- 
src/jdk.security.jgss/share/classes/com/sun/security/jgss/GSSUtil.java


  37 /**
  38  * Do not call.
  39  */
  40 @Deprecated(since="16", forRemoval=true)
  41 public GSSUtil() {}

Is your concern that there may be some code out there that might be 
erroneously using the default constructor so you want to give some 
warning first before making this a private ctor after it is removed? 
(I think I agree, but I want to make sure I understand your thinking).


That is my concern here, yes. We've had a number of other cases in 
the JDK where the default constructor appeared in an static-only 
class as an attractive nuisance. Before static imports, subclassing 
such a class was a way to get its names in the namespace of the 
subclass, but that is an anti-pattern that should not be encouraged.


Ok, sounds good.

src/jdk.security.auth/share/classes/com/sun/security/auth/module/NTLoginModule.java 



87  * Creates a {@code NTLoginModule}.

s/a/an/


Hmm. If the this is pronounced as "N T Login Module", "N" would be a 
consonant sound so should be prefixed by the indefinite article "a", no?


This might be a question for one of the grammar experts amongst us. 
But isn't "N" a vowel sound: "en"?




I suppose looked at that way it is :-)

I'll make the adjustments, copyrights, etc. and push this later today.

Thanks,

-Joe

Re: JDK 16 RFR of JDK-8250246: Address reliance on default constructors in security libs

[Joe Darcy]

Hi Sean,

On 7/24/2020 4:52 AM, Sean Mullan wrote:

Hi Joe,

On 7/24/20 1:17 AM, Joe Darcy wrote:

Hello,

Please review a set of changes to add explicit constructors to 
replace default (implicit) constructors in various classes in 
security libs across several modules:


 JDK-8250246: Address reliance on default constructors in 
security libs

 http://cr.openjdk.java.net/~darcy/8250246.0/
 CSR: https://bugs.openjdk.java.net/browse/JDK-8250248

(This is a companion to similar changes being made across other 
libraries in preparation for creating a new javac lint warning.)


One of the classes seems to only accidentally have a constructor, so 
I added that one as terminally deprecated.


You have not updated the copyright dates; not sure if that is 
necessary for this type of change.


Right; I run a script that handles the copyright updates before a push 
-- less clutter in a review like this where the per-file changes is 
small and reduces the possibility of spurious merge conflicts for the 
copyright line during the review period.





- src/jdk.security.jgss/share/classes/com/sun/security/jgss/GSSUtil.java

  37 /**
  38  * Do not call.
  39  */
  40 @Deprecated(since="16", forRemoval=true)
  41 public GSSUtil() {}

Is your concern that there may be some code out there that might be 
erroneously using the default constructor so you want to give some 
warning first before making this a private ctor after it is removed? 
(I think I agree, but I want to make sure I understand your thinking).


That is my concern here, yes. We've had a number of other cases in the 
JDK where the default constructor appeared in an static-only class as an 
attractive nuisance. Before static imports, subclassing such a class was 
a way to get its names in the namespace of the subclass, but that is an 
anti-pattern that should not be encouraged.





- 
src/jdk.security.auth/share/classes/com/sun/security/auth/module/NTLoginModule.java


87  * Creates a {@code NTLoginModule}.

s/a/an/


Hmm. If the this is pronounced as "N T Login Module", "N" would be a 
consonant sound so should be prefixed by the indefinite article "a", no?





- 
src/jdk.security.auth/share/classes/com/sun/security/auth/module/LdapLoginModule.java


359  * Creates a {@code LdapLoginModule}.

s/a/an/


Okay.

Thanks,

-Joe

JDK 16 RFR of JDK-8250246: Address reliance on default constructors in security libs

[Joe Darcy]

Hello,

Please review a set of changes to add explicit constructors to replace 
default (implicit) constructors in various classes in security libs 
across several modules:


    JDK-8250246: Address reliance on default constructors in security libs
    http://cr.openjdk.java.net/~darcy/8250246.0/
    CSR: https://bugs.openjdk.java.net/browse/JDK-8250248

(This is a companion to similar changes being made across other 
libraries in preparation for creating a new javac lint warning.)


One of the classes seems to only accidentally have a constructor, so I 
added that one as terminally deprecated.


Thanks,

-Joe

JDK 16 RFR of JDK-8247374: Remove default constructors from javax.net.ssl

[Joe Darcy]

Hello,

Please review the addition of several explicit constructors to abstract 
classes in javax.net.ssl to remove the use of implicit default 
constructors; CSR link and patch below:


    JDK-8247374: Remove default constructors from javax.net.ssl
    CSR: https://bugs.openjdk.java.net/browse/JDK-8247375
    webrev: http://cr.openjdk.java.net/~darcy/8247374.0/

Thanks,

-Joe

diff -r d9daa4ce8017 
src/java.base/share/classes/javax/net/ssl/ExtendedSSLSession.java
--- a/src/java.base/share/classes/javax/net/ssl/ExtendedSSLSession.java 
Wed Jun 10 13:29:44 2020 -0700
+++ b/src/java.base/share/classes/javax/net/ssl/ExtendedSSLSession.java 
Wed Jun 10 16:09:50 2020 -0700

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2017, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2010, 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
@@ -35,6 +35,11 @@
  */
 public abstract class ExtendedSSLSession implements SSLSession {
 /**
+ * Constructor for subclasses to call.
+ */
+    public ExtendedSSLSession() {}
+
+    /**
  * Obtains an array of supported signature algorithms that the 
local side

  * is willing to use.
  * 
diff -r d9daa4ce8017 
src/java.base/share/classes/javax/net/ssl/KeyManagerFactorySpi.java
--- 
a/src/java.base/share/classes/javax/net/ssl/KeyManagerFactorySpi.java 
Wed Jun 10 13:29:44 2020 -0700
+++ 
b/src/java.base/share/classes/javax/net/ssl/KeyManagerFactorySpi.java 
Wed Jun 10 16:09:50 2020 -0700

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2001, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1999, 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
@@ -41,6 +41,11 @@
  */
 public abstract class KeyManagerFactorySpi {
 /**
+ * Constructor for subclasses to call.
+ */
+    public KeyManagerFactorySpi() {}
+
+    /**
  * Initializes this factory with a source of key material.
  *
  * @param ks the key store or null
diff -r d9daa4ce8017 
src/java.base/share/classes/javax/net/ssl/SSLContextSpi.java
--- a/src/java.base/share/classes/javax/net/ssl/SSLContextSpi.java Wed 
Jun 10 13:29:44 2020 -0700
+++ b/src/java.base/share/classes/javax/net/ssl/SSLContextSpi.java Wed 
Jun 10 16:09:50 2020 -0700

@@ -40,6 +40,11 @@
  */
 public abstract class SSLContextSpi {
 /**
+ * Constructor for subclasses to call.
+ */
+    public SSLContextSpi() {}
+
+    /**
  * Initializes this context.
  *
  * @param km the sources of authentication keys
diff -r d9daa4ce8017 
src/java.base/share/classes/javax/net/ssl/TrustManagerFactorySpi.java
--- 
a/src/java.base/share/classes/javax/net/ssl/TrustManagerFactorySpi.java 
Wed Jun 10 13:29:44 2020 -0700
+++ 
b/src/java.base/share/classes/javax/net/ssl/TrustManagerFactorySpi.java 
Wed Jun 10 16:09:50 2020 -0700

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2008, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1999, 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
@@ -41,6 +41,11 @@
  */
 public abstract class TrustManagerFactorySpi {
 /**
+ * Constructor for subclasses to call.
+ */
+    public TrustManagerFactorySpi() {}
+
+    /**
  * Initializes this factory with a source of certificate
  * authorities and related trust material.
  *
diff -r d9daa4ce8017 
src/java.base/share/classes/javax/net/ssl/X509ExtendedTrustManager.java
--- 
a/src/java.base/share/classes/javax/net/ssl/X509ExtendedTrustManager.java 
Wed Jun 10 13:29:44 2020 -0700
+++ 
b/src/java.base/share/classes/javax/net/ssl/X509ExtendedTrustManager.java 
Wed Jun 10 16:09:50 2020 -0700

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2015, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2010, 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
@@ -55,6 +55,11 @@
  */
 public abstract class X509ExtendedTrustManager implements 
X509TrustManager {

 /**
+ * Constructor for subclasses to call.
+ */
+    public X509ExtendedTrustManager() {}
+
+    /**
  * Given the partial or complete certificate chain provided by the
  * peer, build and validate the certificate path based on the
  * authentication type and ssl parameters.

Re: RFR(S): 8236111 : narrow allowSmartActionArgs disabling

[Joe Darcy]

PS Some additional context, the "smart action args" feature is being 
commonly used is that regression tests for preview features can say


    --enable-preview -source {$jdk.version}

without needing to be updated explicitly for each release.

HTH,

-Joe

On 1/14/2020 2:29 PM, Igor Ignatyev wrote:

Hi Sean,

allowSmartActionArgs has been added quite recently to jtreg by CODETOOLS-7902352[1], it 
is a jtreg feature which replaces ${X} in jtreg actions (such as @run) with the value of 
X where X is either a "test" system property[2] or a name from @requires 
context[3].

[1] https://bugs.openjdk.java.net/browse/CODETOOLS-7902352
[2] 
https://hg.openjdk.java.net/code-tools/jtreg/raw-file/90e9eef3d433/src/share/doc/javatest/regtest/tag-spec.html#testvars
[3] 
https://hg.openjdk.java.net/code-tools/jtreg/raw-file/90e9eef3d433/src/share/doc/javatest/regtest/tag-spec.html#requires_names

Thanks,
-- Igor



On Jan 14, 2020, at 2:17 PM, Sean Mullan  wrote:

dropping core-libs-dev and hotspot-dev

On 1/14/20 12:03 PM, Igor Ignatyev wrote:

Joe and Roger, thank you for your reviews.
security-libs guys, could you please take a look?

Sure, but what is allowSmartActionArgs? I'm assuming this is a jtreg feature 
but I have not heard of it before.

Thanks,
Sean



Thanks,
-- Igor

On Jan 2, 2020, at 12:58 PM, Roger Riggs  wrote:

The core lib changes look ok.

Roger
On Jan 2, 2020, at 1:26 PM, Joe Darcy  wrote:

The removal of the existing TEST.properties files look fine.

Please also solicit feedback from the security libs team as their area is 
affected.

Roger, FYI the serial filter tests are updated as part of this changeset.

Cheers,

-Joe

On 12/23/2019 8:13 PM, Igor Ignatyev wrote:

Thanks David.

core-libs folks, could you please review jdk part of this patch?

Thanks,
-- Igor


On Dec 23, 2019, at 1:33 PM, David Holmes  wrote:

Hi Igor,

Hotspot changes seem fine. Can't comment on jdk tests.

Thanks,
David

On 24/12/2019 6:42 am, Igor Ignatyev wrote:

ping?

On Dec 17, 2019, at 11:30 AM, Igor Ignatyev  wrote:

http://cr.openjdk.java.net/~iignatyev/8236111/webrev.00/

31 lines changed: 20 ins; 11 del; 0 mod;

Hi all,

could you please review this small patch which enables allowSmartActionArgs in 
hotspot and jdk test suites and disables them in a small number of test 
directories? the patch also removes TEST.properties files which enabled 
allowSmartActionArgs as they aren't needed anymore.

from JBS:

currently, allowSmartActionArgs is disabled for the whole hotspot and jdk test 
suites and enabled just in few places. this makes it a bit harder for people to 
use smart action arguments in these test suites as they have to not to forget 
to enable them. and given in all the other test suites, smart action arguments 
are enabled, it can be confusing and frustrating.

testing: tier1-5
JBS: https://bugs.openjdk.java.net/browse/JDK-8236111
webrev: http://cr.openjdk.java.net/~iignatyev/8236111/webrev.00/

Thanks,
-- Igor

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

[Joe Darcy]

Hi Chris and Sean,

I'll push a fix for JDK-8231262 with a single class-level suppression in 
X509CertImpl:


    @SuppressWarnings("serial") // See writeReplace method in Certificate

I've filed

        JDK-8232062: Clarify serialization mechanisms of X509CertImpl

for the follow-up work.

Thanks,

-Joe

On 10/9/2019 7:14 AM, Chris Hegarty wrote:



On 09/10/2019 14:54, Sean Mullan wrote:

...

X509CertImpl extends X509Certificate which extends Certificate. 
Certificate has a writeReplace method.


Another possible follow-on is to add readObject methods, that 
unconditionally throw, to both X509Certificate and X509CertImpl, since 
serialized instances of these types should not appear in the stream. 
That would be a nice addition to the suggestion to make all the fields 
transient - and improve the readability of the code.


-Chris.

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

[Joe Darcy]

Hi Sean,

Getting back to this review...

On 9/26/2019 1:55 PM, Sean Mullan wrote:

On 9/26/19 4:20 PM, Sean Mullan wrote:
Would you prefer I revise the patch where there are multiple 
SuppressWarnings("serial") on fields to put a single one on the 
class instead?


Yes, but only in the cases where we are clearly using some form of 
alternate serialization like ASN.1 encoding. I need to double-check 
the review again (it's a bit more time consuming because I have to 
look at the code in more detail), but the two that I spotted so far are:


src/java.base/share/classes/sun/security/x509/X509CertImpl.java
src/java.security.jgss/share/classes/sun/security/krb5/internal/KRBError.java 
(from the JDK-8231368 review)


Ok, I double-checked everything. The only other class in the webrev 
that uses an alternate serial form is:


sun/security/x509/X509Key.java

but since that only has one field that is not Serializable, it 
probably is ok to leave as-is.




I've put the updated webrev at:

    http://cr.openjdk.java.net/~darcy/8231262.1/

The difference from the first webrev is I marked a field in X509Key.java 
as transient; diff of webrev patches:


> --- old/src/java.base/share/classes/sun/security/x509/X509Key.java 
2019-10-08 18:37:02.143999531 -0700
> +++ new/src/java.base/share/classes/sun/security/x509/X509Key.java 
2019-10-08 18:37:01.815999531 -0700

> @@ -84,7 +84,7 @@
214,215c214,215
< +    @SuppressWarnings("serial") // Not statically typed as Serializable
<  private BitArray bitStringKey = null;
---
> -    private BitArray bitStringKey = null;
> +    private transient BitArray bitStringKey = null;

For src/java.base/share/classes/sun/security/x509/X509CertImpl.java, I 
don't see any of the read* or write* methods associated with 
serialization defined in the class. I would seem clearer to me to having 
the @SuppressWarning("serial") at each field, but if you'd prefer to 
have it at the class level, I'll defer to your judgement on the matter.


FYI, of the files being modified only 
src/java.base/share/classes/javax/security/auth/Subject.java uses 
readFields or putFields.


Thanks,

-Joe

Re: JDK 14 RFR of JDK-8231368: Suppress warnings on non-serializable non-transient instance fields in java.security.jgss

[Joe Darcy]

Hi Sean,

Amended as requested before pushing; thanks,

-Joe

On 10/8/2019 2:12 PM, Sean Mullan wrote:
I would change "asn1" to "ASN.1" in the comment as that is the more 
common usage of the acronym, otherwise looks good.


Thanks,
Sean

On 10/8/19 1:36 PM, Joe Darcy wrote:
PS And a revised webrev acting on comments from the JDK-8231262 to 
use a single class-level @SuppressWarnings when an alternative serial 
form is implicitly being used:


 http://cr.openjdk.java.net/~darcy/8231368.1/

Thanks,

-Joe

On 10/8/2019 10:11 AM, Joe Darcy wrote:

Hi Sean,

Returning to this review

On 9/26/2019 12:35 PM, Sean Mullan wrote:

- Krb5Context.java

1394 @SuppressWarnings("serial") // Not statically typed as 
Serializable

1395 private final EncryptionKey key;

EncryptionKey is Serializable (it derives from java.security.Key 
which is Serializable). I was wondering why we needed to suppress 
the warning here.



Taking a closer look, the field in question is of type

    sun.security.krb5.EncryptionKey

which is *not* declared to be Serializable:

public class EncryptionKey
    implements Cloneable {

In contrast, the javax.security.auth.kerberos.EncryptionKey class is 
declared to be Serializable. Therefore, the @SuppressWarnings on the 
field in the initial patch is needed.


If the patch looks good, I'll get this pushed.

Thanks,

-Joe



--Sean

On 9/23/19 8:15 PM, Joe Darcy wrote:

Hello,

Another module, another review request as part of making serial 
warnings more robust:


 JDK-8231368: Suppress warnings on non-serializable 
non-transient instance fields in java.security.jgss

 http://cr.openjdk.java.net/~darcy/8231368.0/

(Related earlier review 
https://mail.openjdk.java.net/pipermail/security-dev/2019-September/020672.html.) 



In this latest review, I included a comment in KRBError.java that 
its writeObject method uses a different encoding scheme.


Thanks,

-Joe

Re: JDK 14 RFR of JDK-8231368: Suppress warnings on non-serializable non-transient instance fields in java.security.jgss

[Joe Darcy]

PS And a revised webrev acting on comments from the JDK-8231262 to use a 
single class-level @SuppressWarnings when an alternative serial form is 
implicitly being used:


    http://cr.openjdk.java.net/~darcy/8231368.1/

Thanks,

-Joe

On 10/8/2019 10:11 AM, Joe Darcy wrote:

Hi Sean,

Returning to this review

On 9/26/2019 12:35 PM, Sean Mullan wrote:

- Krb5Context.java

1394 @SuppressWarnings("serial") // Not statically typed as 
Serializable

1395 private final EncryptionKey key;

EncryptionKey is Serializable (it derives from java.security.Key 
which is Serializable). I was wondering why we needed to suppress the 
warning here.



Taking a closer look, the field in question is of type

    sun.security.krb5.EncryptionKey

which is *not* declared to be Serializable:

public class EncryptionKey
    implements Cloneable {

In contrast, the javax.security.auth.kerberos.EncryptionKey class is 
declared to be Serializable. Therefore, the @SuppressWarnings on the 
field in the initial patch is needed.


If the patch looks good, I'll get this pushed.

Thanks,

-Joe



--Sean

On 9/23/19 8:15 PM, Joe Darcy wrote:

Hello,

Another module, another review request as part of making serial 
warnings more robust:


 JDK-8231368: Suppress warnings on non-serializable 
non-transient instance fields in java.security.jgss

 http://cr.openjdk.java.net/~darcy/8231368.0/

(Related earlier review 
https://mail.openjdk.java.net/pipermail/security-dev/2019-September/020672.html.) 



In this latest review, I included a comment in KRBError.java that 
its writeObject method uses a different encoding scheme.


Thanks,

-Joe

Re: JDK 14 RFR of JDK-8231368: Suppress warnings on non-serializable non-transient instance fields in java.security.jgss

[Joe Darcy]

Hi Sean,

Returning to this review

On 9/26/2019 12:35 PM, Sean Mullan wrote:

- Krb5Context.java

1394 @SuppressWarnings("serial") // Not statically typed as 
Serializable

1395 private final EncryptionKey key;

EncryptionKey is Serializable (it derives from java.security.Key which 
is Serializable). I was wondering why we needed to suppress the 
warning here.



Taking a closer look, the field in question is of type

    sun.security.krb5.EncryptionKey

which is *not* declared to be Serializable:

public class EncryptionKey
    implements Cloneable {

In contrast, the javax.security.auth.kerberos.EncryptionKey class is 
declared to be Serializable. Therefore, the @SuppressWarnings on the 
field in the initial patch is needed.


If the patch looks good, I'll get this pushed.

Thanks,

-Joe



--Sean

On 9/23/19 8:15 PM, Joe Darcy wrote:

Hello,

Another module, another review request as part of making serial 
warnings more robust:


 JDK-8231368: Suppress warnings on non-serializable non-transient 
instance fields in java.security.jgss

 http://cr.openjdk.java.net/~darcy/8231368.0/

(Related earlier review 
https://mail.openjdk.java.net/pipermail/security-dev/2019-September/020672.html.) 



In this latest review, I included a comment in KRBError.java that its 
writeObject method uses a different encoding scheme.


Thanks,

-Joe

Re: RFR (S) 8230407 : SocketPermission and FilePermission action list allows leading comma

[Joe Darcy]

Hello,

At least from a quick reading, either the spec change or the behavior 
change would seem to merit a CSR.


Cheers,

-Joe

On 10/2/2019 4:26 PM, Ivan Gerasimov wrote:

Hi Chris!

Thank you very much for review!

I agree that it makes sense to update the javadoc for consistency.

I don't think CSR is required in this case, is it? (IAE is unchecked 
anyway, and the fix doesn't really change the behavior.)


Here's the updated webrev:

http://cr.openjdk.java.net/~igerasim/8230407/01/webrev/

With kind regards,

Ivan


On 10/2/19 6:44 AM, Chris Hegarty wrote:

Ivan,

On 01/10/2019 21:26, Ivan Gerasimov wrote:

Hello!

The constructors of SocketPermission and FilePermission expect a 
String argument with comma-separated list of actions.


If the list is malformed, then the constructors throw 
IllegalArgumentException.


It turns out that the current implementation fails to throw IAE if 
the list starts with a leading comma.


Would you please help review a simple fix, which will make the 
behavior more consistent?


BUGURL: https://bugs.openjdk.java.net/browse/JDK-8230407
WEBREV: http://cr.openjdk.java.net/~igerasim/8230407/00/webrev/


The implementation changes look ok.

The SocketPermission constructor should be updated to specify IAE 
too, right?


-Chris.

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

[Joe Darcy]

Hi Sean,

On 9/26/2019 12:46 PM, Sean Mullan wrote:

On 9/23/19 4:16 PM, Joe Darcy wrote:

Hi Sean,

On 9/23/2019 12:19 PM, Sean Mullan wrote:

Hi Joe,

It's a little odd to suppress the warnings in the X509CertImpl class 
since it is a subclass of java.security.cert.Certificate which 
implements the writeReplace method so these fields are not serialized.


Also for other classes like X509Key which are internal it is a 
little odd to suppress the warnings for fields like bitStringKey 
that are not Serializable and are never serialized. It is probably 
better to mark them as transient, but I'm not really sure it is 
worth making those changes for otherwise stable code. I guess when I 
look at some of the warnings, I might think there is an issue when 
there really isn't.


I suppose these are not things you can easily detect at compile 
time, but I am wondering what you think.



Thanks for the feedback. One motivation to send out these review is 
to help tune-up the warning analysis.


A brief philosophical note, when designing any sort of warning scheme 
the cost of false positives vs false negatives needs to be weighed as 
any static check can likely only approximate the full range of 
dynamic behavior. The checks as written give up, i.e. do not attempt 
to analyze and warn, if the class uses serialPersistentFields. It 
would be possible for the checks to similarly decline to analyze if 
any of the various write* or read* serialization methods are defined. 
My current impression is that the net benefit for the checks when 
write* methods are defined is still worthwhile, even though there are 
some false positives.


As you observe, given the handling of the serial methods, 
bitStringKey in X509Key is effectively transient. I think it would be 
better serial coding for the field to be explicitly marked as 
transient to make it more obviously consistent with the 
implementation. Moreover, all the instance fields in X509Key except 
encodedKey look to be effectively transient. Since the class already 
has a serialVersionUID, marking the fields as transient is a 
serial-compatible change.


Ok. Well I understand that the benefit here is to try to detect 
serialization issues, and that is great. But at least in some cases 
like in security libs where classes are doing their own alternate 
serialization and have more than a few fields, the workaround of 
leaving the warnings in or having to mark all the fields as transient 
- either approach is not ideal to me. Could the 
SuppressWarnings("serial") annotation be applied at the class level so 
that it basically said "I know what I'm doing here, don't warn me 
about serialization issues"?


Yes; applying SuppressWarnings("serial") to the class will silence this 
new warning for all the fields. I didn't take that approach in the 
version I sent to review since, by default, I wanted to provide the 
narrowest scope of the suppression.


Would you prefer I revise the patch where there are multiple 
SuppressWarnings("serial") on fields to put a single one on the class 
instead?


Cheers,

-Joe

JDK 14 RFR of JDK-8231368: Suppress warnings on non-serializable non-transient instance fields in java.security.jgss

[Joe Darcy]

Hello,

Another module, another review request as part of making serial warnings 
more robust:


    JDK-8231368: Suppress warnings on non-serializable non-transient 
instance fields in java.security.jgss

    http://cr.openjdk.java.net/~darcy/8231368.0/

(Related earlier review 
https://mail.openjdk.java.net/pipermail/security-dev/2019-September/020672.html.)


In this latest review, I included a comment in KRBError.java that its 
writeObject method uses a different encoding scheme.


Thanks,

-Joe

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

[Joe Darcy]

Hi Sean,

On 9/23/2019 12:19 PM, Sean Mullan wrote:

Hi Joe,

It's a little odd to suppress the warnings in the X509CertImpl class 
since it is a subclass of java.security.cert.Certificate which 
implements the writeReplace method so these fields are not serialized.


Also for other classes like X509Key which are internal it is a little 
odd to suppress the warnings for fields like bitStringKey that are not 
Serializable and are never serialized. It is probably better to mark 
them as transient, but I'm not really sure it is worth making those 
changes for otherwise stable code. I guess when I look at some of the 
warnings, I might think there is an issue when there really isn't.


I suppose these are not things you can easily detect at compile time, 
but I am wondering what you think.



Thanks for the feedback. One motivation to send out these review is to 
help tune-up the warning analysis.


A brief philosophical note, when designing any sort of warning scheme 
the cost of false positives vs false negatives needs to be weighed as 
any static check can likely only approximate the full range of dynamic 
behavior. The checks as written give up, i.e. do not attempt to analyze 
and warn, if the class uses serialPersistentFields. It would be possible 
for the checks to similarly decline to analyze if any of the various 
write* or read* serialization methods are defined. My current impression 
is that the net benefit for the checks when write* methods are defined 
is still worthwhile, even though there are some false positives.


As you observe, given the handling of the serial methods, bitStringKey 
in X509Key is effectively transient. I think it would be better serial 
coding for the field to be explicitly marked as transient to make it 
more obviously consistent with the implementation. Moreover, all the 
instance fields in X509Key except encodedKey look to be effectively 
transient. Since the class already has a serialVersionUID, marking the 
fields as transient is a serial-compatible change.


Cheers,

-Joe

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

[Joe Darcy]

On 9/21/2019 4:15 AM, Chris Hegarty wrote:



On 19 Sep 2019, at 18:32, Joe Darcy  wrote:

Hello,

Ahead of augmenting javac's serial lint checks under JDK-8160675, it would be 
helpful to mark fields in security libs classes where the class is 
serializable, but a non-transient instance field does *not* have a serialiable 
type. Such classes may have difficulties being serialized at runtime:

 JDK-8231262 : Suppress warnings on non-serializable instance fields in 
security libs serializable classes
 http://cr.openjdk.java.net/~darcy/8231262.0/

The changes look good to me.

The fields in PrivateCredentialPermission and SecureRandom, could be made final 
and assigned null, ensuring non-Serializable types will never leak into them. 
But equally, this could be left to a follow on change for someone working in 
the security area.



I'd prefer to leave such code changes to people working more directly in 
the area.


Thanks for the review,

-Joe

JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

[Joe Darcy]

Hello,

Ahead of augmenting javac's serial lint checks under JDK-8160675, it 
would be helpful to mark fields in security libs classes where the class 
is serializable, but a non-transient instance field does *not* have a 
serialiable type. Such classes may have difficulties being serialized at 
runtime:


    JDK-8231262 : Suppress warnings on non-serializable instance fields 
in security libs serializable classes

    http://cr.openjdk.java.net/~darcy/8231262.0/

The review thread of the of the analogous core libs change, JDK-8231202: 
"Suppress warnings on non-serializable non-transient instance fields in 
serializable classes", is out on core-libs:


http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-September/062456.html

Thanks,

-Joe

Re: JDK 14 RFR of JDK-8229999 : Apply java.io.Serial annotations to security types in java.base

[Joe Darcy]

Hi Sean,

On 8/29/2019 6:18 AM, Sean Mullan wrote:

Looks fine to me although I didn't check if you missed anything.

What kind of documentation changes can we expect by using this 
annotation?


The documentation changes are just when reading the sources, the 
annotation type itself is not @Documented so it doesn't appear in the 
generated javadoc.


Thanks,

-Joe




--Sean

On 8/27/19 8:16 PM, Joe Darcy wrote:

Hello,

Recent work for JDK-8202385: "Annotation to mark serial-related 
fields and methods" added the java.io.Serial annotation type to the 
platform. The intention of this new annotation type is to allow 
serialization-related fields and methods to be marked as 
documentation and to allow stricter compile-time checking, analogous 
to the checking done for @Override. Implementing those stricter 
serialization-related checks will be done under JDK-8202056.


Please review the application of java.io.Serial to the 
security-related types in the base module:


 JDK-822 : Apply java.io.Serial annotations to security types 
in java.base

 http://cr.openjdk.java.net/~darcy/822.0/

As a reminder, the 5 serialization-related methods and 2 fields are:

 * private void writeObject(java.io.ObjectOutputStream stream) 
throws IOException
 * private void readObject(java.io.ObjectInputStream stream) 
throws IOException, ClassNotFoundException

 * private void readObjectNoData() throws ObjectStreamException
 * ANY-ACCESS-MODIFIER Object writeReplace() throws 
ObjectStreamException
 * ANY-ACCESS-MODIFIER Object readResolve() throws 
ObjectStreamException

 * private static final ObjectStreamField[] serialPersistentFields
 * private static final long serialVersionUID

Thanks,

-Joe

JDK 14 RFR of JDK-8229999 : Apply java.io.Serial annotations to security types in java.base

[Joe Darcy]

Hello,

Recent work for JDK-8202385: "Annotation to mark serial-related fields 
and methods" added the java.io.Serial annotation type to the platform. 
The intention of this new annotation type is to allow 
serialization-related fields and methods to be marked as documentation 
and to allow stricter compile-time checking, analogous to the checking 
done for @Override. Implementing those stricter serialization-related 
checks will be done under JDK-8202056.


Please review the application of java.io.Serial to the security-related 
types in the base module:


    JDK-822 : Apply java.io.Serial annotations to security types in 
java.base

    http://cr.openjdk.java.net/~darcy/822.0/

As a reminder, the 5 serialization-related methods and 2 fields are:

    * private void writeObject(java.io.ObjectOutputStream stream) 
throws IOException
    * private void readObject(java.io.ObjectInputStream stream) throws 
IOException, ClassNotFoundException

    * private void readObjectNoData() throws ObjectStreamException
    * ANY-ACCESS-MODIFIER Object writeReplace() throws 
ObjectStreamException

    * ANY-ACCESS-MODIFIER Object readResolve() throws ObjectStreamException
    * private static final ObjectStreamField[] serialPersistentFields
    * private static final long serialVersionUID

Thanks,

-Joe

Re: RFR - CSR: 8213082: (zipfs) Add support for POSIX file permissions

[Joe Darcy]

Hello,

On 12/21/2018 8:43 AM, Volker Simonis wrote:

Hi Alan,

thanks for looking at this issue. I've dived into the ZipFS
implementation during the last weeks and together with Christoph we've
extended and improved both the implementation the test coverage. As
Christoph already emphasized, this change is only for improving
jdk.nio.zipfs without any side effects on java.util.zip and
java.util.jar.

Please find my comments for the CSR below (for some reason I couldn't
add them to the CSR directly).

To leave a comment on a CSR, using the "Comment" button at the top of 
the page or hit "Edit", rather than the "Comment" button at the bottom 
of the page (known bug).


HTH,

-Joe

JDK 12 RFR of JDK-8213911: Use example.com in java.net and other examples

[joe darcy]

Hello,

Please review a simple doc-only change to address:

    JDK-8213911: Use example.com in java.net and other examples
    http://cr.openjdk.java.net/~darcy/8213911.0/

Patch below.

Thanks,

-Joe


--- old/src/java.base/share/classes/java/net/HostPortrange.java 
2018-11-26 13:32:47.07800 -0800
+++ new/src/java.base/share/classes/java/net/HostPortrange.java 
2018-11-26 13:32:46.90200 -0800

@@ -60,7 +60,7 @@
 HostPortrange(String scheme, String str) {
 // Parse the host name.  A name has up to three components, the
 // hostname, a port number, or two numbers representing a port
-    // range.   "www.sun.com:8080-9090" is a valid host name.
+    // range.   "www.example.com:8080-9090" is a valid host name.

 // With IPv6 an address can be 2010:836B:4179::836B:4179
 // An IPv6 address needs to be enclose in []
--- old/src/java.base/share/classes/java/net/InetAddress.java 2018-11-26 
13:32:47.47400 -0800
+++ new/src/java.base/share/classes/java/net/InetAddress.java 2018-11-26 
13:32:47.29800 -0800

@@ -1168,7 +1168,7 @@
  * No name service is checked for the validity of the address.
  *
  *  The host name can either be a machine name, such as
- * "{@code java.sun.com}", or a textual representation of its IP
+ * "{@code www.example.com}", or a textual representation of its IP
  * address.
  *  No validity checking is done on the host name either.
  *
@@ -1213,7 +1213,7 @@
  * Determines the IP address of a host, given the host's name.
  *
  *  The host name can either be a machine name, such as
- * "{@code java.sun.com}", or a textual representation of its
+ * "{@code www.example.com}", or a textual representation of its
  * IP address. If a literal IP address is supplied, only the
  * validity of the address format is checked.
  *
@@ -1259,7 +1259,7 @@
  * based on the configured name service on the system.
  *
  *  The host name can either be a machine name, such as
- * "{@code java.sun.com}", or a textual representation of its IP
+ * "{@code www.example.com}", or a textual representation of its IP
  * address. If a literal IP address is supplied, only the
  * validity of the address format is checked.
  *
--- old/src/java.base/share/classes/java/net/SocketPermission.java 
2018-11-26 13:32:47.85000 -0800
+++ new/src/java.base/share/classes/java/net/SocketPermission.java 
2018-11-26 13:32:47.67800 -0800

@@ -63,7 +63,7 @@
  * or as "localhost" (for the local machine).
  * The wildcard "*" may be included once in a DNS name host
  * specification. If it is included, it must be in the leftmost
- * position, as in "*.sun.com".
+ * position, as in "*.example.com".
  * 
  * The format of the IPv6reference should follow that specified in http://www.ietf.org/rfc/rfc2732.txt;>RFC2732: Format
@@ -115,11 +115,11 @@
  * note that if the following permission:
  *
  * 
- *   p1 = new SocketPermission("puffin.eng.sun.com:", 
"connect,accept");

+ *   p1 = new SocketPermission("foo.example.com:", "connect,accept");
  * 
  *
  * is granted to some code, it allows that code to connect to port  on
- * {@code puffin.eng.sun.com}, and to accept connections on that port.
+ * {@code foo.example.com}, and to accept connections on that port.
  *
  * Similarly, if the following permission:
  *
@@ -211,7 +211,7 @@
 // all the IP addresses of the host
 private transient InetAddress[] addresses;

-    // true if the hostname is a wildcard (e.g. "*.sun.com")
+    // true if the hostname is a wildcard (e.g. "*.example.com")
 private transient boolean wildcard;

 // true if we were initialized with a single numeric IP address
@@ -274,9 +274,9 @@
  * 
  * Examples of SocketPermission instantiation are the following:
  * 
- *    nr = new SocketPermission("www.catalog.com", "connect");
- *    nr = new SocketPermission("www.sun.com:80", "connect");
- *    nr = new SocketPermission("*.sun.com", "connect");
+ *    nr = new SocketPermission("www.example.com", "connect");
+ *    nr = new SocketPermission("www.example.com:80", "connect");
+ *    nr = new SocketPermission("*.example.com", "connect");
  *    nr = new SocketPermission("*.edu", "resolve");
  *    nr = new SocketPermission("204.160.241.0", "connect");
  *    nr = new SocketPermission("localhost:1024-65535", "listen");
@@ -400,7 +400,7 @@

 // Parse the host name.  A name has up to three components, the
 // hostname, a port number, or two numbers representing a port
-    // range.   "www.sun.com:8080-9090" is a valid host name.
+    // range.   "www.example.com:8080-9090" is a valid host name.

 // With IPv6 an address can be 2010:836B:4179::836B:4179
 // An IPv6 address needs to be enclose in []
@@ -835,10 +835,10 @@
  * 
  *  If this object was initialized with a single IP address 
and one of p's

Re: RFR (XS) 8200381 : Typos in javadoc - missing verb "be" and alike

[joe darcy]

Changes look fine; thanks,

-Joe


On 10/1/2018 4:43 PM, Lance Andersen wrote:

The changes look reasonable Ivan


On Oct 1, 2018, at 7:37 PM, Ivan Gerasimov  wrote:

Hello!

A handful of a few similar typos across core-libs/security-libs areas.

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8200381
WEBREV: http://cr.openjdk.java.net/~igerasim/8200381/00/webrev/

Would you please help review the fix?

--
With kind regards,
Ivan Gerasimov


  
   

  Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: JDK 12 RFR of JDK-8209024: Use SuppressWarnings -- now RFR for 8209304: Deprecate serialVersionUID fields in interfaces

[joe darcy]

Hi Alan,

On 8/6/2018 11:12 PM, Alan Bateman wrote:



On 06/08/2018 20:11, joe darcy wrote:

Hello,

Various interfaces in the JDK extend Serializable and declare 
serialVersionUID fields. Such fields are ineffectual and 
@SuppressWarnings("serial") should be applied to such fields to 
suppress future planned serial lint checks (JDK-8202056).


Most of the affected files are in the security-libs area with a 
handful in RMI and JNDI:


    http://cr.openjdk.java.net/~darcy/8209024.0/

Patch below. I'll fix-up the copyright years before any push.

Is there a follow-on patch coming to deprecate these fields?




Filed 8209304: Deprecate serialVersionUID fields in interfaces; webrev 
for review:


    http://cr.openjdk.java.net/~darcy/8209304.0/

Security libs team, for

src/java.base/share/classes/sun/security/internal/interfaces/TlsMasterSecret.java

I added a separate @Deprecated annotation for the serialVersionUID field 
even though the interface itself is already deprecated. That interface's 
javadoc states:


  41  * @deprecated Sun JDK internal use only --- WILL BE REMOVED in a 
future

  42  * release.

If you like, as part of this changeset I can upgrade (downgrade?) the 
@Deprecated annotation for this class to forRemoval=true.


Once that detail is sorted out, I'll create the corresponding CSR. 
(Since the java.lang.Deprecated annotation type is @Documented, adding, 
removing, or changing @Deprecated annotations is an interface change.)


Thanks,

-Joe

Re: JDK 12 RFR of JDK-8209024: Use SuppressWarnings on serialVersionUID fields in interfaces

[joe darcy]

Hi Sergey,

On 8/6/2018 3:39 PM, Sergey Bylokhov wrote:

Hi, Joe.

On 06/08/2018 14:30, joe darcy wrote:
Even if currently less commonly used, I think "ineffectual" better 
captures the intention of what I want to convey in the comment than 
"ineffective."


Can you please clarify this: what does it mean "ineffectual" in this 
context? why we need to "suppress" them and why these fields cannot be 
dropped?




A serialVersionUID field is intended to be used to provide the serial 
hash of a class both to avoid the cost of its computation at runtime and 
to provide cross-version serial stability in the face of innocuous 
changes to the class. The serialVersionUID of an interface type is *not* 
used in the serialization machinery. The names of interfaces are used 
however. Therefore, a serialVersionUID field on an interface does not 
have the effect one would assume it has, namely altering the behavior of 
serialization. In that sense, these field declarations in interfaces are 
ineffectual.


I'm working on expanding the checks done by "javac -Xlint:serial", 
including flagging these and other ineffectual serialization coding 
patterns. I've pushed a number of related cleanup fixes earlier in JDK 
12 (JDK-8208060, JDK-8207751, JDK-8208782, etc.). Much of the JDK code 
base is compiled using "-Xlint:all -Werror", meaning that if the set of 
checks is expanded and new warnings are generated, the build will fail. 
While it would be possible to exclude the expanded serial check from the 
build, I'd prefer to address the serial issues instead.


Since the files modified by  JDK-8209024 are pre-JDK 9 interfaces, their 
static serialVersionUID fields are *public* fields, meaning they are 
part of the interfaces' contract. Therefore, the fieldscannot just be 
removed per our usual compatibility policy, unlike, say a private field 
in a class. For that reason, I think it is reasonable to suppress the 
(future) serial warnings for these fields, rather than to remove the 
fields. I wouldn't oppose the effort if someone else wanted to 
deprecated these fields for removal and then remove the fields in a 
future release, but I don't think the cost/benefit of that particular 
route is justified.


HTH,

-Joe

Re: JDK 12 RFR of JDK-8209024: Use SuppressWarnings on serialVersionUID fields in interfaces

[joe darcy]

Hi Roger,

Even if currently less commonly used, I think "ineffectual" better 
captures the intention of what I want to convey in the comment than 
"ineffective."


Thanks all for the reviews; cheers,

-Joe


On 8/6/2018 12:44 PM, Roger Riggs wrote:

Hi Joe,

Looks fine.  I would probably have used "ineffective" instead of 
"ineffectual".


(Googling "define ineffective" and "define ineffectual" shows an 
interesting graph of
the use of the term with ineffective growing and ineffectual dropping 
in use.

Look under the more tag)

Regards, Roger


On 8/6/2018 3:11 PM, joe darcy wrote:

Hello,

Various interfaces in the JDK extend Serializable and declare 
serialVersionUID fields. Such fields are ineffectual and 
@SuppressWarnings("serial") should be applied to such fields to 
suppress future planned serial lint checks (JDK-8202056).


Most of the affected files are in the security-libs area with a 
handful in RMI and JNDI:


    http://cr.openjdk.java.net/~darcy/8209024.0/

Patch below. I'll fix-up the copyright years before any push.

Thanks,

-Joe

--- old/src/java.base/share/classes/java/security/Key.java 2018-08-06 
12:04:46.666000999 -0700
+++ new/src/java.base/share/classes/java/security/Key.java 2018-08-06 
12:04:46.51999 -0700

@@ -109,6 +109,7 @@
 * serialization compatibility with a previous
 * version of the class.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface 
is ineffectual

 static final long serialVersionUID = 6603384152749567654L;

 /**
--- old/src/java.base/share/classes/java/security/PrivateKey.java 
2018-08-06 12:04:47.062000999 -0700
+++ new/src/java.base/share/classes/java/security/PrivateKey.java 
2018-08-06 12:04:46.914000999 -0700

@@ -64,5 +64,6 @@
  * The class fingerprint that is set to indicate serialization
  * compatibility with a previous version of the class.
  */
+    @SuppressWarnings("serial") // serialVersionUID in an interface 
is ineffectual

 static final long serialVersionUID = 6034044314589513430L;
 }
--- old/src/java.base/share/classes/java/security/PublicKey.java 
2018-08-06 12:04:47.454000999 -0700
+++ new/src/java.base/share/classes/java/security/PublicKey.java 
2018-08-06 12:04:47.306000999 -0700

@@ -50,5 +50,6 @@
  * The class fingerprint that is set to indicate serialization
  * compatibility with a previous version of the class.
  */
+    @SuppressWarnings("serial") // serialVersionUID in an interface 
is ineffectual

 static final long serialVersionUID = 7187392471159151072L;
 }
--- 
old/src/java.base/share/classes/java/security/interfaces/DSAPrivateKey.java 
2018-08-06 12:04:47.858000999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/DSAPrivateKey.java 
2018-08-06 12:04:47.706000999 -0700

@@ -48,6 +48,7 @@
 * serialization compatibility with a previous
 * version of the class.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface 
is ineffectual

 static final long serialVersionUID = 7776497482533790279L;

 /**
--- 
old/src/java.base/share/classes/java/security/interfaces/DSAPublicKey.java 
2018-08-06 12:04:48.27999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/DSAPublicKey.java 
2018-08-06 12:04:48.114000999 -0700

@@ -48,6 +48,7 @@
 * serialization compatibility with a previous
 * version of the class.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface 
is ineffectual

 static final long serialVersionUID = 1234526332779022332L;

 /**
--- 
old/src/java.base/share/classes/java/security/interfaces/ECPrivateKey.java 
2018-08-06 12:04:48.674000999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/ECPrivateKey.java 
2018-08-06 12:04:48.522000999 -0700

@@ -43,6 +43,7 @@
 * The class fingerprint that is set to indicate
 * serialization compatibility.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface 
is ineffectual

 static final long serialVersionUID = -7896394956925609184L;

 /**
--- 
old/src/java.base/share/classes/java/security/interfaces/ECPublicKey.java 
2018-08-06 12:04:49.078000999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/ECPublicKey.java 
2018-08-06 12:04:48.926000999 -0700

@@ -45,6 +45,7 @@
 * The class fingerprint that is set to indicate
 * serialization compatibility.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface 
is ineffectual

 static final long serialVersionUID = -3314988629879632826L;

 /**
--- 
old/src/java.base/share/classes/java/security/interfaces/RSAMultiPrimePrivateCrtKey.java 
2018-08-06 12:04:49.47999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/RSAMultiPrimePrivateCrtKey.java 
2018-08-06 12:04:49.318000999 -0700

JDK 12 RFR of JDK-8209024: Use SuppressWarnings on serialVersionUID fields in interfaces

[joe darcy]

Hello,

Various interfaces in the JDK extend Serializable and declare 
serialVersionUID fields. Such fields are ineffectual and 
@SuppressWarnings("serial") should be applied to such fields to suppress 
future planned serial lint checks (JDK-8202056).


Most of the affected files are in the security-libs area with a handful 
in RMI and JNDI:


    http://cr.openjdk.java.net/~darcy/8209024.0/

Patch below. I'll fix-up the copyright years before any push.

Thanks,

-Joe

--- old/src/java.base/share/classes/java/security/Key.java 2018-08-06 
12:04:46.666000999 -0700
+++ new/src/java.base/share/classes/java/security/Key.java 2018-08-06 
12:04:46.51999 -0700

@@ -109,6 +109,7 @@
 * serialization compatibility with a previous
 * version of the class.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface is 
ineffectual

 static final long serialVersionUID = 6603384152749567654L;

 /**
--- old/src/java.base/share/classes/java/security/PrivateKey.java 
2018-08-06 12:04:47.062000999 -0700
+++ new/src/java.base/share/classes/java/security/PrivateKey.java 
2018-08-06 12:04:46.914000999 -0700

@@ -64,5 +64,6 @@
  * The class fingerprint that is set to indicate serialization
  * compatibility with a previous version of the class.
  */
+    @SuppressWarnings("serial") // serialVersionUID in an interface is 
ineffectual

 static final long serialVersionUID = 6034044314589513430L;
 }
--- old/src/java.base/share/classes/java/security/PublicKey.java 
2018-08-06 12:04:47.454000999 -0700
+++ new/src/java.base/share/classes/java/security/PublicKey.java 
2018-08-06 12:04:47.306000999 -0700

@@ -50,5 +50,6 @@
  * The class fingerprint that is set to indicate serialization
  * compatibility with a previous version of the class.
  */
+    @SuppressWarnings("serial") // serialVersionUID in an interface is 
ineffectual

 static final long serialVersionUID = 7187392471159151072L;
 }
--- 
old/src/java.base/share/classes/java/security/interfaces/DSAPrivateKey.java 
2018-08-06 12:04:47.858000999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/DSAPrivateKey.java 
2018-08-06 12:04:47.706000999 -0700

@@ -48,6 +48,7 @@
 * serialization compatibility with a previous
 * version of the class.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface is 
ineffectual

 static final long serialVersionUID = 7776497482533790279L;

 /**
--- 
old/src/java.base/share/classes/java/security/interfaces/DSAPublicKey.java 
2018-08-06 12:04:48.27999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/DSAPublicKey.java 
2018-08-06 12:04:48.114000999 -0700

@@ -48,6 +48,7 @@
 * serialization compatibility with a previous
 * version of the class.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface is 
ineffectual

 static final long serialVersionUID = 1234526332779022332L;

 /**
--- 
old/src/java.base/share/classes/java/security/interfaces/ECPrivateKey.java 
2018-08-06 12:04:48.674000999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/ECPrivateKey.java 
2018-08-06 12:04:48.522000999 -0700

@@ -43,6 +43,7 @@
 * The class fingerprint that is set to indicate
 * serialization compatibility.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface is 
ineffectual

 static final long serialVersionUID = -7896394956925609184L;

 /**
--- 
old/src/java.base/share/classes/java/security/interfaces/ECPublicKey.java 
2018-08-06 12:04:49.078000999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/ECPublicKey.java 
2018-08-06 12:04:48.926000999 -0700

@@ -45,6 +45,7 @@
 * The class fingerprint that is set to indicate
 * serialization compatibility.
 */
+    @SuppressWarnings("serial") // serialVersionUID in an interface is 
ineffectual

 static final long serialVersionUID = -3314988629879632826L;

 /**
--- 
old/src/java.base/share/classes/java/security/interfaces/RSAMultiPrimePrivateCrtKey.java 
2018-08-06 12:04:49.47999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/RSAMultiPrimePrivateCrtKey.java 
2018-08-06 12:04:49.318000999 -0700

@@ -51,6 +51,7 @@
  * serialization compatibility with a previous
  * version of the type.
  */
+    @SuppressWarnings("serial") // serialVersionUID in an interface is 
ineffectual

 static final long serialVersionUID = 618058533534628008L;

 /**
--- 
old/src/java.base/share/classes/java/security/interfaces/RSAPrivateCrtKey.java 
2018-08-06 12:04:49.89999 -0700
+++ 
new/src/java.base/share/classes/java/security/interfaces/RSAPrivateCrtKey.java 
2018-08-06 12:04:49.726000999 -0700

@@ -46,6 +46,7 @@
  * serialization compatibility with a previous
  * version of the type.
  */
+    @SuppressWarnings("serial") // serialVersionUID in an interface is 
ineffectual

 static final long serialVersionUID = 

Re: JDK 11 RFR of JDK-8196414: Update ProviderVersionCheck.java to pass on updated JDK versions

[joe darcy]

Hi Sean,

On 1/30/2018 10:03 AM, Sean Mullan wrote:
Does Runtime.version().feature() return the same value as the 
"java.specification.version" property? (see 
sun.security.util.SecurityConstants.PROVIDER_VER).


That is the value that the JDK security providers use as their 
version. If not, this test may fail when we bump up the version to 11 
and we probably would want to also set SecurityConstants.PROVIDER_VER 
to the value of Runtime.version().feature() instead (you could include 
that as part of this fix).




The following patch based on java.specification.version

@@ -42,7 +42,8 @@

 for (Provider p: Security.getProviders()) {
 System.out.print(p.getName() + " ");
-    if (p.getVersion() != 10.0d) {
+    String specVersion = 
System.getProperty("java.specification.version");

+    if (p.getVersion() != Double.parseDouble(specVersion)) {
 System.out.println("failed. " + "Version received was " +
 p.getVersion());
 failure = true;

passes both on JDK 10 builds and an internal JDK 11 build with the 
version updated.


Thanks,

-Joe

Re: RFR 9 test-only RFR 8177328 : java/lang/ClassLoader/securityManager/ClassLoaderTest.java times out with -Xcomp

[joe darcy]

I understand the interest in having test pass reliably, but I don't 
think giving the test very large timeouts is the preferred way of 
accomplishing that.


For all configurations, the test can now run for up to 20 minutes, up 
from 4 minutes. We want to run the entire test suite, thousands of 
tests, in about 20 minutes. The the timeout factor used for Xcomp run, 
the test would probably now be able to run for over an hour before 
timing out.


I suggest making the test run faster, or seeing if there has been a 
regressions in Xcomp to make test perform more poorly there.


Thanks,

-Joe


On 4/27/2017 12:08 PM, Brent Christian wrote:

Hi,

This test times out under our automated testing configurations that 
include -Xcomp.


Please review my change to increase the timeout for this test.  It is 
sufficient for the test configurations in question to pass on two 
different local machines (Mac & Linux).


diff -r 7c04ab31b4d6 
test/java/lang/ClassLoader/securityManager/ClassLoaderTest.java
--- a/test/java/lang/ClassLoader/securityManager/ClassLoaderTest.java 
Wed Apr 26 09:37:23 2017 -0700
+++ b/test/java/lang/ClassLoader/securityManager/ClassLoaderTest.java 
Thu Apr 27 12:03:33 2017 -0700

@@ -29,7 +29,7 @@
  * @library /lib/testlibrary
  * @modules java.base/jdk.internal.module
  * @build JarUtils CompilerUtils
- * @run main/timeout=240 ClassLoaderTest
+ * @run main/timeout=1200 ClassLoaderTest
  */

Thanks,
-Brent

Re: RFR: 8179370: Replace use of , and tags in java.base

[joe darcy]

Looks better; thanks Jon,

-Joe


On 4/26/2017 6:49 PM, Jonathan Gibbons wrote:
Updated webrev to address Joe's suggestion to try harder to use 
{@code} as a substitute for .


http://cr.openjdk.java.net/~jjg/8179370/webrev.01

The modified sed script has a new first line:

s|\([^&<>]*\)|{@code \1}|g
s|||g
s|||g
s|<\\/tt>|<\\/code>|g
s|\(]*\)>\([^<]*\)|\1 
style="text-align:center">\2|
s|style="margin:0 auto" |

s|||
s|||

Also note that there is a small visible bug in webrev. If you look at 
ServiceLoader.java:133,
the second instance of  is not converted to {@code} because the 
embedded '\' is actually
an entity in the source file ... and so it is inappropriate to use 
{@code} for that instance.


-- Jon



On 04/26/2017 06:09 PM, Jonathan Gibbons wrote:

Joe,

Yes, there are occurrences here that require  instead of 
{@code}, because of the presence of HTML entities. It's about 50/50.


I'd prefer to stay with mechanical updates as much as possible for 
these bulk updates, as compared to manual updates, but I may be able 
to improve the sed script for this case.


-- Jon

On 04/26/2017 05:55 PM, Joseph D. Darcy wrote:

Hi Jon,

I'd prefer if the "foo" were replaced with "{@code tt}" 
rather than "foo"- none of the tricky cases which 
preclude use of {@code } use seem to be present here - but will 
approve the changeset in its current form too.


Cheers,

-Joe

On 4/26/2017 5:50 PM, Jonathan Gibbons wrote:
Please review these mostly simple changes to replace HTML tags 
which are not valid in HTML 5 in public doc comments in java.base.


As with the previous changes, the changes were done mechanically, 
using the following sed script:


s|||g
s|||g
s|<\\/tt>|<\\/code>|g
s|\(]*\)>\([^<]*\)|\1 
style="text-align:center">\2|
s|style="margin:0 auto" |

s|||
s|||

The unusual form of the 3rd line was to cover the occurrence in a 
makefile.


The 4th line is specific for DataInput.java, and replaces  
within a  with a style on the  element itself.


The 5th and 6th lines are specific to URLConnection. The use of the 
table itself and the ASCII art that follows it are questionable, 
but the intent here is just to update the HTML and not to rework 
the visual appearance of the generated documentation.


The changes cover files in the following packages:

java.base java/io
java.base java/net
java.base java/util
java.base javax/net/ssl


JBS: https://bugs.openjdk.java.net/browse/JDK-8179370
Webrev: http://cr.openjdk.java.net/~jjg/8179370/webrev/

-- Jon

Re: RFR of 8177683: Suppress lint removal warnings in jdk.security and jdk.policytool

[joe darcy]

Hi Max,

Since entire classes were being covered by the annotation, I wanted to 
make clear that the removal warnings in those cases were for PolicyTool 
as opposed to being for something else.


Thanks,

-Joe


On 3/28/2017 4:30 PM, Weijun Wang wrote:

Hi Joe

This looks fine to me.

One question:

-@SuppressWarnings("deprecation")
+@SuppressWarnings({"deprecation",
+   "removal"}) // PolicyTool

Why did you split the line to 2?

Thanks
Max

On 03/28/2017 09:09 AM, joe darcy wrote:

Hello,

The jdk.security and jdk.policytool modules use various APIs that are
deprecated for removal. Until the types in question are actually
removed, the lint removal warnings should be suppressed in support of
having a warnings-free build. Please review the webrev:

http://cr.openjdk.java.net/~darcy/8177683.0/

(This issue is a subtask of JDK-8177553: Address removal lint warnings
in the JDK build. I'd prefer to get JDK-8177553 addressed in JDK 9;
however, if that is not approved as part of rampdown 2, I'll push the
associated changes to JDK 10.)

Thanks,

-Joe

RFR of 8177683: Suppress lint removal warnings in jdk.security and jdk.policytool

[joe darcy]

Hello,

The jdk.security and jdk.policytool modules use various APIs that are 
deprecated for removal. Until the types in question are actually 
removed, the lint removal warnings should be suppressed in support of 
having a warnings-free build. Please review the webrev:


http://cr.openjdk.java.net/~darcy/8177683.0/

(This issue is a subtask of JDK-8177553: Address removal lint warnings 
in the JDK build. I'd prefer to get JDK-8177553 addressed in JDK 9; 
however, if that is not approved as part of rampdown 2, I'll push the 
associated changes to JDK 10.)


Thanks,

-Joe

Re: 9 RFR of JDK-8176337: Mark several tests as intermittently failing

[joe darcy]

Looks fine Hamlin; thanks,

-Joe


On 3/7/2017 7:36 PM, Hamlin Li wrote:

Would you please review below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8176337

webrev: http://cr.openjdk.java.net/~mli/8176337/webrev.00/


These tests are failing intermittently, they should be marked 
accordingly with

@key intermittent

java/nio/channels/FileChannel/Transfer.java (JDK-8140263 
)
java/nio/channels/FileChannel/Transfers.java (JDK-8140263 
)
java/io/FileInputStream/LargeFileAvailable.java (JDK-8174810 
)
java/nio/channels/FileChannel/LoopingTruncate.java (JDK-8173386 
)
java/lang/ProcessBuilder/Basic.java (JDK-8171426 
)
sun/security/tools/keytool/DefaultSignatureAlgorithm.java (JDK-8172657 
)
javax/net/ssl/DTLS/CipherSuite.java (JDK-8174072 
)



Thank you

-Hamlin

JDK 10 RFR of JDK-8173903: Update various tests to pass under JDK 10

[joe darcy]

Hello,

After the version update to "10" in JDK 10 ( JDK-8029942 ), various 
libraries tests failed including:


java/lang/module/MultiReleaseJarTest.java
java/security/Provider/ProviderVersionCheck.java
sun/security/tools/jarsigner/multiRelease/MVJarSigningTest.java

These tests need to be updated for the new JDK. When it is clear how to 
do so, I've updated the tests in a way so that they don't need to be 
updated again for JDK 11.


Webrev:

http://cr.openjdk.java.net/~darcy/8173903.0/

and patch below. I'll update the other copyrights before pushing.

Thanks,

-Joe


diff -r 72f33dbfcf3b test/java/lang/module/MultiReleaseJarTest.java
--- a/test/java/lang/module/MultiReleaseJarTest.javaTue Jan 31 
19:26:10 2017 -0500
+++ b/test/java/lang/module/MultiReleaseJarTest.javaFri Feb 03 
11:18:23 2017 -0800

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2017, 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
@@ -65,7 +65,7 @@

 private static final String MODULE_INFO = "module-info.class";

-private static final int RELEASE = Runtime.version().major();
+private static final String RELEASE = "" + Runtime.version().major();

 // are multi-release JARs enabled?
 private static final boolean MULTI_RELEASE;
@@ -88,8 +88,8 @@
 .moduleInfo("module-info.class", descriptor)
 .resource("p/Main.class")
 .resource("p/Helper.class")
-.resource("META-INF/versions/9/p/Helper.class")
- .resource("META-INF/versions/9/p/internal/Helper9.class")
+.resource("META-INF/versions/" + RELEASE + 
"/p/Helper.class")
+.resource("META-INF/versions/" + RELEASE + 
"/p/internal/HelperNew.class")

 .build();

 // find the module
@@ -131,9 +131,9 @@
 .moduleInfo(MODULE_INFO, descriptor1)
 .resource("p/Main.class")
 .resource("p/Helper.class")
-.moduleInfo("META-INF/versions/9/" + MODULE_INFO, 
descriptor2)

-.resource("META-INF/versions/9/p/Helper.class")
- .resource("META-INF/versions/9/p/internal/Helper9.class")
+.moduleInfo("META-INF/versions/" + RELEASE + "/" + 
MODULE_INFO, descriptor2)
+.resource("META-INF/versions/" + RELEASE + 
"/p/Helper.class")
+.resource("META-INF/versions/" + RELEASE + 
"/p/internal/HelperNew.class")

 .build();

 // find the module
@@ -161,8 +161,8 @@
 Path jar = new JarBuilder(name)
 .resource("p/Main.class")
 .resource("p/Helper.class")
-.resource("META-INF/versions/9/p/Helper.class")
- .resource("META-INF/versions/9/p/internal/Helper9.class")
+.resource("META-INF/versions/" + RELEASE + 
"/p/Helper.class")
+.resource("META-INF/versions/" + RELEASE + 
"/p/internal/HelperNew.class")

 .build();

 // find the module
@@ -200,7 +200,7 @@

 Path jar = new JarBuilder(name)
 .moduleInfo(MODULE_INFO, descriptor1)
-.moduleInfo("META-INF/versions/9/" + MODULE_INFO, 
descriptor2)
+.moduleInfo("META-INF/versions/" + RELEASE + "/" + 
MODULE_INFO, descriptor2)

 .build();

 // find the module
diff -r 72f33dbfcf3b test/java/security/Provider/ProviderVersionCheck.java
--- a/test/java/security/Provider/ProviderVersionCheck.javaTue Jan 
31 19:26:10 2017 -0500
+++ b/test/java/security/Provider/ProviderVersionCheck.javaFri Feb 
03 11:18:23 2017 -0800

@@ -42,7 +42,7 @@

 for (Provider p: Security.getProviders()) {
 System.out.print(p.getName() + " ");
-if (p.getVersion() != 9.0d) {
+if (p.getVersion() != 10.0d) {
 System.out.println("failed. " + "Version received was " +
 p.getVersion());
 failure = true;
diff -r 72f33dbfcf3b 
test/sun/security/tools/jarsigner/multiRelease/MVJarSigningTest.java
--- 
a/test/sun/security/tools/jarsigner/multiRelease/MVJarSigningTest.java 
Tue Jan 31 19:26:10 2017 -0500
+++ 
b/test/sun/security/tools/jarsigner/multiRelease/MVJarSigningTest.java 
Fri Feb 03 11:18:23 2017 -0800

@@ -74,7 +74,8 @@
 private static final String KEYPASS = "changeit";
 private static final String SIGNED_JAR = "Signed.jar";
 private static final String POLICY_FILE = "SignedJar.policy";
-private static final String VERSION_MESSAGE = "I am running on 
version 9";

+private static final String VERSION = "" + Runtime.version().major();
+private static final String VERSION_MESSAGE = "I am running on 
version " + VERSION;


 public static void main(String[] args) throws Throwable {

JDK 9 RFR of JDK-8171062: Problem list ServerIdentityTest.java on window

[joe darcy]

Hello,

Until JDK-8171061 is fixed, the test

sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java

should be problem listed on windows.

Patch below.

Thanks,

-Joe

diff -r b9cdffb87bea test/ProblemList.txt
--- a/test/ProblemList.txtWed Dec 07 10:55:13 2016 -0500
+++ b/test/ProblemList.txtSun Dec 11 19:19:54 2016 -0800
@@ -223,6 +223,8 @@

 sun/security/ssl/SSLSocketImpl/AsyncSSLSocketClose.java 8161232 macosx-all

+sun/net/www/protocol/https/HttpsClient/ServerIdentityTest.java 8171061 
windows-all

+
 

 # jdk_sound

Re: JDK 9 RFR of JDK-8039854: Broken link in java.lang.RuntimePermission

[joe darcy]

Hi Sean,

I'm going to push the fix to get a valid link in the table, but I don't 
oppose if someone else files a follow-up bug to change the spec and 
remove the row if that is appropriate course of action.


Thanks,

-Joe


On 9/8/2016 12:55 PM, Sean Mullan wrote:
Actually, I don't really think this permission target belongs in 
RuntimePermission since it is specific to Oracle's Java Plugin. I 
would be in favor of removing it and only documenting it in the 
deployment guides.


--Sean

On 09/08/2016 02:17 PM, Lance Andersen wrote:

all good Joe

On Sep 8, 2016, at 2:16 PM, joe darcy <joe.da...@oracle.com
<mailto:joe.da...@oracle.com>> wrote:

Hello,

Please review the patch below to address

   JDK-8039854: Broken link in java.lang.RuntimePermission

Two broken links are replaced by a live link to the deployment guide.

Thanks,

-Joe


--- a/src/java.base/share/classes/java/lang/RuntimePermission.java Thu
Sep 08 16:16:44 2016 +0100
+++ b/src/java.base/share/classes/java/lang/RuntimePermission.java Thu
Sep 08 10:35:10 2016 -0700
@@ -323,11 +323,9 @@
 *   usePolicy
 *   Granting this permission disables the Java Plug-In's default
 *   security prompting behavior.
- *   For more information, refer to Java Plug-In's guides, href=

- * "../../../technotes/guides/plugin/developer_guide/security.html">
- *   Applet Security Basics and - * 
"../../../technotes/guides/plugin/developer_guide/rsa_how.html#use">

- *   usePolicy Permission.
+ *   For more information, refer to the deployment guide.
+ *   
 * 
 * 
 *   manageProcess



<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif> 


<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>

JDK 9 RFR of JDK-8039854: Broken link in java.lang.RuntimePermission

[joe darcy]

Hello,

Please review the patch below to address

JDK-8039854: Broken link in java.lang.RuntimePermission

Two broken links are replaced by a live link to the deployment guide.

Thanks,

-Joe


--- a/src/java.base/share/classes/java/lang/RuntimePermission.java Thu 
Sep 08 16:16:44 2016 +0100
+++ b/src/java.base/share/classes/java/lang/RuntimePermission.java Thu 
Sep 08 10:35:10 2016 -0700

@@ -323,11 +323,9 @@
  *   usePolicy
  *   Granting this permission disables the Java Plug-In's default
  *   security prompting behavior.
- *   For more information, refer to Java Plug-In's guides, 
- *   Applet Security Basics and 
- *   usePolicy Permission.
+ *   For more information, refer to the deployment guide.
+ *   
  * 
  * 
  *   manageProcess

JDK 9 RFR mark java/security/SignedObject/Chain.java as failing intermittently

[joe darcy]

Hello,

The test

java/security/SignedObject/Chain.java

has been seen to fail intermittently (JDK-8136842). I'd like to mark the 
test accordingly; please review the test below which does this.


Thanks,

-Joe


--- a/test/java/security/SignedObject/Chain.javaThu Jul 07 10:16:47 
2016 -0700
+++ b/test/java/security/SignedObject/Chain.javaThu Jul 07 13:59:49 
2016 -0700

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, 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
@@ -33,6 +33,7 @@
 /*
  * @test
  * @bug 8050374
+ * @key intermittent
  * @summary Verify a chain of signed objects
  */
 public class Chain {

JDK 9 RFR of JDK-8159502: Mark ShortRSAKey512.java as intermittently failing

[joe darcy]

The test

javax/net/ssl/TLSv12/ShortRSAKey512.java

has been seen to intermittently fail (JDK-8159501). The test should be 
marked accordingly.


Please review the patch below which does this.

Thanks,

-Joe

diff -r d6a1ad87842f test/javax/net/ssl/TLSv12/ShortRSAKey512.java
--- a/test/javax/net/ssl/TLSv12/ShortRSAKey512.javaTue Jun 14 
09:03:12 2016 -0700
+++ b/test/javax/net/ssl/TLSv12/ShortRSAKey512.javaTue Jun 14 
14:36:39 2016 -0700

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2015, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2012, 2016, 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
@@ -33,6 +33,7 @@
  *
  * SunJSSE does not support dynamic system properties, no way to 
re-use

  * system properties in samevm/agentvm mode.
+ * @key intermittent
  * @run main/othervm ShortRSAKey512 PKIX
  * @run main/othervm ShortRSAKey512 SunX509
  */

JDK 9 RFR of JDK-8156897: Problem list ShortRSAKey1024.sh on windows

[joe darcy]

Hello,

The test

sun/security/mscapi/ShortRSAKey1024.sh

has been seen to fail with some frequency on windows. Until JDK-8153948 
is addressed, the test should be problem listed.


Please review the patch below which does this.

Thanks,

-Joe

diff -r b14b89e259ac test/ProblemList.txt
--- a/test/ProblemList.txtThu May 12 14:45:06 2016 -0700
+++ b/test/ProblemList.txtThu May 12 15:10:53 2016 -0700
@@ -212,6 +212,8 @@

 sun/security/tools/keytool/ListKeychainStore.sh 8156889 macosx-all

+sun/security/mscapi/ShortRSAKey1024.sh 8153948 windows-all
+
 java/security/Security/ClassLoaderDeadlock/Deadlock2.sh 8062758 
generic-all


 sun/security/tools/jarsigner/warnings/BadKeyUsageTest.java 8026393 
generic-all

Fwd: Re: JDK 9 pre-review of JDK-6850612: Deprecate Class.newInstance since it violates the checked exception language contract

[joe darcy]

Security libs team,

Over on core-libs, the Class.newInstance method is up for deprecation 
since it does not obey the general checked exceptions contract.


Please review the security-libs portion of

http://cr.openjdk.java.net/~darcy/6850612.0/

Generally a new variable was introduced to host the Class.newInstance 
call so that the minimal-size region would be affected by the 
@SuppressWarnings("deprecation") annotation.


Thanks,

-Joe


 Forwarded Message 
Subject: 	Re: JDK 9 pre-review of JDK-6850612: Deprecate 
Class.newInstance since it violates the checked exception language contract

Date:   Thu, 21 Apr 2016 09:25:27 -0700
From:   joe darcy <joe.da...@oracle.com>
Organization:   Oracle Corporation
To: core-libs-dev <core-libs-...@openjdk.java.net>



Hello,

After a generally positive reception, please review the webrev to
implement the deprecation of Class.newInstance and the suppression of
the resulting warnings:

http://cr.openjdk.java.net/~darcy/6850612.0/

There are also some changes in the langtools repo; I'll send a separate
review request for those changes to compiler-dev.

I'll also forward this review request to other areas with affected code.

Thanks,

-Joe

On 4/17/2016 10:31 AM, joe darcy wrote:

Hello,

With talk of deprecation in the air [1], I thought it would be a fine
time to examine one of the bugs on my list

JDK-6850612: Deprecate Class.newInstance since it violates the
checked exception language contract

As the title of the bug implies, The Class.newInstance method
knowingly violates the checking exception contract. This has long been
documented in its specification:


Note that this method propagates any exception thrown by the nullary
constructor, including a checked exception. Use of this method
effectively bypasses the compile-time exception checking that would
otherwise be performed by the compiler. The Constructor.newInstance
method avoids this problem by wrapping any exception thrown by the
constructor in a (checked) InvocationTargetException.


Roughly, the fix would be to turn the text of this note into the
@deprecated text and to add a @Deprecated(since="9") annotation to the
method. There are a few dozen uses of the method in the JDK that would
have to be @SuppressWarning-ed or otherwise updated.

Thoughts on the appropriateness of deprecating this method at this time?

Comments on the bug have suggested that besides deprecating the
method, a new method on Class could be introduced,
newInstanceWithProperExceptionBehavior, that had the same signature
but wrapped exceptions thrown by the constructor call in the same way
Constructor.newInstance does.

Thanks,

-Joe

[1]
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040192.html

Re: JDK 9 RFR of JDK-8151763; Use more informative format for problem list

[joe darcy]

On 3/30/2016 5:34 PM, Mandy Chung wrote:

On Mar 30, 2016, at 4:48 PM, Joseph D. Darcy  wrote:

Hi Mandy,

Hopefully the third time will be the charm for this changeset after your 
correction to the commented-out test:

http://cr.openjdk.java.net/~darcy/8151763.2

I aligned the bug number in column 64 unless the test name took more 
characters. (This isn't as evident in the webrev since the tab expansion is 
different than in a text editor.)


Thanks for doing it.  Looks fine with me.

Just to mention it: these few lines are somewhat strange (shorter test name has 
more whitespace) that you may want to double check.  Ok to push what you have.

! java/nio/file/WatchService/Basic.java   
7158947 solaris-all Solaris 11
! java/nio/file/WatchService/MayFlies.java
7158947 solaris-all Solaris 11
! java/nio/file/WatchService/LotsOfEvents.java
7158947 solaris-all Solaris 11



Pushed after a de-tabbification and verifying the set of tests to run 
was the same before and after the update.


Thanks,

-Joe

Re: JDK 9 RFR of JDK-8151763; Use more informative format for problem list

[joe darcy]

Hi Mandy,

On 3/28/2016 8:48 PM, Mandy Chung wrote:

On Mar 28, 2016, at 5:03 PM, Joseph D. Darcy  wrote:

Hello,

New iteration of the webrev updated after the Jigsaw integration and 
incorporating the earlier feedback.

http://cr.openjdk.java.net/~darcy/8151763.1


# tools/jimage/JImageTest.java
linux-i586,windows-i586

Is this test accidentally removed?  Other than this, looks okay.


The "#" lines are comments so I was removing a commented out line. (I 
assumed, but did not verify, the line for this test was a leftover 
artifact of the recent Jigsaw merge.)




Nit: it’d be good to have most of bug ids aligned in the same column start.
Here are a few ones:

  210 sun/security/krb5/auto/Unreachable.java 7164518 macosx-all no 
PortUnreachableException on Mac
  212 java/security/KeyPairGenerator/SolarisShortDSA.java 7041639 solaris-all 
Solaris DSA keypair generation bug
  213 sun/security/tools/keytool/standard.sh  7041639 solaris-all 
Solaris DSA keypair generation bug
  346 java/util/Arrays/ParallelPrefix.java 8080165,8085982 generic-all
  348 java/util/BitSet/BitSetStreamTest.java 8079538 generic-all
  360 sun/tools/jmap/heapconfig/JMapHeapConfigTest.java 8072131,8132452 
generic-all
  370 sun/tools/jinfo/JInfoSanityTest.java   8059035 
generic-all




I was trying to avoid introducing lots of spacing changes in an attempt 
to make the patch easier to review, but I can look over these cases again.


Thanks,

-Joe

JDK 9 RFR of JDK-8151835: Mark SmallPrimeExponentP.java as intermittently failing

[joe darcy]

Hello,

The test

sun/security/mscapi/SmallPrimeExponentP.java

has been seen to timeout intermittently, bug JDK-8151834.

The sources of the test should be marked accordingly; please review the 
corresponding patch below.


Thanks,

-Joe

--- a/test/sun/security/mscapi/SmallPrimeExponentP.javaFri Mar 11 
17:07:57 2016 -0800
+++ b/test/sun/security/mscapi/SmallPrimeExponentP.javaMon Mar 14 
10:27:59 2016 -0700

@@ -32,6 +32,7 @@
 /*
  * @test
  * @bug 8023546
+ * @key intermittent
  * @modules java.base/sun.security.x509
  *  java.base/sun.security.tools.keytool
  * @summary sun/security/mscapi/ShortRSAKey1024.sh fails intermittently

JDK 9 RFR of JDK-8151763; Use more informative format for problem list

[joe darcy]

Hello,

As Jon Gibbons has noted off-list, the problem list entries can directly 
include the bug number associated with the test in question, enabling 
better reporting. This format should be used rather than the current 
convention of putting the bug number in a comment.


Please review the webrev to adopt the revised format for the problem list:

http://cr.openjdk.java.net/~darcy/8151763.0/

I've verified jtreg produces the same test list with the old and new 
versions of the problem list.


Thanks,

-Joe

JDK 9 RFR of JDK-8151228: Mark TestDSAGenParameterSpec.java as intermittently failing

[joe darcy]

Hello,

The test

sun/security/provider/NSASuiteB/TestDSAGenParameterSpec.java

has been observed to intermittently fail (JDK-8137255). Until that 
problem is resolved, the test should be marked accordingly.


Please review the patch below which does that marking.

Thanks,

-Joe

--- a/test/sun/security/provider/NSASuiteB/TestDSAGenParameterSpec.java 
Thu Mar 03 15:47:08 2016 -0800
+++ b/test/sun/security/provider/NSASuiteB/TestDSAGenParameterSpec.java 
Thu Mar 03 16:20:25 2016 -0800

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, 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
@@ -37,6 +37,7 @@
 /*
  * @test
  * @bug 8075286
+ * @key intermittent
  * @summary Verify that DSAGenParameterSpec can and can only be used 
to generate
  *  DSA within some certain range of key sizes as described in 
the class
  *  specification (L, N) as (1024, 160), (2048, 224), (2048, 
256) and

JDK 9 RFR of JDK-8151225: Mark SpecTest.java as intermittently failing

[joe darcy]

Hello,

The test

sun/security/rsa/SpecTest.java

has been observed to fail intermittently with a timeout (JDK-8137231). 
The observed timeouts have been for the largest key size. Until that 
issue is resolved, the test should be marked as failing intermittently.


Please review the patch below which does this marking.

Thanks,

-Joe

--- a/test/sun/security/rsa/SpecTest.javaThu Mar 03 12:49:12 2016 -0800
+++ b/test/sun/security/rsa/SpecTest.javaThu Mar 03 14:51:29 2016 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, 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
@@ -34,6 +34,7 @@
 /**
  * @test
  * @bug 8044199
+ * @key intermittent
  * @summary Check same KeyPair's private key and public key have same 
modulus.

  *  also check public key's public exponent equals to given spec's public
  *  exponent.