Re: RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java [v3]

2021-06-21 Thread Sibabrata Sahoo
On Fri, 18 Jun 2021 13:24:17 GMT, Abdul Kolarkunnu  
wrote:

>> ParamsTest is an interop test between keytool <-> openssl. There are some 
>> manual steps listed in jdk/sun/security/pkcs12/params/README to perform 
>> after the execution of jtreg execution. So this test is to perform that 
>> manual steps.
>
> Abdul Kolarkunnu has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java

Marked as reviewed by ssahoo (Committer).

-

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


Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance [v4]

2021-06-21 Thread Xue-Lei Andrew Fan
On Tue, 22 Jun 2021 02:46:02 GMT, Dongbo He  wrote:

>> Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm 
>> has been disabled. It is less efficient when there are more disabled 
>> elements in the list, we can use Set instead of List to speed up the search.
>> 
>> Patch contains a benchmark that can be run with `make test 
>> TEST="micro:java.security.AlgorithmConstraintsPermits"`.
>> Baseline results before patch:
>> 
>> Benchmark(algorithm)  Mode  CntScore 
>> Error  Units
>> AlgorithmConstraintsPermits.permitsSSLv3  avgt5   21.687 ?   
>> 1.118  ns/op
>> AlgorithmConstraintsPermits.permits  DES  avgt5  324.216 ?   
>> 6.233  ns/op
>> AlgorithmConstraintsPermits.permits NULL  avgt5  709.462 ?  
>> 51.259  ns/op
>> AlgorithmConstraintsPermits.permits   TLS1.3  avgt5  687.497 ? 
>> 170.181  ns/op
>> 
>> Benchmark results after patch:
>> 
>> Benchmark(algorithm)  Mode  CntScore
>> Error  Units
>> AlgorithmConstraintsPermits.permitsSSLv3  avgt5   46.407 ?  
>> 1.057  ns/op
>> AlgorithmConstraintsPermits.permits  DES  avgt5   65.722 ?  
>> 0.578  ns/op
>> AlgorithmConstraintsPermits.permits NULL  avgt5   43.988 ?  
>> 1.264  ns/op
>> AlgorithmConstraintsPermits.permits   TLS1.3  avgt5  399.546 ? 
>> 11.194  ns/op
>> 
>> SSLv3, DES, NULL are the first, middle and last element in 
>> `jdk.tls.disabledAlgorithms` from `conf/security/java.security`.
>> 
>> Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 
>> keep-alive)+Jmeter:
>> Before patch:
>> 
>> summary +  50349 in 00:00:30 = 1678.4/s Avg:   238 Min:   188 Max:   298 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 135183 in 00:01:22 = 1654.5/s Avg:   226 Min:16 Max:  1281 
>> Err: 0 (0.00%)
>> summary +  50240 in 00:00:30 = 1674.1/s Avg:   238 Min:   200 Max:   308 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 185423 in 00:01:52 = 1659.7/s Avg:   229 Min:16 Max:  1281 
>> Err: 0 (0.00%)
>> summary +  50351 in 00:00:30 = 1678.4/s Avg:   238 Min:   191 Max:   306 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 235774 in 00:02:22 = 1663.7/s Avg:   231 Min:16 Max:  1281 
>> Err: 0 (0.00%)
>> summary +  50461 in 00:00:30 = 1681.9/s Avg:   237 Min:   174 Max:   303 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> 
>> After patch:
>> 
>> summary +  59003 in 00:00:30 = 1966.6/s Avg:   203 Min:   158 Max:   272 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 146675 in 00:01:18 = 1884.6/s Avg:   198 Min:26 Max:   697 
>> Err: 0 (0.00%)
>> summary +  58965 in 00:00:30 = 1965.9/s Avg:   203 Min:   166 Max:   257 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 205640 in 00:01:48 = 1907.2/s Avg:   199 Min:26 Max:   697 
>> Err: 0 (0.00%)
>> summary +  59104 in 00:00:30 = 1969.1/s Avg:   203 Min:   157 Max:   266 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 264744 in 00:02:18 = 1920.7/s Avg:   200 Min:26 Max:   697 
>> Err: 0 (0.00%)
>> summary +  59323 in 00:00:30 = 1977.6/s Avg:   202 Min:   158 Max:   256 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> 
>> 
>> Testing: tier1, tier2
>
> Dongbo He has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   refactor the code && fix some errors

src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java 
line 133:

