Re: RFR: 8266182: Create a manual test for jdk/sun/security/pkcs12/ParamsTest.java [v3]
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]
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]
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]
> 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]
> 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]
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
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
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
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
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]
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
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]
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]
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]
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