> 131: 
> 132: // Check for alias
> 133: Pattern INCLUDE_PATTERN = Pattern.compile(

You may miss my previous comment.  This variable is not static, hence all 
capital letters naming does not apply here.

As you are already here, it may be nice if you'd like to have this variable as 
a static final class field.  Then, the compile() will be called one time at 
most for this class.  As a static class field, you could use the capitalized 
name.

src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java 
line 139:

> 137: if ((matcher = INCLUDE_PATTERN.matcher(s)).matches()) {
> 138: disabledAlgorithms.remove(matcher.group());
> 139: 
> disabledAlgorithms.addAll(getAlgorithms(PROPERTY_DISABLED_EC_CURVES));

I guess this line exceed 80 characters?  Please keep it in 80 characters for 
each line.

src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java 
line 139:

> 137: if ((matcher = INCLUDE_PATTERN.matcher(s)).matches()) {
> 138: disabledAlgorithms.remove(matcher.group());
> 139: 
> disabledAlgorithms.addAll(getAlgorithms(PROPERTY_DISABLED_EC_CURVES));

It may increase the readability a little bit if having the assignment in the 
declaration line:

- Matcher matcher;
-  

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

2021-06-21 Thread Yi Yang
On Tue, 22 Jun 2021 02:39:01 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:
> 
>   correct exception type; use anonymous inner classes

I found that after solving the problem that Preconditions cannot be used during 
the VM startup, a series of functions such as String.checkIndex/checkOffset/.. 
can also be harmlessly replaced, but this changeset is somewhat large and may 
overwrite the previous review progress. If it will confuse the current review 
progress for reviewers involving in this PR, I'd like to file a new PR to do 
this change later.

-

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


Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance [v4]

2021-06-21 Thread Dongbo He
> Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm 
> has been disabled. It is less efficient when there are more disabled elements 
> in the list, we can use Set instead of List to speed up the search.
> 
> Patch contains a benchmark that can be run with `make test 
> TEST="micro:java.security.AlgorithmConstraintsPermits"`.
> Baseline results before patch:
> 
> Benchmark(algorithm)  Mode  CntScore 
> Error  Units
> AlgorithmConstraintsPermits.permitsSSLv3  avgt5   21.687 ?   
> 1.118  ns/op
> AlgorithmConstraintsPermits.permits  DES  avgt5  324.216 ?   
> 6.233  ns/op
> AlgorithmConstraintsPermits.permits NULL  avgt5  709.462 ?  
> 51.259  ns/op
> AlgorithmConstraintsPermits.permits   TLS1.3  avgt5  687.497 ? 
> 170.181  ns/op
> 
> Benchmark results after patch:
> 
> Benchmark(algorithm)  Mode  CntScoreError 
>  Units
> AlgorithmConstraintsPermits.permitsSSLv3  avgt5   46.407 ?  1.057 
>  ns/op
> AlgorithmConstraintsPermits.permits  DES  avgt5   65.722 ?  0.578 
>  ns/op
> AlgorithmConstraintsPermits.permits NULL  avgt5   43.988 ?  1.264 
>  ns/op
> AlgorithmConstraintsPermits.permits   TLS1.3  avgt5  399.546 ? 11.194 
>  ns/op
> 
> SSLv3, DES, NULL are the first, middle and last element in 
> `jdk.tls.disabledAlgorithms` from `conf/security/java.security`.
> 
> Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 keep-alive)+Jmeter:
> Before patch:
> 
> summary +  50349 in 00:00:30 = 1678.4/s Avg:   238 Min:   188 Max:   298 Err: 
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 135183 in 00:01:22 = 1654.5/s Avg:   226 Min:16 Max:  1281 Err: 
> 0 (0.00%)
> summary +  50240 in 00:00:30 = 1674.1/s Avg:   238 Min:   200 Max:   308 Err: 
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 185423 in 00:01:52 = 1659.7/s Avg:   229 Min:16 Max:  1281 Err: 
> 0 (0.00%)
> summary +  50351 in 00:00:30 = 1678.4/s Avg:   238 Min:   191 Max:   306 Err: 
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 235774 in 00:02:22 = 1663.7/s Avg:   231 Min:16 Max:  1281 Err: 
> 0 (0.00%)
> summary +  50461 in 00:00:30 = 1681.9/s Avg:   237 Min:   174 Max:   303 Err: 
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> 
> After patch:
> 
> summary +  59003 in 00:00:30 = 1966.6/s Avg:   203 Min:   158 Max:   272 Err: 
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 146675 in 00:01:18 = 1884.6/s Avg:   198 Min:26 Max:   697 Err: 
> 0 (0.00%)
> summary +  58965 in 00:00:30 = 1965.9/s Avg:   203 Min:   166 Max:   257 Err: 
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 205640 in 00:01:48 = 1907.2/s Avg:   199 Min:26 Max:   697 Err: 
> 0 (0.00%)
> summary +  59104 in 00:00:30 = 1969.1/s Avg:   203 Min:   157 Max:   266 Err: 
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> summary = 264744 in 00:02:18 = 1920.7/s Avg:   200 Min:26 Max:   697 Err: 
> 0 (0.00%)
> summary +  59323 in 00:00:30 = 1977.6/s Avg:   202 Min:   158 Max:   256 Err: 
> 0 (0.00%) Active: 400 Started: 400 Finished: 0
> 
> 
> Testing: tier1, tier2

Dongbo He has updated the pull request incrementally with one additional commit 
since the last revision:

  refactor the code && fix some errors

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4424/files
  - new: https://git.openjdk.java.net/jdk/pull/4424/files/0253fdb2..b863e2b3

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

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

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


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

2021-06-21 Thread Yi Yang
> 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:

  correct exception type; use anonymous inner classes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4507/files
  - new: https://git.openjdk.java.net/jdk/pull/4507/files/3a8875ec..2330ee38

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4507=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4507=05-06

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

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


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

2021-06-21 Thread Yi Yang
On Mon, 21 Jun 2021 20:49:56 GMT, Paul Sandoz  wrote:

>> Yi Yang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   more replacement 2
>
> src/java.base/share/classes/jdk/internal/util/Preconditions.java line 78:
> 
>> 76: = Preconditions.outOfBoundsExceptionFormatter(new 
>> StringIndexOutOfBoundsExceptionProducer());
>> 77: 
>> 78: public static final BiFunction, 
>> StringIndexOutOfBoundsException> AIOOBE_FORMATTER
> 
> Using incorrect exception type. Suggest you embed as inner class rather than 
> separate declaration, since they are only used in one place.

Fixed.

FYI: Current exception message looks like this:

Exception in thread "main" java.lang.StringIndexOutOfBoundsException: Range [3, 
1) out of bounds for length 6
at 
CheckIndex$StringIndexOutOfBoundsExceptionProducer.apply(CheckIndex.java:77)
at 
CheckIndex$StringIndexOutOfBoundsExceptionProducer.apply(CheckIndex.java:72)
at 
java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:159)
at 
java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:156)
at 
java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:62)
at 
java.base/jdk.internal.util.Preconditions.outOfBoundsCheckFromToIndex(Preconditions.java:76)
at 
java.base/jdk.internal.util.Preconditions.checkFromToIndex(Preconditions.java:295)
at CheckIndex.main(CheckIndex.java:110)

I think now it expresses more exception information than before(and more 
consistent).

-

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


[jdk17] Integrated: 8268349: Provide clear run-time warnings about Security Manager deprecation

2021-06-21 Thread Weijun Wang
On Fri, 11 Jun 2021 01:59:59 GMT, Weijun Wang  wrote:

> More loudly and precise warning messages when a security manager is either 
> enabled at startup or installed at runtime.
> 
> This is new PR for the `openjdk/jdk17` repo copied from 
> https://github.com/openjdk/jdk/pull/4400. A new commit is added.

This pull request has now been integrated.

Changeset: ef4ba224
Author:Weijun Wang 
URL:   
https://git.openjdk.java.net/jdk17/commit/ef4ba224c4887b2e307937754064d3623a2d3de5
Stats: 177 lines in 6 files changed: 102 ins; 33 del; 42 mod

8268349: Provide clear run-time warnings about Security Manager deprecation

Reviewed-by: lancea, mullan, alanb

-

PR: https://git.openjdk.java.net/jdk17/pull/13


[jdk17] Integrated: 8267100: [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs

2021-06-21 Thread Weijun Wang
On Mon, 21 Jun 2021 22:43:58 GMT, Weijun Wang  wrote:

> This is a copy of https://github.com/openjdk/jdk17/pull/100 so that I can 
> integrate the fix for @seanjmullan.

This pull request has now been integrated.

Changeset: e2d7ec38
Author:Weijun Wang 
URL:   
https://git.openjdk.java.net/jdk17/commit/e2d7ec38af4e13cfbd303fa37e766aa2071cfd1f
Stats: 3 lines in 1 file changed: 0 ins; 1 del; 2 mod

8267100: [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs

Co-authored-by: Sean Mullan 
Reviewed-by: hchao, xuelei

-

PR: https://git.openjdk.java.net/jdk17/pull/113


Re: [jdk17] RFR: 8267100: [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs

2021-06-21 Thread Xue-Lei Andrew Fan
On Mon, 21 Jun 2021 22:43:58 GMT, Weijun Wang  wrote:

> This is a copy of https://github.com/openjdk/jdk17/pull/100 so that I can 
> integrate the fix for @seanjmullan.

Marked as reviewed by xuelei (Reviewer).

-

PR: https://git.openjdk.java.net/jdk17/pull/113


Re: [jdk17] RFR: 8267100: [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs

2021-06-21 Thread Hai-May Chao
On Mon, 21 Jun 2021 22:43:58 GMT, Weijun Wang  wrote:

> This is a copy of https://github.com/openjdk/jdk17/pull/100 so that I can 
> integrate the fix for @seanjmullan.

Marked as reviewed by hchao (Committer).

-

PR: https://git.openjdk.java.net/jdk17/pull/113


Re: RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java [v3]

2021-06-21 Thread Hai-May Chao
On Fri, 18 Jun 2021 13:24:17 GMT, Abdul Kolarkunnu  
wrote:

>> ParamsTest is an interop test between keytool <-> openssl. There are some 
>> manual steps listed in jdk/sun/security/pkcs12/params/README to perform 
>> after the execution of jtreg execution. So this test is to perform that 
>> manual steps.
>
> Abdul Kolarkunnu has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java

Marked as reviewed by hchao (Committer).

-

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


[jdk17] RFR: 8267100: [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs

2021-06-21 Thread Weijun Wang
This is a copy of https://github.com/openjdk/jdk17/pull/100 so that I can 
integrate the fix for @seanjmullan.

-

Commit messages:
 - [BACKOUT] JDK-8196415 Disable SHA-1 Signed JARs.

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

PR: https://git.openjdk.java.net/jdk17/pull/113


Re: RFR: 8268427: Improve AlgorithmConstraints:checkAlgorithm performance [v3]

2021-06-21 Thread Xue-Lei Andrew Fan
On Thu, 17 Jun 2021 08:16:42 GMT, Dongbo He  wrote:

>> Now AlgorithmConstraints:checkAlgorithm uses List to check if an algorithm 
>> has been disabled. It is less efficient when there are more disabled 
>> elements in the list, we can use Set instead of List to speed up the search.
>> 
>> Patch contains a benchmark that can be run with `make test 
>> TEST="micro:java.security.AlgorithmConstraintsPermits"`.
>> Baseline results before patch:
>> 
>> Benchmark(algorithm)  Mode  CntScore 
>> Error  Units
>> AlgorithmConstraintsPermits.permitsSSLv3  avgt5   21.687 ?   
>> 1.118  ns/op
>> AlgorithmConstraintsPermits.permits  DES  avgt5  324.216 ?   
>> 6.233  ns/op
>> AlgorithmConstraintsPermits.permits NULL  avgt5  709.462 ?  
>> 51.259  ns/op
>> AlgorithmConstraintsPermits.permits   TLS1.3  avgt5  687.497 ? 
>> 170.181  ns/op
>> 
>> Benchmark results after patch:
>> 
>> Benchmark(algorithm)  Mode  CntScore
>> Error  Units
>> AlgorithmConstraintsPermits.permitsSSLv3  avgt5   46.407 ?  
>> 1.057  ns/op
>> AlgorithmConstraintsPermits.permits  DES  avgt5   65.722 ?  
>> 0.578  ns/op
>> AlgorithmConstraintsPermits.permits NULL  avgt5   43.988 ?  
>> 1.264  ns/op
>> AlgorithmConstraintsPermits.permits   TLS1.3  avgt5  399.546 ? 
>> 11.194  ns/op
>> 
>> SSLv3, DES, NULL are the first, middle and last element in 
>> `jdk.tls.disabledAlgorithms` from `conf/security/java.security`.
>> 
>> Tomcat(maxKeepAliveRequests=1, which will disable HTTP/1.0 
>> keep-alive)+Jmeter:
>> Before patch:
>> 
>> summary +  50349 in 00:00:30 = 1678.4/s Avg:   238 Min:   188 Max:   298 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 135183 in 00:01:22 = 1654.5/s Avg:   226 Min:16 Max:  1281 
>> Err: 0 (0.00%)
>> summary +  50240 in 00:00:30 = 1674.1/s Avg:   238 Min:   200 Max:   308 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 185423 in 00:01:52 = 1659.7/s Avg:   229 Min:16 Max:  1281 
>> Err: 0 (0.00%)
>> summary +  50351 in 00:00:30 = 1678.4/s Avg:   238 Min:   191 Max:   306 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 235774 in 00:02:22 = 1663.7/s Avg:   231 Min:16 Max:  1281 
>> Err: 0 (0.00%)
>> summary +  50461 in 00:00:30 = 1681.9/s Avg:   237 Min:   174 Max:   303 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> 
>> After patch:
>> 
>> summary +  59003 in 00:00:30 = 1966.6/s Avg:   203 Min:   158 Max:   272 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 146675 in 00:01:18 = 1884.6/s Avg:   198 Min:26 Max:   697 
>> Err: 0 (0.00%)
>> summary +  58965 in 00:00:30 = 1965.9/s Avg:   203 Min:   166 Max:   257 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 205640 in 00:01:48 = 1907.2/s Avg:   199 Min:26 Max:   697 
>> Err: 0 (0.00%)
>> summary +  59104 in 00:00:30 = 1969.1/s Avg:   203 Min:   157 Max:   266 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> summary = 264744 in 00:02:18 = 1920.7/s Avg:   200 Min:26 Max:   697 
>> Err: 0 (0.00%)
>> summary +  59323 in 00:00:30 = 1977.6/s Avg:   202 Min:   158 Max:   256 
>> Err: 0 (0.00%) Active: 400 Started: 400 Finished: 0
>> 
>> 
>> Testing: tier1, tier2
>
> Dongbo He has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Replace HashSet with TreeSet

Changes requested by xuelei (Reviewer).

src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java 
line 90:

> 88: }
> 89: 
> 90: Set elements = null;

This line could be placed and merged with line 96-98.

`Set elements = decomposer.decompose(algorithm);`

src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java 
line 101:

> 99: // check the element of the elements
> 100: for (String element : elements) {
> 101: if (algorithms.contains(algorithm)) {

The check should be placed on the decomposed elements.


- if (algorithms.contains(algorithm)) 
+ if (algorithms.contains(element))

src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java 
line 133:

> 131: 
> 132: // Check for alias
> 133: Pattern INCLUDE_PATTERN = Pattern.compile("include " + 
> PROPERTY_DISABLED_EC_CURVES, Pattern.CASE_INSENSITIVE);

Per the Java code conventions, the variable name could be "includePattern",  
rather than using capital all letters.

Please keep each line less than or equal to 80 bytes.

src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java 
line 134:

> 132: // Check for alias
> 133: Pattern INCLUDE_PATTERN = Pattern.compile("include " + 
> PROPERTY_DISABLED_EC_CURVES, Pattern.CASE_INSENSITIVE);
> 134: Matcher matcher;

I think the performance impact should be 

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

2021-06-21 Thread Paul Sandoz
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?

src/java.base/share/classes/jdk/internal/util/Preconditions.java line 78:

> 76: = Preconditions.outOfBoundsExceptionFormatter(new 
> StringIndexOutOfBoundsExceptionProducer());
> 77: 
> 78: public static final BiFunction, 
> StringIndexOutOfBoundsException> AIOOBE_FORMATTER

Using incorrect exception type. Suggest you embed as inner class rather than 
separate declaration, since they are only used in one place.

-

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


Re: RFR: JDK-8268464 : Remove dependancy of TestHttpsServer, HttpTransaction, HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests [v4]

2021-06-21 Thread Michael McMahon
On Thu, 17 Jun 2021 16:23:08 GMT, Mahendra Chhipa 
 wrote:

>> …HttpCallback from open/test/jdk/sun/net/www/protocol/https/ tests
>
> Mahendra Chhipa has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Implemented review comments

test/jdk/sun/net/www/protocol/https/ChunkedOutputStream.java line 89:

> 87: return hostaddr + ":" + server.getAddress().getPort();
> 88: }
> 89: public void handle(HttpExchange req) throws IOException {

Minor style point. I would put a blank line between the two methods.

test/jdk/sun/net/www/protocol/https/ChunkedOutputStream.java line 170:

> 168: req.sendResponseHeaders(404, -1);
> 169: break;
> 170: }

Probably should add a call to "req.close()" at the end of the method.

test/jdk/sun/net/www/protocol/https/ChunkedOutputStream.java line 366:

> 364: TrustManagerFactory tmf = 
> TrustManagerFactory.getInstance("SunX509");
> 365: tmf.init(ts);
> 366: 

Could SimpleSSLContext be used here instead of manually writing this code? Same 
for other tests with same pattern.

-

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