Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v9]
On Wed, 1 Jun 2022 07:50:58 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов has updated the pull request incrementally with one additional > commit since the last revision: > > 8282662: Revert wrong copyright year change Marked as reviewed by rriggs (Reviewer). Thanks for the improvements. - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v8]
On Tue, 31 May 2022 19:30:07 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - 8282662: Revert wrong copyright year change > - 8282662: Revert ProxyGenerator > - 8282662: Revert ProxyGenerator > - 8282662: Revert dubious changes in MethodType > - 8282662: Revert dubious changes > - 8282662: Use List/Set.of() factory methods to save memory Merging is preferable, it doesn't disturb the history. Usually an explicit merge is not necessary; Skara will indicate when an automatic merge is needed due to conflicts. If you want to run your own tests then use a merge, not rebase. Thanks - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v8]
On Tue, 31 May 2022 19:30:07 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - 8282662: Revert wrong copyright year change > - 8282662: Revert ProxyGenerator > - 8282662: Revert ProxyGenerator > - 8282662: Revert dubious changes in MethodType > - 8282662: Revert dubious changes > - 8282662: Use List/Set.of() factory methods to save memory Why the force push? They are discouraged, making it harder to review. - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v7]
On Tue, 31 May 2022 07:40:56 GMT, Сергей Цыпанов wrote: >> `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with >> smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when >> called with vararg of size 0, 1, 2. >> >> In general replacement of `Arrays.asList()` with `List.of()` is dubious as >> the latter is null-hostile, however in some cases we are sure that arguments >> are non-null. Into this PR I've included the following cases (in addition to >> those where the argument is proved to be non-null at compile-time): >> - `MethodHandles.longestParameterList()` never returns null >> - parameter types are never null >> - interfaces used for proxy construction and returned from >> `Class.getInterfaces()` are never null >> - exceptions types of method signature are never null > > Сергей Цыпанов 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 six additional > commits since the last revision: > > - 8282662: Revert wrong copyright year change > - 8282662: Revert ProxyGenerator > - 8282662: Revert ProxyGenerator > - 8282662: Revert dubious changes in MethodType > - 8282662: Revert dubious changes > - 8282662: Use List/Set.of() factory methods to save memory Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8286045: Use ForceGC for cleaner test cases
On Wed, 25 May 2022 15:20:45 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > May I have the test case update reviewed? > > This patch is trying to use ForceGC for cleaner test cases. As would make > the test more stable and easier to maintain. > > Thanks, > Xuelei The usage looks fine. (But ForceGC is a heavyweight blunt instrument. It creates a new Cleaner for every instance and an instance can only be used once. Also, its minimum wait/sleep time is 1 second, that's a lng time.). - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8885
Re: RFR: JDK-6725221 Standardize obtaining boolean properties with defaults [v2]
On Mon, 23 May 2022 18:58:34 GMT, Mark Powers wrote: >> JDK-6725221 Standardize obtaining boolean properties with defaults > > Mark Powers has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains six commits: > > - Alan and Jamil comments > - Merge > - reverse true.equals and false.equals changes > - Merge > - Merge > - first iteration LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8559
Re: RFR: JDK-6725221 Standardize obtaining boolean properties with defaults
On Tue, 10 May 2022 19:24:24 GMT, Mark Powers wrote:
>> src/java.base/share/classes/java/lang/reflect/AccessibleObject.java line 777:
>>
>>> 775: if (!printStackPropertiesSet && VM.initLevel() >= 1) {
>>> 776: printStackWhenAccessFails = GetBooleanAction.
>>> 777:
>>> privilegedGetProperty("sun.reflect.debugModuleAccessChecks");
>>
>> Running with `-Dsun.reflect.debugModuleAccessChecks` should set
>> printStackWhenAccessFails to true, not false.
>
> You are right. The old way maps the null string to true, and the new way maps
> it to false. I did not notice that. At this point, I see no value in making
> the "true".equals and "false".equals changes. Too much can break. I'll
> reverse these changes.
This change still needs to be reversed.
-
PR: https://git.openjdk.java.net/jdk/pull/8559
Integrated: 8286393: Address possibly lossy conversions in java.rmi
On Thu, 12 May 2022 16:47:43 GMT, Roger Riggs wrote: > Updates to modules java.rmi and java.smartcardio to remove warnings about > lossy-conversions introduced by PR#8599. > > Explicit casts are inserted where implicit casts occur. > > 8286393: Address possibly lossy conversions in java.rmi > 8286388: Address possibly lossy conversions in java.smartcardio This pull request has now been integrated. Changeset: 237f2801 Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/237f28014ab9d27d2cdfe3fdc4a5b0a0680f8e95 Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod 8286393: Address possibly lossy conversions in java.rmi 8286388: Address possibly lossy conversions in java.smartcardio Reviewed-by: lancea, dfuchs, smarks - PR: https://git.openjdk.java.net/jdk/pull/8683
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Fri, 13 May 2022 05:54:15 GMT, Alan Bateman wrote: >> src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java line 128: >> >>> 126: // timed poll interrupted so need to adjust timeout >>> 127: long adjust = System.nanoTime() - startTime; >>> 128: to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust); >> >> This will now always assign a negative number to `to`. >> >> >> >> `=-` is not a compound assignment, it’s negation followed by a normal >> assignment. > > Well spotted, I don't think that change was intentionally. Ouch; Will fix: I took Alan's earlier comment literally: "This one can also be changed to: to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust);" - PR: https://git.openjdk.java.net/jdk/pull/8642
RFR: 8286393: Address possibly lossy conversions in java.rmi
Updates to modules java.rmi and java.smartcardio to remove warnings about lossy-conversions introduced by PR#8599. Explicit casts are inserted where implicit casts occur. 8286393: Address possibly lossy conversions in java.rmi 8286388: Address possibly lossy conversions in java.smartcardio - Commit messages: - 8286393: Address possibly lossy conversions in java.rmi Changes: https://git.openjdk.java.net/jdk/pull/8683/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8683=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286393 Stats: 6 lines in 2 files changed: 0 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/8683.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8683/head:pull/8683 PR: https://git.openjdk.java.net/jdk/pull/8683
Integrated: 8286378: Address possibly lossy conversions in java.base
On Tue, 10 May 2022 21:32:10 GMT, Roger Riggs wrote: > PR#8599 8244681: proposes to add compiler warnings for possible lossy > conversions > From the CSR: > > "If the type of the right-hand operand of a compound assignment is not > assignment compatible with the type of the variable, a cast is implied and > possible lossy conversion may silently occur. While similar situations are > resolved as compilation errors for primitive assignments, there are no > similar rules defined for compound assignments." > > This PR anticipates the warnings and adds explicit casts to replace the > implicit casts. > In most cases, the cast matches the type of the right-hand side to type of > the compound assignment. > Since these casts are truncating the value, there might be a different > refactoring that avoids the cast > but a direct replacement was chosen here. > > (Advise and suggestions will inform similar updates to other OpenJDK modules). This pull request has now been integrated. Changeset: 17c52789 Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/17c52789b79a4ccd65308f90c4e02c1732b206be Stats: 71 lines in 32 files changed: 0 ins; 3 del; 68 mod 8286378: Address possibly lossy conversions in java.base Reviewed-by: naoto, xuelei, bpb, alanb - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
> PR#8599 8244681: proposes to add compiler warnings for possible lossy > conversions > From the CSR: > > "If the type of the right-hand operand of a compound assignment is not > assignment compatible with the type of the variable, a cast is implied and > possible lossy conversion may silently occur. While similar situations are > resolved as compilation errors for primitive assignments, there are no > similar rules defined for compound assignments." > > This PR anticipates the warnings and adds explicit casts to replace the > implicit casts. > In most cases, the cast matches the type of the right-hand side to type of > the compound assignment. > Since these casts are truncating the value, there might be a different > refactoring that avoids the cast > but a direct replacement was chosen here. > > (Advise and suggestions will inform similar updates to other OpenJDK modules). Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Updated copyrights Fixed cast style to add a space after cast, (where consistent with file style) Improved code per review comments in PollSelectors. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8642/files - new: https://git.openjdk.java.net/jdk/pull/8642/files/7ff806a5..a6679ce7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8642=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8642=01-02 Stats: 41 lines in 24 files changed: 0 ins; 0 del; 41 mod Patch: https://git.openjdk.java.net/jdk/pull/8642.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8642/head:pull/8642 PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v2]
> PR#8599 8244681: proposes to add compiler warnings for possible lossy > conversions > From the CSR: > > "If the type of the right-hand operand of a compound assignment is not > assignment compatible with the type of the variable, a cast is implied and > possible lossy conversion may silently occur. While similar situations are > resolved as compilation errors for primitive assignments, there are no > similar rules defined for compound assignments." > > This PR anticipates the warnings and adds explicit casts to replace the > implicit casts. > In most cases, the cast matches the type of the right-hand side to type of > the compound assignment. > Since these casts are truncating the value, there might be a different > refactoring that avoids the cast > but a direct replacement was chosen here. > > (Advise and suggestions will inform similar updates to other OpenJDK modules). Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: remove stray file - Changes: - all: https://git.openjdk.java.net/jdk/pull/8642/files - new: https://git.openjdk.java.net/jdk/pull/8642/files/e72ce85c..7ff806a5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8642=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8642=00-01 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/8642.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8642/head:pull/8642 PR: https://git.openjdk.java.net/jdk/pull/8642
RFR: 8286378: Address possibly lossy conversions in java.base
PR#8599 8244681: proposes to add compiler warnings for possible lossy conversions >From the CSR: "If the type of the right-hand operand of a compound assignment is not assignment compatible with the type of the variable, a cast is implied and possible lossy conversion may silently occur. While similar situations are resolved as compilation errors for primitive assignments, there are no similar rules defined for compound assignments." This PR anticipates the warnings and adds explicit casts to replace the implicit casts. In most cases, the cast matches the type of the right-hand side to type of the compound assignment. Since these casts are truncating the value, there might be a different refactoring that avoids the cast but a direct replacement was chosen here. (Advise and suggestions will inform similar updates to other OpenJDK modules). - Commit messages: - 8286378: Address possibly lossy conversions in java.base Changes: https://git.openjdk.java.net/jdk/pull/8642/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8642=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8286378 Stats: 57 lines in 33 files changed: 1 ins; 3 del; 53 mod Patch: https://git.openjdk.java.net/jdk/pull/8642.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8642/head:pull/8642 PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8285785: CheckCleanerBound test fails with PasswordCallback object is not released
On Thu, 28 Apr 2022 12:19:35 GMT, Sean Mullan wrote: >> Hi, >> >> May I have this test update reviewed? >> >> The javax/security/auth/callback/PasswordCallback/CheckCleanerBound.java >> test case failed on one of the test setups. The test runs gc in a loop and >> expects the GC to have garbage collected contents of a WeakHashMap. The loop >> runs for 10 iterations. Some delay needs to be added between each iteration >> to increase the chances of GC garbage collecting the instances. >> >> Thanks, >> Xuelei > > test/jdk/javax/security/auth/callback/PasswordCallback/CheckCleanerBound.java > line 50: > >> (failed to retrieve contents of file, check the PR for context) > Looks ok to me although @RogerRiggs commented in the previous PR that 10ms > should be sufficient. Alternatively, the loop count could be raised by 10x. That would keep the typical running time low and still allow for a worst case. Your choice. - PR: https://git.openjdk.java.net/jdk/pull/8443
Re: RFR: 8285785: CheckCleanerBound test fails with PasswordCallback object is not released
On Thu, 28 Apr 2022 07:01:25 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > May I have this test update reviewed? > > The javax/security/auth/callback/PasswordCallback/CheckCleanerBound.java test > case failed on one of the test setups. The test runs gc in a loop and > expects the GC to have garbage collected contents of a WeakHashMap. The loop > runs for 10 iterations. Some delay needs to be added between each iteration > to increase the chances of GC garbage collecting the instances. > > Thanks, > Xuelei Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8443
Re: RFR: 8284910: Buffer clean in PasswordCallback [v8]
On Wed, 27 Apr 2022 16:02:05 GMT, Xue-Lei Andrew Fan wrote:
>> Please review this password cleanup enhancement in the PasswordCallback
>> implementation. This is one of the effort to clean up the buffered
>> passwords.
>>
>> The PasswordCallback.setPassword() clones the password, but is not
>> registered for cleanup. An application could call clearPassword() for the
>> purpose, but it would be nice to cleanup the buffer as well if
>> clearPassword() was not called in an application. And, if the setPassword()
>> get called multiple times, the clearPassword() should also be called the
>> same times if not relying on finalization. It could be fragile in practice.
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> remove trailing whitespace
test/jdk/javax/security/auth/callback/PasswordCallback/CheckCleanerBound.java
line 50:
> 48: for (int i = 0; i < 10 && weakHashMap.size() != 0; i++) {
> 49: System.gc();
> 50: Thread.sleep(100);
You can drop this sleep to 10ms to cut the average test time. It might be
interesting to know how many retries are typical.
-
PR: https://git.openjdk.java.net/jdk/pull/8272
Re: RFR: 8284910: Buffer clean in PasswordCallback [v2]
On Wed, 20 Apr 2022 19:59:16 GMT, Sean Mullan wrote: >>> Won't there be a small performance hit (perhaps negligible) for code that >>> already calls clearPassword? >> >> The impact should be minimal. If clearPassword() has been called, the >> cleanup (Cleanerable.clean()) won't happen again in the finalization or >> setPassword cleanup. >> >>> A specification clarification would provide clarity to applications that >>> they do not have to call clearPassword in between calls to setPassword. >> >> As far as I know from the JDK code, it might be not common to call >> clearPassword in between calls to setPassword. I would like to have >> applications calling clearPassword() methods as before, if it is not missed, >> to speed up the collection rather than rely on finalization. >> >> The relationship among setPassword, getPassword and clearPassword() is >> complicated. I fully agree that the spec should be clarified. I would like >> to have the clarify update in another PR, and have this one focus on cleanup >> if an application forget to call clearPassword properly. > >> > Won't there be a small performance hit (perhaps negligible) for code that >> > already calls clearPassword? >> >> The impact should be minimal. If clearPassword() has been called, the >> cleanup (Cleanerable.clean()) won't happen again in the finalization or >> setPassword cleanup. > > I don't think that is true, but maybe I am missing something. From looking at > the code, it appears a `clearPassword` followed by a `setPassword` would call > `Arrays.fill` twice on the same password. I think this can be fixed by > setting the cleaner to null in the `clearPassword` method after the password > has been cleared. I think this would address my concern and we may not need a > spec. update (though I want to think it thru one more time). > >> > A specification clarification would provide clarity to applications that >> > they do not have to call clearPassword in between calls to setPassword. >> >> As far as I know from the JDK code, it might be not common to call >> clearPassword in between calls to setPassword. I would like to have >> applications calling clearPassword() methods as before, if it is not missed, >> to speed up the collection rather than rely on finalization. > > Yes, I agree calling `clearPassword` should be recommended as a best practice > and callers should not solely rely on cleaners. > >> The relationship among setPassword, getPassword and clearPassword() is >> complicated. I fully agree that the spec should be clarified. I would like >> to have the clarify update in another PR, and have this one focus on cleanup >> if an application forget to call clearPassword properly. > > See above. Calling `Cleanable.clean()` calls the `Runnable` action at-most-once. Each `Cleanable` inserted in a list when it is created and is removed when `clear()` or `clean()` is invoked. The action is called only if it is still in the list. So there is no extra overhead. There is no harm in clearing the cleanable field; but it does not save much. The code in `clearPassword` can be simplified and only test `cleanable != null`; it will be null unless there is an inputPassword to clean. - PR: https://git.openjdk.java.net/jdk/pull/8272
Re: RFR: 8284910: Buffer clean in PasswordCallback [v2]
On Mon, 18 Apr 2022 18:15:25 GMT, Sean Mullan wrote: >> setPassword can/should always clear the previous password. It is an >> internal copy that no one else has a reference to and is being replaced. >> It will need to either explicitly call cleanable.clean() or fill/erase the >> array itself. >> Overwriting the cleanable will prevent the existing cleanable from being >> processed. >> It don't think it needs a spec change, the internal value is implementation >> only. > > What about code that is already calling `clearPassword` between calls to > `setPassword`? This seems to be a change in the design of this API. The > `clearPassword` method is there to allow callers to manage and clear the > passwords itself. I think its "belt and suspenders". If the caller does not call `clearPassword` before calling a second `setPassword,` the previous char array will still contain the previous password and remain uncleared in memory for a (longer) indeterminate time. It is fulfilling the same purpose as the original finalizer. - PR: https://git.openjdk.java.net/jdk/pull/8272
Re: RFR: 8284910: Buffer clean in PasswordCallback [v2]
On Mon, 18 Apr 2022 17:27:04 GMT, Sean Mullan wrote: >> src/java.base/share/classes/javax/security/auth/callback/PasswordCallback.java >> line 123: >> >>> 121: cleanable = CleanerFactory.cleaner().register( >>> 122: this, cleanerFor(inputPassword)); >>> 123: } >> >> If `setPassword` is called twice in succession, should the previous password >> be cleaned before the new one is assigned and registered? > > I can see why that might be a good idea. Would require a specification change > though. I also think it is fine to keep the behavior the same, and place the > responsibility on the application to call `clearPassword` before setting a > new one. We could add a warning though, something like: "Note: > `clearPassword` should be called to clear any prior password before calling > `setPassword` multiple times on the same `PasswordCallback` instance." setPassword can/should always clear the previous password. It is an internal copy that no one else has a reference to and is being replaced. It will need to either explicitly call cleanable.clean() or fill/erase the array itself. Overwriting the cleanable will prevent the existing cleanable from being processed. It don't think it needs a spec change, the internal value is implementation only. - PR: https://git.openjdk.java.net/jdk/pull/8272
Re: RFR: 8284910: Buffer clean in PasswordCallback
On Sat, 16 Apr 2022 15:45:21 GMT, Xue-Lei Andrew Fan wrote: > Please review this password cleanup enhancement in the PasswordCallback > implementation. This is one of the effort to clean up the buffered passwords. > > The PasswordCallback.setPassword() clones the password, but is not registered > for cleanup. An application could call clearPassword() for the purpose, but > it would be nice to cleanup the buffer as well if clearPassword() was not > called in an application. And, if the setPassword() get called multiple > times, the clearPassword() should also be called the same times if not > relying on finalization. It could be fragile in practice. Please combine these closely related cases into a single test, preferably with TestNG or JUnit. test/jdk/javax/security/auth/callback/ClearPasswordMethod.java line 53: > 51: System.gc(); > 52: Thread.sleep(100); > 53: } The test could complete more quickly if the loop was exited when whm.size() == 0. And in the other test. - PR: https://git.openjdk.java.net/jdk/pull/8272
Re: RFR: 8284855: Update needed to Cleaners added to jdk.crypto.cryptoki [v4]
On Fri, 15 Apr 2022 15:04:20 GMT, Xue-Lei Andrew Fan wrote:
>> This is an effort to fix a problem introduced in the fix for
>> [JDK-8284368](https://bugs.openjdk.java.net/browse/JDK-8284368), which
>> replaced the finalizers in jdk.crypto.cryptoki with Cleaners. However,
>> there is a problem with the code changes. The Runnables registered with
>> Cleaner refer to the object being registered ('this'). Meaning, the Cleaner
>> mechanism will keep the objects reachable, preventing them from being
>> cleaned and collected.
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> more update on replace lambda
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java
line 117:
> 115:
> 116: // Note: Please don't update this field other than the constructor.
> 117: // Otherwise, the native data is not able to be collected.
If it is only set in the constructor, it can be 'final".
src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java
line 235:
> 233: * and any subsequent calls to C_ methods will fail. This method is
> for
> 234: * internal use only. Please don't use this method other than
> finalization.
> 235: *
Perhaps refer to the code in which the cleanup occurs.
-
PR: https://git.openjdk.java.net/jdk/pull/8248
Re: RFR: 8284490: Remove finalizer method in java.security.jgss [v3]
On Thu, 7 Apr 2022 19:21:31 GMT, Xue-Lei Andrew Fan wrote: >> Please review the update to remove finalizer method in the >> java.security.jgss module. It is one of the efforts to clean up the use of >> finalizer method in JDK. > > Xue-Lei Andrew Fan has updated the pull request incrementally with one > additional commit since the last revision: > > Update copyright year LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8136
Re: RFR: 8284490: Remove finalizer method in java.security.jgss
On Thu, 7 Apr 2022 10:51:13 GMT, Daniel Fuchs wrote: >> Please review the update to remove finalizer method in the >> java.security.jgss module. It is one of the efforts to clean up the use of >> finalizer method in JDK. > > src/java.security.jgss/share/classes/sun/security/jgss/wrapper/GSSCredElement.java > line 74: > >> 72: name = srcName; >> 73: >> 74: Cleaner.create().register(this, this::dispose); > > This will create a memory leak: `this` will never be garbage collected if > it's referenced by the cleaner action. The cleaner gets triggered when the instance is unreachable, no values from that instance can be modified or referenced. Any information needed for the cleanup must be in a separate object. In this case, that is pCred and cStub. Typically, new nested class is defined that holds the pCred and cStub and has a run() method that calls `cStub.releaseCred(pCred)`. But the package access to non-final `pCred` raises a flag about when its mutated. If its mutated after the cleaner is created, the cleaner will release the wrong cred. If the GSSCredElement is itself just a holder of the credentials, then perhaps the cleaner should be triggered on the instance that is referring to the GSSCredElement being unreachable. - PR: https://git.openjdk.java.net/jdk/pull/8136
Re: RFR: 8282819: Deprecate Locale class constructors
On Thu, 24 Mar 2022 22:01:30 GMT, Naoto Sato wrote: > Proposing to deprecate the constructors in the `java.util.Locale` class. > There is already a factory method and a builder to return singletons, so > there is no need to have constructors anymore unless one purposefully wants > to create `ill-formed` Locale objects, which is discouraged. We cannot > terminally deprecate those constructors for the compatibility to serialized > objects created with older JDKs. Please see the draft CSR for more detail. Given the large number of cleanups, I would have suggested to keep them separate from the change to re-focus the API on factories. - PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282819: Deprecate Locale class constructors
On Thu, 24 Mar 2022 22:01:30 GMT, Naoto Sato wrote:
> Proposing to deprecate the constructors in the `java.util.Locale` class.
> There is already a factory method and a builder to return singletons, so
> there is no need to have constructors anymore unless one purposefully wants
> to create `ill-formed` Locale objects, which is discouraged. We cannot
> terminally deprecate those constructors for the compatibility to serialized
> objects created with older JDKs. Please see the draft CSR for more detail.
src/java.base/share/classes/java/util/Locale.java line 245:
> 243: * Factory Method
> 244: *
> 245: * The method {@link #forLanguageTag} obtains a {@code Locale}
The factory name `forLanguageTag` is a bit off-putting, it doesn't seem like
the best name for the factory.
Yes, it already exists and does what's required but you might get better uptake
with a more natural name.
Some alternatives:
- `Locale.of("en_US")` - short and conventional
- `Locale.ofLanguage("en_US")` - 'of' prefix is used in other factories
- `Locale.forLanguage("en_US")` - natural but less conventional
-
PR: https://git.openjdk.java.net/jdk/pull/7947
Re: RFR: 8282723: Add constructors taking a cause to JSSE exceptions [v2]
On Tue, 8 Mar 2022 05:51:21 GMT, Xue-Lei Andrew Fan wrote:
>> src/java.base/share/classes/sun/security/ssl/ECDHKeyExchange.java line 204:
>>
>>> 202: } catch (GeneralSecurityException | java.io.IOException e)
>>> {
>>> 203: throw new SSLHandshakeException(
>>> 204: "Could not generate ECPublicKey", e);
>>
>> Nit: I think combining these lines would be < 80 chars
>
> The exception name is too long to have in one line. There are 83 characters
> in total if combining line 203 and 204. 3 characters exceeding 80 chars per
> line limit is not easy to tell with eyes.
The 80 character recommendation isn't a hard limit, I favor readability in the
80-100 char range.
-
PR: https://git.openjdk.java.net/jdk/pull/7722
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
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 rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
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 We usually request that these be be broken up by area to attract the appropriate reviewers and avoid eye-strain. The client modules are usually separated out, as are hotspot. And corresponding tests. This kind of change is pretty low value for the code base and requires the involvement of quite a few reviewers. If you had ask ahead of time, I would have suggested finding something with more value. - Changes requested by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282279: Interpret case-insensitive string locale independently
On Wed, 23 Feb 2022 00:05:58 GMT, Xue-Lei Andrew Fan wrote: > The String.toUpperCase() or String.toLowerCase() method is locale sensitive, > and may produce unexpected results if used for strings that are intended to > be interpreted locale independently. The use of the two methods had been > cleaned before, but there are a few new introduced laterly. > > See https://www.ivi.co/java/2011/07/07/4878.html for examples. Locale.ROOT is preferred for locale independent uppercase and lowercase(). BTW, for the strings written to the SSLLogger, I would have kept the original case intact. Yes, they are equivalent but it slightly obscures the original data and uses a bit of cpu. - Changes requested by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7583
Re: RFR: 8272777: Clean up remaining AccessController warnings in test library
On Wed, 2 Feb 2022 21:35:59 GMT, Kevin Walls wrote:
> Reduce noise in test output by adding the @SuppressWarnings("removal")
> annotation (which has already been widely applied).
Marked as reviewed by rriggs (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/7328
Re: RFR: JDK-8281082: Improve javadoc references to JOSS
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. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7315
Re: RFR: 8272317: jstatd has dependency on Security Manager which needs to be removed [v2]
On Mon, 10 Jan 2022 11:17:12 GMT, Kevin Walls wrote: >> Remove the use of Security Manager from jstatd. >> Add use of an ObjectInputFilter to restrict RMI. >> >> Also we can undo the property-setting Launcher.gmk change from: 8279007: >> jstatd fails to start because SecurityManager is disabled >> ..as that is no longer needed. >> >> Docs/man page update to follow (JDK-8278619). > > Kevin Walls has updated the pull request incrementally with one additional > commit since the last revision: > > Wildcard in object filter to permit proxies, in case other activity in this > JVM changes the nameing/numbering of proxy classes. LGTM test/jdk/sun/tools/jstatd/JstatdTest.java line 2: > 1: /* > 2: * Copyright (c) 2013, 2021, Oracle and/or its affiliates. All rights > reserved. Update copyrights. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6919
Re: RFR: 8272317: jstatd has dependency on Security Manager which needs to be removed
On Wed, 22 Dec 2021 21:41:13 GMT, Mandy Chung wrote: >> Remove the use of Security Manager from jstatd. >> Add use of an ObjectInputFilter to restrict RMI. >> >> Also we can undo the property-setting Launcher.gmk change from: 8279007: >> jstatd fails to start because SecurityManager is disabled >> ..as that is no longer needed. >> >> Docs/man page update to follow (JDK-8278619). > > src/jdk.jstatd/share/classes/sun/tools/jstatd/Jstatd.java line 51: > >> 49: private static RemoteHost remoteHost; >> 50: >> 51: private static final String rmiFilterPattern = >> "sun.jvmstat.monitor.remote.RemoteVm;com.sun.proxy.jdk.proxy1.$Proxy1;com.sun.proxy.jdk.proxy1.$Proxy2;java.lang.reflect.Proxy;java.rmi.server.RemoteObjectInvocationHandler;java.rmi.server.RemoteObject;!*"; > > The class name of the dynamic proxy is generated at runtime and can be > different. As Bernd commented, the proxy classes cannot/should not be > listed in the filter pattern. @mlchung The Proxy class passed to the filter has been created in this VM from the interfaces listed. The interfaces have already been filtered prior to creating the proxy. The Proxy classes can safely be allowed based on a wildcard of the name. (As Stuart said). - PR: https://git.openjdk.java.net/jdk/pull/6919
Re: RFR: 8277932: Subject:callAs() not throwing NPE when action is null
On Mon, 6 Dec 2021 22:22:14 GMT, Weijun Wang wrote: > Add null check. I must have thought the NPE will be thrown anyway but the > `catch Exception` block swallows it. > > I added a noreg-trivial label. If you think differently can add one. Yes, the stack trace will be more useful with the explicit check, otherwise it would be buried inside other calls and it won't be so obvious that the argument passed to callAs is null. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6728
Re: RFR: JDK-8276447 Deprecate finalization-related methods for removal
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 ).
LGTM
-
Marked as reviewed by rriggs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6465
Re: RFR: 8274333: Redundant null comparison after Pattern.split
On Sun, 26 Sep 2021 15:10:52 GMT, Andrey Turbanov wrote: > In couple of classes, result part of arrays of Pattern.split is compared with > `null`. Pattern.split (and hence String.split) never returns `null` in array > elements. Such comparisons are redundant. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5708
Re: RFR: 4337793: Mark non-serializable fields of java.security.cert.Certificate and CertPath [v2]
On Mon, 15 Nov 2021 21:53:56 GMT, Sean Mullan wrote:
>> Please review this 20+ year old bug (!), which marks the non-serializable
>> fields of Certificate and CertPath with the transient modifier. These
>> classes use an alternate serialization mechanism by overriding the
>> writeReplace method. However, the fields of each class were never marked as
>> transient and as a result were incorrectly documented in the Serialized Form
>> section of the javadoc.
>>
>> CSR: https://bugs.openjdk.java.net/browse/JDK-8277128
>
> Sean Mullan has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Improve writeReplace methods to better describe what object is returned.
> Small wording improvements and use of {@code} in readResolve methods and
> CertPath serial fields.
> Corrected typo in type param of CertPathRep ctor (should be "Certificate
> type" and not "CertPath type").
Marked as reviewed by rriggs (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/6392
Re: RFR: 4337793: Mark non-serializable fields of java.security.cert.Certificate and CertPath
On Mon, 15 Nov 2021 19:05:27 GMT, Sean Mullan wrote: > > The serialized form doc for both classes should say something about what is > > serialized. Since it is using writeReplace, it can indicate that > > CertificateRep/CertPathRep is used instead (and the arguments). likely > > you'll need to use the @serial javadoc tag and check the generated javadoc > > to verify. > > The `writeReplace` methods do have `@serial` tags, and they do show up in the > Serialized Form of the javadoc, ex: > https://docs.oracle.com/en/java/javase/17/docs/api/serialized-form.html#java.security.cert.Certificate > > Is your comment more that these methods could more clearly specify what is > returned? The `@java.io.Serial` annotation doesn't add any description to the generated javadoc or serialized form doc. The javadoc in the serialized for says only: "Replace the Certificate to be serialized." For my purposes it would enough to reword it to say it returns a CertificateRep holding the type and data or something similar. The `@serial` javadoc tag might be useful but not necessary if the regular javadoc that is in the serialized form doc is concrete. - PR: https://git.openjdk.java.net/jdk/pull/6392
Re: RFR: 4337793: Mark non-serializable fields of java.security.cert.Certificate and CertPath
On Mon, 15 Nov 2021 17:03:51 GMT, Sean Mullan wrote: > Please review this 20+ year old bug (!), which marks the non-serializable > fields of Certificate and CertPath with the transient modifier. These classes > use an alternate serialization mechanism by overriding the writeReplace > method. However, the fields of each class were never marked as transient and > as a result were incorrectly documented in the Serialized Form section of the > javadoc. > > CSR: https://bugs.openjdk.java.net/browse/JDK-8277128 The serialized form doc for both classes should say something about what is serialized. Since it is using writeReplace, it can indicate that CertificateRep/CertPathRep is used instead (and the arguments). likely you'll need to use the @serial javadoc tag and check the generated javadoc to verify. - Changes requested by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6392
Re: RFR: 8276348: Use blessed modifier order in java.base
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 loose, 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 Keep it as is with the modifiers in the recommended order. I don't think adding extra typography is warranted. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
On Tue, 2 Nov 2021 18:48:06 GMT, Mark Sheppard wrote: >> Here's a bit of archaeology. I found the original JDK-8136583 patch, which >> has moved from where it was in the RFR to >> https://cr.openjdk.java.net/~martin/webrevs/jdk9/blessed-modifier-order/. >> That patch doesn't change Object.java. The RFR thread mentions neither that >> javadoc change nor any javadoc change for that matter. So either the script >> was different, or Martin filtered that line (from Object.java) out before >> creating the webrev. >> >> Now, in his followup thread on core-libs-dev, >> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-September/035273.html, >> Martin specifically pointed out javadoc changes and said that they seem >> fine to him. > > "to each his own". I think static synchronized reads best and more natural > than synchronzied static. Also from a semantic point of view as it conveys > class level characteristic thus lends static to having a prominence in > specified order. Pragmatically, fix the script to ignore those keywords on comment lines. Learn Perl, its just a regular expression pattern match and replace expression. All of the changes have to be manually reviewed by the author and then the reviewers. Checking unneeded changes is part of every mechanical change. The text being changed in the javadoc is the *spec*; that deserves special attention in review. But having seen several reviewers be unmoved by the difference, the real pragmatic view is to ignore the English. - PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8276348: Use blessed modifier order in java.base
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
src/java.base/share/classes/java/lang/Object.java line 282:
> 280: * For objects of type {@code Class,} by executing a
> 281: * static synchronized method of that class.
> 282: *
In comments, I think the 'synchronized static 'reads better, the conventional
order is for the code.
-
PR: https://git.openjdk.java.net/jdk/pull/6213
Re: RFR: 8273261: Replace 'while' cycles with iterator with enhanced-for in java.base
On Wed, 1 Sep 2021 07:37:53 GMT, Andrey Turbanov wrote: > There are few places in code where manual while loop is used with Iterator to > iterate over Collection. > Instead of manual while cycles it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > It doesn't have any performance impact: java compiler generates similar code > when compiling enhanced-for cycle. > > Similar cleanups: > * https://bugs.openjdk.java.net/browse/JDK-8258006 > * https://bugs.openjdk.java.net/browse/JDK-8257912 LGTM For the security related code a second reviewer would be good. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5328
Re: RFR: 8270380: Change the default value of the java.security.manager system property to disallow [v2]
On Tue, 31 Aug 2021 02:08:48 GMT, Weijun Wang wrote: >> This change modifies the default value of the `java.security.manager` system >> property from "allow" to "disallow". This means unless it's explicitly set >> to "allow", any call to `System.setSecurityManager()` would throw an UOE. >> >> The `AllowSecurityManager.java` and `SecurityManagerWarnings.java` tests are >> updated to confirm this behavior change. Two other tests are updated because >> they were added after JDK-8267184 and do not have >> `-Djava.security.manager=allow` on its `@run` line even it they need to >> install a `SecurityManager` at runtime. >> >> Please note that this code change requires jtreg to be upgraded to 6.1, >> where a security manager [will not be >> set](https://bugs.openjdk.java.net/browse/CODETOOLS-7902990). > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > sections etc Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5204
Re: RFR: 8272626: Avoid C-style array declarations in java.*
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. - PR: https://git.openjdk.java.net/jdk/pull/5161
[jdk17] Withdrawn: 8271489: (doc) Clarify Filter Factory example
On Thu, 29 Jul 2021 16:36:21 GMT, Roger Riggs wrote: > Improve the clarity of comments in the ObjectInputFilter FilterInThread > example. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk17/pull/292
[jdk17] RFR: 8271489: (doc) Clarify Filter Factory example
Improve the clarity of comments in the ObjectInputFilter FilterInThread example. - Commit messages: - 8271489: (doc) Clarify Filter Factory example - 8270398: Enhance canonicalization - 8270404: Better canonicalization - Merge - Merge - 8263531: Remove unused buffer int - 8262731: [macOS] Exception from "Printable.print" is swallowed during "PrinterJob.print" - 8269763: The JEditorPane is blank after JDK-8265167 - 8265580: Enhanced style for RTF kit - 8265574: Improve handling of sheets - ... and 15 more: https://git.openjdk.java.net/jdk17/compare/c1304519...650e1561 Changes: https://git.openjdk.java.net/jdk17/pull/292/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk17=292=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8271489 Stats: 1001 lines in 42 files changed: 625 ins; 181 del; 195 mod Patch: https://git.openjdk.java.net/jdk17/pull/292.diff Fetch: git fetch https://git.openjdk.java.net/jdk17 pull/292/head:pull/292 PR: https://git.openjdk.java.net/jdk17/pull/292
Re: RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} for java.base [v11]
On Thu, 8 Jul 2021 03:12:24 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 refreshed the contents of this pull request, and previous commits > have been removed. The incremental views will show differences compared to > the previous content of the PR. The pull request contains one new commit > since the last revision: > > restore JSR166; restore java.desktop Looks good. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4507
Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager should only appear once for each caller
On Tue, 29 Jun 2021 19:35:40 GMT, Weijun Wang wrote: >> Using a HashSet could use the callerClass as the key and be a stable >> reference for having given the message. >> or use a ConcurrentHashMap>, boolean> and avoid any separate >> synchronization that would be needed with a HashSet or HashMap. > > If I switch to a "non-weak" set or map, then it seems I can safely use the > source string as the key. Will using the Class object as a key prevent them > from unloading? Using a synchronized WeakHashMap with the class as the key would not prevent class unloading. Using a non-weak set or map to strings would keep the strings around for the life of the runtime. - PR: https://git.openjdk.java.net/jdk17/pull/166
Re: [jdk17] RFR: 8269543: The warning for System::setSecurityManager should only appear once for each caller
On Tue, 29 Jun 2021 18:47:23 GMT, Daniel Fuchs wrote: >> Add a cache to record which sources have called `System::setSecurityManager` >> and only print out warning lines once for each. > > src/java.base/share/classes/java/lang/System.java line 337: > >> 335: = Collections.synchronizedMap(new WeakHashMap<>()); >> 336: } >> 337: > > I wonder about the use of a WeakHashMap here. That may work well when the > source is an interned string (a class name) which will be strongly referenced > elsewhere and may be garbage collected if the class is unloaded, but in the > case where it contains the name of the source jar then that string will only > be referenced by the weak hashmap, and therefore it could be garbage > collected any time - which would cause the mapping to be removed. In that > case you cannot guarantee that the warning will be emitted only once. Using a HashSet could use the callerClass as the key and be a stable reference for having given the message. or use a ConcurrentHashMap>, boolean> and avoid any separate synchronization that would be needed with a HashSet or HashMap. - PR: https://git.openjdk.java.net/jdk17/pull/166
Re: RFR: 8267123: Remove RMI Activation
On Tue, 25 May 2021 18:04:45 GMT, Stuart Marks wrote: > This is the implementation of [JEP 407](https://openjdk.java.net/jeps/407). > > This is a fairly straightforward removal of this component. > - Activation implementation classes removed > - Activation tests removed > - adjustments to various comments to remove references to Activation > - adjustments to some code to remove special-case support for Activation > from core RMI > - adjustments to several tests to remove testing and support for, and > mentions of Activation > - remove `rmid` tool > > (Personally, I found that reviewing using the webrev was easier than > navigating github's diff viewer.) LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4194
Re: RFR: 8265426: Update java.security to use instanceof pattern variable [v5]
On Thu, 6 May 2021 14:42:20 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the >> `java.security` package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8265426: changed order of equals check; refactored Identity.equals method Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3687
Re: RFR: 8265426: Update java.security to use instanceof pattern variable [v3]
On Thu, 6 May 2021 11:52:15 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the >> `java.security` package to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8265426: Reverted parameter names; removed redundant parenthesis Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3687
Re: RFR: 8265426: Update java.security to use instanceof pattern variable
On Mon, 26 Apr 2021 08:50:36 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the > `java.security` package to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick Marked as reviewed by rriggs (Reviewer). src/java.base/share/classes/java/security/CodeSource.java line 174: > 172: // certs must match > 173: return matchCerts(other, true); > 174: } Can this (160-173) be collapsed to: return (obj instanceof CodeSource cs) && Objects.equals(location, other.location) && matchCerts(cs, true) - PR: https://git.openjdk.java.net/jdk/pull/3687
Re: RFR: 8265890: ProblemList sun/security/ssl/SSLSocketImpl/SSLSocketImplThrowsWrongExceptions.java on macOS-X64 and Linux-aarch64
On Sat, 24 Apr 2021 14:35:32 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList > sun/security/ssl/SSLSocketImpl/SSLSocketImplThrowsWrongExceptions.java > on macOS-X64 and Linux-aarch64. It is already ProblemListed on linux-x64. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3668
Re: RFR: 8264208: Console charset API [v12]
On Thu, 22 Apr 2021 17:38:43 GMT, Naoto Sato wrote: >> Please review the changes for the subject issue. This has been suggested in >> a recent discussion thread for the JEP 400 >> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)]. >> A CSR has also been drafted, and comments are welcome >> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)]. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Revived charset() in JavaIOAccess interface. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8264208: Console charset API [v11]
On Thu, 22 Apr 2021 16:11:16 GMT, Naoto Sato wrote: >> Not always, for example, if stderr was redirected to a terminal but not >> stdin and stdout. >> The istty check is only true if both stdin and stdout are ttys. > > Then `charset()` in the shared secret would return `null`. Would that suffice > your case? I read lines 575-587 as initializing CHARSET regardless of whether the Console was created. - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8264208: Console charset API [v11]
On Thu, 22 Apr 2021 15:42:02 GMT, Naoto Sato wrote: >> src/java.base/share/classes/java/io/Console.java line 597: >> >>> 595: return null; >>> 596: } >>> 597: }); >> >> Please keep the charset() method and return CHARSET. >> >> I'm looking at a use case that needs to know the platform charset regardless >> of whether the console exists. >> When a process is launched it may be redirected to /dev/tty or a pseudo tty >> and in that case >> a Reader from that stream should be able to use the encoding of the platform. >> Its still a work in progress, but it would save some refactoring or >> duplication later. > > Would the singleton `Console.cons` be instantiated in your use case? It is > created only when isatty() (or Windows' equivalent) in the native code > returns true. Not always, for example, if stderr was redirected to a terminal but not stdin and stdout. The istty check is only true if both stdin and stdout are ttys. - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8264208: Console charset API [v11]
On Tue, 20 Apr 2021 22:35:00 GMT, Naoto Sato wrote: >> Please review the changes for the subject issue. This has been suggested in >> a recent discussion thread for the JEP 400 >> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)]. >> A CSR has also been drafted, and comments are welcome >> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)]. > > Naoto Sato 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 13 additional commits since > the last revision: > > - Refined the test case. > - Merge branch 'master' into JDK-8264208 > - Changed shell based test into java based > - Added link to Charset#defaultChaset() in InputStreamReader. > - Modified javadocs per suggestions. > - Added @see links. > - Added Console::charset() relation with System.in > - Added comment to System.out/err init. > - Reflected further review comments. > - Reverted PrintStream changes > - ... and 3 more: > https://git.openjdk.java.net/jdk/compare/72f17eb7...e585d16f src/java.base/share/classes/java/io/Console.java line 597: > 595: return null; > 596: } > 597: }); Please keep the charset() method and return CHARSET. I'm looking at a use case that needs to know the platform charset regardless of whether the console exists. When a process is launched it may be redirected to /dev/tty or a pseudo tty and in that case a Reader from that stream should be able to use the encoding of the platform. Its still a work in progress, but it would save some refactoring or duplication later. - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8264208: Console charset API [v9]
On Thu, 15 Apr 2021 18:29:17 GMT, Naoto Sato wrote: >> Please review the changes for the subject issue. This has been suggested in >> a recent discussion thread for the JEP 400 >> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)]. >> A CSR has also been drafted, and comments are welcome >> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)]. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Modified javadocs per suggestions. For the test, can it be re-written in Java. The direction has been to avoid creating new shell tests as they are fragile. There are test utilities in ProcessTool to make launching and checking for output very easy. src/java.base/share/classes/java/io/InputStreamReader.java line 48: > 46: * For top efficiency, consider wrapping an InputStreamReader within a > 47: * BufferedReader. For example: > 48: * Oddly, none of the reference in this class to the default charset are links to Charset.defaultCharset(). That would be a useful addition, either in the class javadoc or in the 1-arg constructor that uses the default charset. - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib [v3]
On Wed, 14 Apr 2021 22:57:55 GMT, Weijun Wang wrote: >> I'd like to move this tool to test/lib inside a proper named package. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > do not call internal method Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3496
Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib
On Wed, 14 Apr 2021 18:12:57 GMT, Weijun Wang wrote: > I'd like to move this tool to test/lib inside a proper named package. I think is fine to move it first and refactor it separately; that's a bigger job. If its fit for the purpose, the overlap is fine between tools. - PR: https://git.openjdk.java.net/jdk/pull/3496
Re: RFR: 8265227: Move Proc.java from security/testlibrary to test/lib
On Wed, 14 Apr 2021 18:12:57 GMT, Weijun Wang wrote: > I'd like to move this tool to test/lib inside a proper named package. Good idea. Is there a more evocative class name than Proc? It looks similar to jdk.test.lib.process.ProcessTools. Would it fit in the jdk.test.lib.process package? It would make it easier for non-security testers to find and use? ( I like the fluid construction) - PR: https://git.openjdk.java.net/jdk/pull/3496
Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI [v2]
On Mon, 12 Apr 2021 14:09:55 GMT, Conor Cleary wrote: >> ### Description >> This fix is part of a previous effort to both cleanup/modernise JNDI code, >> the details of which can be seen in >> [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number >> JNDI methods under `java.naming` use Anonymous Inner Classes in cases where >> only a single object unique to the requirements of the method is used. The >> issues these occurrences of AICs cause are highlighted below. >> >> - AICs, in the cases concerned with this fix, are used where only one >> operation is required. While AICs can be useful for more complex >> implementations (using interfaces, multiple methods needed, local fields >> etc.), Lambdas are better suited here as they result in a more readable and >> concise solution. >> >> ### Fixes >> - Where applicable, occurrences of AICs were replaced with lambdas to >> address the issues above resulting primarily in more readable/concise code. > > Conor Cleary has updated the pull request incrementally with two additional > commits since the last revision: > > - Update copyright headers > - Tidied up lambdas Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3416
Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI
On Fri, 9 Apr 2021 15:47:35 GMT, Conor Cleary wrote: >> src/java.naming/share/classes/com/sun/jndi/ldap/LdapPoolManager.java line >> 413: >> >>> 411: return AccessController.doPrivileged( >>> 412: (PrivilegedAction) () -> Long.getLong(propName, >>> defVal).longValue() >>> 413: ); >> >> And GetIntegerAction here. Though it only supports an int value. > > Thanks for the suggestion Roger, I think the `privilegedGetProperty(prop, > default)` for the `getProperty()` method looks great. > > WRT to using it for `getInt()` and `getLong()`, I think its reasonable to use > other means for these methods in the interest of consistency due to, as you > pointed out, only `int` being supported. Would you think? Or would it be > better to use the same means in all 3 methods? Its a slippery slope that might require a bit more investigation to check the expected value sizes to see if a change to the number of bits in the value for each property might break something. Technical debt can take you down a rabbit hole. Quickest to just convert to the lamba as you proposed. - PR: https://git.openjdk.java.net/jdk/pull/3416
Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI
On Fri, 9 Apr 2021 13:51:30 GMT, Conor Cleary wrote: >> An alternative here would be to use >> sun.security.action.privilegedGetProperty(prop, default). >> The package is already exported from java.base to java.desktop, etc. > > That is a very neat alternative yes. Approaching the problem like that > especially improves the readability > [JdkLDAP.java](https://github.com/openjdk/jdk/pull/3416/files#diff-bf4c67da93cf2b9196508db2d57f7e01bc884f2268f5bfd43a9f01dfd55e4955) > in my opinion. > I don't think casting has any major performance hits for these fixes, so is > your suggestion mostly for readability's sake? Right, not a performance problem. Just simpler to use an existing method to read a privileged property. And there will be one less doPriv. - PR: https://git.openjdk.java.net/jdk/pull/3416
Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI
On Fri, 9 Apr 2021 13:15:16 GMT, Conor Cleary wrote: > ### Description > This fix is part of a previous effort to both cleanup/modernise JNDI code, > the details of which can be seen in > [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number > JNDI methods under `java.naming` use Anonymous Inner Classes in cases where > only a single object unique to the requirements of the method is used. The > issues these occurrences of AICs cause are highlighted below. > > - AICs, in the cases concerned with this fix, are used where only one > operation is required. While AICs can be useful for more complex > implementations (using interfaces, multiple methods needed, local fields > etc.), Lambdas are better suited here as they result in a more readable and > concise solution. > > ### Fixes > - Where applicable, occurrences of AICs were replaced with lambdas to address > the issues above resulting primarily in more readable/concise code. Looks good. A general reminder about one important difference between an anonymous inner class and a lambda. An anonymous has *identity* but a lambda does not; or only accidentally. None of the uses here require identity. src/java.naming/share/classes/com/sun/jndi/ldap/LdapPoolManager.java line 413: > 411: return AccessController.doPrivileged( > 412: (PrivilegedAction) () -> Long.getLong(propName, > defVal).longValue() > 413: ); And GetIntegerAction here. Though it only supports an int value. - PR: https://git.openjdk.java.net/jdk/pull/3416
Re: RFR: 8048199: Replace anonymous inner classes with lambdas, where applicable, in JNDI
On Fri, 9 Apr 2021 13:30:27 GMT, Alan Bateman wrote: >> ### Description >> This fix is part of a previous effort to both cleanup/modernise JNDI code, >> the details of which can be seen in >> [JDK-8048091](https://bugs.openjdk.java.net/browse/JDK-8048091). A number >> JNDI methods under `java.naming` use Anonymous Inner Classes in cases where >> only a single object unique to the requirements of the method is used. The >> issues these occurrences of AICs cause are highlighted below. >> >> - AICs, in the cases concerned with this fix, are used where only one >> operation is required. While AICs can be useful for more complex >> implementations (using interfaces, multiple methods needed, local fields >> etc.), Lambdas are better suited here as they result in a more readable and >> concise solution. >> >> ### Fixes >> - Where applicable, occurrences of AICs were replaced with lambdas to >> address the issues above resulting primarily in more readable/concise code. > > src/java.naming/share/classes/com/sun/jndi/ldap/LdapPoolManager.java line 401: > >> 399: return AccessController.doPrivileged( >> 400: (PrivilegedAction) () -> >> System.getProperty(propName, defVal) >> 401: ); > > If you want to avoid the cast then you could create the PrivilegedAction > explicitly, e.g. > > PrivilegedAction pa = () -> System.getProperty(propName, defVal); > return AccessController.doPrivileged(pa); An alternative here would be to use sun.security.action.privilegedGetProperty(prop, default). The package is already exported from java.base to java.desktop, etc. - PR: https://git.openjdk.java.net/jdk/pull/3416
Re: Integrated: 8264656: ProblemList sun/security/ssl/SSLSocketImpl/SSLSocketImplThrowsWrongExceptions.java on linux-x64
On Fri, 2 Apr 2021 18:04:26 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList > sun/security/ssl/SSLSocketImpl/SSLSocketImplThrowsWrongExceptions.java > on linux-x64 Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3328
Re: RFR: 8264148: Update spec for exceptions retrofitted for exception chaining
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`
-
Marked as reviewed by rriggs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/3182
Integrated: 8263754: HexFormat 'fromHex' methods should be static
On Thu, 25 Mar 2021 20:08:14 GMT, Roger Riggs wrote: > A number of HexFormat methods converting from strings to numbers do not use > delimiter, prefix, suffix, and uppercase parameters and would be more > convenient if the methods were static. > > These APIs were added early in JDK 17 and are being updated before GA. > This PR updates existing uses in the JDK but there may be compiler warnings > in non-JDK source files. > >public boolean isHexDigit(int); >public int fromHexDigit(int); >public int fromHexDigits(java.lang.CharSequence); >public int fromHexDigits(java.lang.CharSequence, int, int); >public long fromHexDigitsToLong(java.lang.CharSequence); >public long fromHexDigitsToLong(java.lang.CharSequence, int, int); This pull request has now been integrated. Changeset: 8cf1c62c Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/8cf1c62c Stats: 54 lines in 5 files changed: 0 ins; 15 del; 39 mod 8263754: HexFormat 'fromHex' methods should be static Reviewed-by: redestad, naoto, chegar - PR: https://git.openjdk.java.net/jdk/pull/3205
Re: RFR: 8263754: HexFormat 'fromHex' methods should be static [v2]
> A number of HexFormat methods converting from strings to numbers do not use > delimiter, prefix, suffix, and uppercase parameters and would be more > convenient if the methods were static. > > These APIs were added early in JDK 17 and are being updated before GA. > This PR updates existing uses in the JDK but there may be compiler warnings > in non-JDK source files. > >public boolean isHexDigit(int); >public int fromHexDigit(int); >public int fromHexDigits(java.lang.CharSequence); >public int fromHexDigits(java.lang.CharSequence, int, int); >public long fromHexDigitsToLong(java.lang.CharSequence); >public long fromHexDigitsToLong(java.lang.CharSequence, int, int); Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Remove unused HexFormat instances - Changes: - all: https://git.openjdk.java.net/jdk/pull/3205/files - new: https://git.openjdk.java.net/jdk/pull/3205/files/050495a9..042dc18b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=3205=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3205=00-01 Stats: 7 lines in 4 files changed: 0 ins; 7 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3205.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3205/head:pull/3205 PR: https://git.openjdk.java.net/jdk/pull/3205
Re: RFR: 8263754: HexFormat 'fromHex' methods should be static
On Thu, 25 Mar 2021 21:05:47 GMT, Claes Redestad wrote:
>> A number of HexFormat methods converting from strings to numbers do not use
>> delimiter, prefix, suffix, and uppercase parameters and would be more
>> convenient if the methods were static.
>>
>> These APIs were added early in JDK 17 and are being updated before GA.
>> This PR updates existing uses in the JDK but there may be compiler warnings
>> in non-JDK source files.
>>
>>public boolean isHexDigit(int);
>>public int fromHexDigit(int);
>>public int fromHexDigits(java.lang.CharSequence);
>>public int fromHexDigits(java.lang.CharSequence, int, int);
>>public long fromHexDigitsToLong(java.lang.CharSequence);
>>public long fromHexDigitsToLong(java.lang.CharSequence, int, int);
>
> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 4582:
>
>> 4580: int pos = 0;
>> 4581: for (char c: value.toCharArray()) {
>> 4582: if (!HexFormat.isHexDigit(c)) {
>
> `hexFmt` created on line 4576 appears to be unused after this.
The toHexDigit methods need access to the uppercase/lowercase distinction.
So there is no plan to change them.
I'll amend to include the above and re-check for uncaught uses.
-
PR: https://git.openjdk.java.net/jdk/pull/3205
RFR: 8263754: HexFormat 'fromHex' methods should be static
A number of HexFormat methods converting from strings to numbers do not use delimiter, prefix, suffix, and uppercase parameters and would be more convenient if the methods were static. These APIs were added early in JDK 17 and are being updated before GA. This PR updates existing uses in the JDK but there may be compiler warnings in non-JDK source files. public boolean isHexDigit(int); public int fromHexDigit(int); public int fromHexDigits(java.lang.CharSequence); public int fromHexDigits(java.lang.CharSequence, int, int); public long fromHexDigitsToLong(java.lang.CharSequence); public long fromHexDigitsToLong(java.lang.CharSequence, int, int); - Commit messages: - 8263754: HexFormat 'fromHex' methods should be static Changes: https://git.openjdk.java.net/jdk/pull/3205/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3205=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263754 Stats: 47 lines in 5 files changed: 0 ins; 8 del; 39 mod Patch: https://git.openjdk.java.net/jdk/pull/3205.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3205/head:pull/3205 PR: https://git.openjdk.java.net/jdk/pull/3205
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Sat, 13 Mar 2021 22:45:30 GMT, Alex Blewitt wrote: > Sonar displays a warning message that modifiers should be declared in the > order listed in the JLS; specifically, that isntead of using `final static` > the `static final` should be preferred. > > This fixes the issues in the `java.base` package for ease of reviewing. > > https://sonarcloud.io/project/issues?id=shipilev_jdk=java=false=java%3AS1124 Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8252833: Correct "no comment" warnings from javadoc in java.smartcardio module
On Thu, 11 Mar 2021 01:13:12 GMT, Bradford Wetmore wrote: > Disable the "missing" target for java.smartcardio from doclint. Please assign a new bug and title to ignore/suppress the warnings. The original issue 8252833 should be left open. Thanks - Changes requested by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2930
Re: RFR: 8263105: security-libs doclint cleanup [v2]
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 src/java.base/share/classes/javax/crypto/SealedObject.java line 425: > 423: * Restores the state of the SealedObject from a stream. > 424: * > 425: * @param s the object input stream. Covert to @\throws but keep this declaration. There is no spec that otherwise covers this and it is consistent with other methods. - PR: https://git.openjdk.java.net/jdk/pull/2856
Re: RFR: 8263105: security-libs doclint cleanup [v3]
On Tue, 9 Mar 2021 22:19:28 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: > > More Codereview Comments src/java.base/share/classes/java/security/AllPermission.java line 165: > 163: > 164: /** > 165: * true if any AllPermissions have been added Please capitalize to make it a sentence, and end in ".". src/java.base/share/classes/java/security/BasicPermission.java line 261: > 259: > 260: /** > 261: * readObject is called to restore the state of the BasicPermission > from Please capitalize... The readObject method... src/java.base/share/classes/java/security/BasicPermission.java line 526: > 524: > 525: /** > 526: * readObject is called to restore the state of the Ditto, capitalize. src/java.base/share/classes/java/security/Permissions.java line 579: > 577: }; > 578: > 579: /* The @ serialData comment can be removed entirely. It is unused due to the "@serial exclude" above. src/java.base/share/classes/java/security/UnresolvedPermissionCollection.java line 158: > 156: > 157: /* > 158: * @serialData Default field. the comment block containing @\serialdata can be omitted entirely. - PR: https://git.openjdk.java.net/jdk/pull/2856
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v25]
On Mon, 1 Mar 2021 13:23:48 GMT, Jim Laskey wrote:
>> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java
>> line 232:
>>
>>> 230: Provider provider = fm.get(name);
>>> 231: if (!isSubclass(category, provider)) {
>>> 232: throw new IllegalArgumentException(name + " is an unknown
>>> random number generator");
>>
>> The message is a bit vague. How about:
>>
>> "The random number generator does not support the category"
>
> throw new IllegalArgumentException("The random number generator "" + name +
> "" can not be located");
The message only captures the failure if the result of `fm.get()` is null.
It does not capture the failure if the name is found but does not support the
category.
-
PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v3]
On Wed, 25 Nov 2020 16:22:32 GMT, Jim Laskey wrote: >> src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java >> line 106: >> >>> 104: * Map of provider classes. >>> 105: */ >>> 106: private static volatile Map>> RandomGenerator>> factoryMap; >> >> should be FACTORY_MAP given that it's a static field > > will fall out of using class holder idiom IntelliJ warns that this volatile field is unused. It has been replaced by the method getFactoryMap(). - PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v25]
On Tue, 23 Feb 2021 16:47:59 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 57 commits:
>
> - Merge branch 'master' into 8248862
> - Adjust ThreadLocalRandom javadoc inheritence
> - L32X64StarStarRandom -> L32X64MixRandom
> - Various corrects
> - Revised javadoc per CSR reviews
> - Remove tabs from random/package-info.java
> - Correct copyright notice.
> - Merge branch 'master' into 8248862
> - Update tests for RandomGeneratorFactory.all()
> - Merge branch 'master' into 8248862
> - ... and 47 more:
> https://git.openjdk.java.net/jdk/compare/0257caad...b9094279
src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line
232:
> 230: Provider provider = fm.get(name);
> 231: if (!isSubclass(category, provider)) {
> 232: throw new IllegalArgumentException(name + " is an unknown
> random number generator");
The message is a bit vague. How about:
"The random number generator does not support the category"
src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line
240:
> 238: * Returns a {@link RandomGenerator} that utilizes the {@code name}
> 239: * algorithm.
> 240: *
In javadoc, this seems like is should read as: "utilizes the named algorithm".
The use of the parameter name is unnecessary in this case because it is obvious
and readability suffers as is.
src/java.base/share/classes/java/util/random/RandomGenerator.java line 1313:
> 1311: * furthermore can easily jump to an arbitrarily specified
> distant
> 1312: * point in the state cycle.
> 1313: *
The similarity of the first sentence of each of the Jumpable, Leapable, and
arbitrarlyJumpable interface is so similar as to obscure the differences. You
have to read 25 words in to be able to find the description that makes them
different. The italics help but should include the whole of the phrase that
distinguishes them.
Alternatively, move the phrase to the beginning of the sentence or drop the
redundant phrasing.
"provide a common protocol of objects that generate pseudorandom sequences of
numbers of Boolean values", etc.
src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line
55:
> 53: *
> 54: * A specific {@link RandomGeneratorFactory} can be located by using the
> 55: * {@link RandomGenerator#factoryOf(String)} method, where the argument
> string
Broken link: the method is in this class. should be just "#factoryOf".
src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line
600:
> 598: try {
> 599: ensureConstructors();
> 600: return ctorBytes.newInstance((Object)seed);
IntelliJ warns that the cast to (Object) is redundant.
src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line
29:
> 27:
> 28: import java.lang.reflect.Constructor;
> 29: import java.lang.reflect.Method;
Used import.
-
PR: https://git.openjdk.java.net/jdk/pull/1292
Re: RFR: 8261160: Add a deserialization JFR event [v5]
On Fri, 12 Feb 2021 16:26:09 GMT, Chris Hegarty wrote: >> This issue adds a new event to improve diagnostic information of Java >> deserialization. The event captures the details of deserialization activity >> from ObjectInputStream. The event details are similar to that of the serial >> filter, but is agnostic of whether a filter is installed or not. The event >> also captures the filter status, if there is one. > > Chris Hegarty has updated the pull request incrementally with one additional > commit since the last revision: > > Split exception into type and message Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2479
Re: RFR: 8261160: Add a deserialization JFR event [v4]
On Thu, 11 Feb 2021 15:53:06 GMT, Chris Hegarty wrote: >> This issue adds a new event to improve diagnostic information of Java >> deserialization. The event captures the details of deserialization activity >> from ObjectInputStream. The event details are similar to that of the serial >> filter, but is agnostic of whether a filter is installed or not. The event >> also captures the filter status, if there is one. > > Chris Hegarty has updated the pull request incrementally with one additional > commit since the last revision: > > Filter **C**onfigured Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2479
Re: RFR: 8261160: Add a deserialization JFR event
On Wed, 10 Feb 2021 20:30:02 GMT, Roger Riggs wrote: >> This issue adds a new event to improve diagnostic information of Java >> deserialization. The event captures the details of deserialization activity >> from ObjectInputStream. The event details are similar to that of the serial >> filter, but is agnostic of whether a filter is installed or not. The event >> also captures the filter status, if there is one. > > Marked as reviewed by rriggs (Reviewer). As proposed, events are only created if there is a serialFilter in effect (and enabled by JFR configuration). Being able to create the events without a serialFilter in effect would be useful for monitoring, especially if it could be controlled by a separate JFR configuration option. (always, never, serial-filter , etc.) - PR: https://git.openjdk.java.net/jdk/pull/2479
Re: RFR: 8261160: Add a deserialization JFR event
On Tue, 9 Feb 2021 12:35:27 GMT, Chris Hegarty wrote: > This issue adds a new event to improve diagnostic information of Java > deserialization. The event captures the details of deserialization activity > from ObjectInputStream. The event details are similar to that of the serial > filter, but is agnostic of whether a filter is installed or not. The event > also captures the filter status, if there is one. Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2479
Re: RFR: 8217633: Configurable extensions with system properties
On Fri, 11 Dec 2020 23:20:34 GMT, Xue-Lei Andrew Fan wrote: > The TLS protocols are designed to tolerant unknown TLS extensions. However, > although it is not common, there are a few TLS implementations that cannot > handle unknown extensions properly. As results in unexpected interoperability > issue when new extensions are introduced in JDK. The interoperability impact > could be mitigated If applications can customize the extensions if needed. > > With this update, two system properties are added to configure the default > extensions in either client or server side of TLS connections. Please note > that the impact of blocking TLS extensions is complicated. For example, a > TLS connection may not be able to established if a mandatory extension is > blocked. Please don't use this feature unless you clearly understand the > impact. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8217633 > CSR: https://bugs.openjdk.java.net/browse/JDK-8217993 Where will the new properties be documented? In the CSR, it may be worth mentioning that the extension names are case sensitive. - PR: https://git.openjdk.java.net/jdk/pull/1752
Integrated: 8258796: [test] Apply HexFormat to tests for java.security
On Fri, 8 Jan 2021 15:43:45 GMT, Roger Riggs wrote: > Followup to the addition of java.util.HexFormat. > Uses of adhoc hexadecimal conversions are replaced with HexFormat. > Redundant utility methods are modified or removed. > In a few places the data being printed is ASN.1 and the test utility > HexPrinter with the ASNFormatter is added. This pull request has now been integrated. Changeset: 628c546b Author:Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/628c546b Stats: 758 lines in 43 files changed: 81 ins; 466 del; 211 mod 8258796: [test] Apply HexFormat to tests for java.security Reviewed-by: xuelei - PR: https://git.openjdk.java.net/jdk/pull/2006
Re: RFR: 8258796: [test] Apply HexFormat to tests for java.security [v2]
> Followup to the addition of java.util.HexFormat. > Uses of adhoc hexadecimal conversions are replaced with HexFormat. > Redundant utility methods are modified or removed. > In a few places the data being printed is ASN.1 and the test utility > HexPrinter with the ASNFormatter is added. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Update copyrights to 2021 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2006/files - new: https://git.openjdk.java.net/jdk/pull/2006/files/c437a74b..b6908530 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2006=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2006=00-01 Stats: 43 lines in 43 files changed: 0 ins; 0 del; 43 mod Patch: https://git.openjdk.java.net/jdk/pull/2006.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2006/head:pull/2006 PR: https://git.openjdk.java.net/jdk/pull/2006
Re: RFR: 8258796: [test] Apply HexFormat to tests for java.security
On Fri, 8 Jan 2021 15:43:45 GMT, Roger Riggs wrote: > Followup to the addition of java.util.HexFormat. > Uses of adhoc hexadecimal conversions are replaced with HexFormat. > Redundant utility methods are modified or removed. > In a few places the data being printed is ASN.1 and the test utility > HexPrinter with the ASNFormatter is added. Removing the hotspot-runtime label to avoid spamming that alias. Add it back if it is warranted. - PR: https://git.openjdk.java.net/jdk/pull/2006
RFR: 8258796: [test] Apply HexFormat to tests for java.security
Followup to the addition of java.util.HexFormat. Uses of adhoc hexadecimal conversions are replaced with HexFormat. Redundant utility methods are modified or removed. In a few places the data being printed is ASN.1 and the test utility HexPrinter with the ASNFormatter is added. - Commit messages: - 8258796: [test] Apply HexFormat to tests for java.security Changes: https://git.openjdk.java.net/jdk/pull/2006/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2006=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8258796 Stats: 738 lines in 43 files changed: 81 ins; 466 del; 191 mod Patch: https://git.openjdk.java.net/jdk/pull/2006.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2006/head:pull/2006 PR: https://git.openjdk.java.net/jdk/pull/2006
Re: RFR: 8259021 avoid double racy reads from non-volatile fields of SharedSecrets [v2]
On Mon, 4 Jan 2021 14:27:09 GMT, Peter Levart wrote: >> See: https://bugs.openjdk.java.net/browse/JDK-8259021 >> See also discussion in thread: >> https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-December/072798.html > > Peter Levart has updated the pull request incrementally with one additional > commit since the last revision: > > revert the unrelated change The bug title and the PR title need to be the same. Editing either one is fine. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1914
Integrated: 8252055: Use java.util.HexFormat in java.security
On Thu, 17 Dec 2020 20:45:57 GMT, Roger Riggs wrote: > The java.util.HexFormat methods for formatting and parsing replace a number > of adhoc hex parsing and formatting methods in sun.security implementation > classes. This pull request has now been integrated. Changeset: 68f2acbf Author: Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/68f2acbf Stats: 254 lines in 15 files changed: 13 ins; 183 del; 58 mod 8252055: Use java.util.HexFormat in java.security Reviewed-by: xuelei - PR: https://git.openjdk.java.net/jdk/pull/1828
Re: RFR: 8252055: Use java.util.HexFormat in java.security [v2]
> The java.util.HexFormat methods for formatting and parsing replace a number > of adhoc hex parsing and formatting methods in sun.security implementation > classes. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Update copyrights to 2020 - Changes: - all: https://git.openjdk.java.net/jdk/pull/1828/files - new: https://git.openjdk.java.net/jdk/pull/1828/files/40888c41..7c125a88 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=1828=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1828=00-01 Stats: 7 lines in 7 files changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/1828.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1828/head:pull/1828 PR: https://git.openjdk.java.net/jdk/pull/1828
RFR: 8250255: Use java.util.HexFormat in java.security
The java.util.HexFormat methods for formatting and parsing replace a number of adhoc hex parsing and formatting methods in sun.security implementation classes. - Commit messages: - 8250255: Use java.util.HexFormat in java.security Changes: https://git.openjdk.java.net/jdk/pull/1828/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1828=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8250255 Stats: 247 lines in 14 files changed: 13 ins; 183 del; 51 mod Patch: https://git.openjdk.java.net/jdk/pull/1828.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1828/head:pull/1828 PR: https://git.openjdk.java.net/jdk/pull/1828
RFR: 8251989: Hex formatting and parsing utility
java.util.HexFormat utility: - Format and parse hexadecimal strings, with parameters for delimiter, prefix, suffix and upper/lowercase - Static factories and builder methods to create HexFormat copies with modified parameters. - Consistent naming of methods for conversion of byte arrays to formatted strings and back: formatHex and parseHex - Consistent naming of methods for conversion of primitive types: toHexDigits... and fromHexDigits... - Prefix and suffixes now apply to each formatted value, not the string as a whole - Using java.util.Appendable as a target for buffered conversions so output to Writers and PrintStreams like System.out are supported in addition to StringBuilder. (IOExceptions are converted to unchecked exceptions) - Immutable and thread safe, a "value-based" class See the [HexFormat javadoc](http://cr.openjdk.java.net/~rriggs/8251989-hex-formatter/java.base/java/util/HexFormat.html) for details. Review comments and suggestions welcome. - Commit messages: - 8251989: Hex formatting and parsing utility Changes: https://git.openjdk.java.net/jdk/pull/482/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=482=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8251989 Stats: 1665 lines in 12 files changed: 1503 ins; 144 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/482.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/482/head:pull/482 PR: https://git.openjdk.java.net/jdk/pull/482
Integrated: 8252523: Add ASN.1 Formatter to work with test utility HexPrinter
On Sun, 20 Sep 2020 13:54:02 GMT, Roger Riggs wrote: > # JDK-8252523: Add ASN.1 Formatter to work with test utility HexPrinter > > Debugging functions that utilize ASN.1, DER, and BER encoded streams is > difficult without test utilities to show the contents. > The ASN.1 formatter reads a stream and produces annotated output of the > tags, values, and structures. > When used with the test library jdk.test.lib.hexdump.HexPrinter the > annotations are synchronized > with the hex formatted output. > > Small changes to HexPrinter are included to improve the output readability. > > > Example decoding of a .pem certificate: > SEQUENCE [910] > SEQUENCE [630] > CONTEXT cons 0 [3] > BYTE 2, > BYTE 3, > SEQUENCE [13] > OBJECT ID [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > NULL > SEQUENCE [76] > SET [11] > SEQUENCE [9] > OBJECT ID [3] 2.5.4.6 (CountryName) > 'IN' > ... > SET [16] > SEQUENCE [14] > OBJECT ID [3] 2.5.4.3 (CommonName) > Client1 > SEQUENCE [30] > UTCTIME [13] '150526221718Z' > UTCTIME [13] '250523221718Z' > ... > SEQUENCE [290] > SEQUENCE [13] > OBJECT ID [9] 1.2.840.113549.1.1.1 (RSA) > NULL > BIT STRING [271] > CONTEXT cons 3 [123] > SEQUENCE [121] > SEQUENCE [9] > OBJECT ID [3] 2.5.29.19 (BasicConstraints) > OCTET STRING [2] > SEQUENCE [44] > OBJECT ID [9] 2.16.840.1.113730.1.13 > OCTET STRING [31] '..OpenSSL Generated Certificate' > SEQUENCE [29] > OBJECT ID [3] 2.5.29.14 (SubjectKeyID) > OCTET STRING [22] > SEQUENCE [31] > OBJECT ID [3] 2.5.29.35 (AuthorityKeyID) > OCTET STRING [24] > SEQUENCE [13] > OBJECT ID [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > NULL > BIT STRING [257] > When used with the HexPrinter test utility, the formatting of the > hexadecimal values is selected with the parameters to HexPrinter. > > : 30 82 03 8e ; SEQUENCE [910] > 0004: 30 82 02 76 ; SEQUENCE [630] > 0008: a0 03 ; CONTEXT cons > 0 [3] > 000a: 02 01 02 ; BYTE 2, > 000d:02 01 03 ; BYTE 3, > 0010: 30 0d ; SEQUENCE [13] > 0012: 06 09 2a 86 48 86 f7 0d 01 01 0b ; OBJECT ID > [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > 001d:05 00; NULL > 001f: 30 ; SEQUENCE [76] > 0020: 4c ; > 0021:31 0b; SET [11] > 0023: 30 09 ; SEQUENCE > [9] > 0025:06 03 55 04 06 ; OBJECT > ID [3] 2.5.4.6 (CountryName) > 002a: 13 02 49 4e ; 'IN' > > ... ... > > 005b: 31 10 ; SET [16] > 005d:30 0e; SEQUENCE > [14] > 005f: 06 ; OBJECT > ID [3] 2.5.4.3 (CommonName) > 0060: 03 55 04 03 ; > 0064: 0c 07 43 6c 69 65 6e 74 31 ; Client1 > 006d:30 1e; SEQUENCE [30] > 006f: 17 ; UTCTIME > [13] '150526221718Z' > 0070: 0d 31 35 30 35 32 36 32 32 31 37 31 38 5a ; > 007e: 17 0d ; UTCTIME > [13] '250523221718Z' > 0080: 32 35 30 35 32 33 32 32 31 37 31 38 5a ; > > ... ... > > 00db: 30 82 01 22; SEQUENCE [290] > 00df: 30 ; SEQUENCE > [13] > 00e0: 0d ; > 00e1:06 09 2a 86 48 86 f7 0d 01 01 01 ; OBJECT ID > [9] 1.2.840.113549.1.1.1 (RSA) >
Re: RFR: 8252523: Add ASN1 Formatter to work with HexPrinter [v2]
On Tue, 29 Sep 2020 19:35:42 GMT, Jamil Nimeh wrote: > Regarding the end-of-content identifier, that looks good. Thanks for fixing > the indentation for the right-side ASN.1 > interpretation of the bytes. My only remaining question is whether the > corresponding hex dumps on the left should match > the indentation levels as well. I don't have a strong opinion either way on > that one but if you're indenting for each > element at the same nest level it seems like that could potentially chew up a > lot of horizontal space. Was the extra > indentation for the second octet string done for readability? Max had requested the current offset of the byte values, so it was easy to see where each new value started and to keep the offsets on a modulo boundary. The formatter on the right is largely decoupled from the hex value tabular form on the left while keeping the correspondence between the formatted items and the bytes. - PR: https://git.openjdk.java.net/jdk/pull/268
Re: RFR: 8252523: Add ASN1 Formatter to work with HexPrinter [v2]
On Tue, 29 Sep 2020 15:20:13 GMT, Jamil Nimeh wrote: > Also in that last example, it seems to suggest that the second octet string > is nested within the first one since it > sits at a second indent layer. They are both primitives completely covered by > their two byte values so shouldn't they > sit at the same indentation level? Or is the indentation not there to suggest > nested substructures and is more for > separation between elements? Or is this what you mean by "lost an indent"? > Also, should the end of content be at the > same indentation level as the initial indefinite length encoding? Yes, all of the enclosed items should be at the same indent level. (A bug as it turns out). I chose to indent the END-OF-CONTENT line at the same level to terminate the list of tag-values at that level All of the items enclosed are at the same level. The updated output is: : 24 80 ; UNIVERSAL CONSTRUCTED OCTET STRING [INDEFINITE] 0002: 04 02 61 62 ; OCTET STRING [2] 'ab' 0006: 04 02 63 64 ; OCTET STRING [2] 'cd' 000a: 00 00 ; END-OF-CONTENT - PR: https://git.openjdk.java.net/jdk/pull/268
Re: RFR: 8252523: Add ASN1 Formatter to work with HexPrinter [v5]
L > 00ee: 03 82 ; BIT STRING > [271] > 00f0: 01 0f 00 30 82 01 0a 02 82 01 01 00 d8 70 03 54 ; > > ... > > 01f0: 0a 2d f5 de 59 3e d9 5e 74 93 d2 45 02 03 01 00 ; > 0200: 01 ; > 0201:a3 7b; CONTEXT > cons 3 [123] > 0203: 30 79 ; SEQUENCE > [121] > 0205:30 09; > SEQUENCE [9] > 0207: 06 03 55 1d 13 ; > OBJECT ID [3] 2.5.29.19 (BasicConstraints) > 020c: 04 02 30 00 ; OCTET > STRING [2] > 0210: 30 2c ; > SEQUENCE [44] > 0212: 06 09 60 86 48 01 86 f8 42 01 0d ; > OBJECT ID [9] 2.16.840.1.113730.1.13 > 021d:04 1f 16 ; OCTET > STRING [31] '..OpenSSL Generated > Certificate' 0220: 1d 4f 70 65 6e 53 53 4c 20 47 65 6e 65 72 61 74 ; > 0230: 65 64 20 43 65 72 74 69 66 69 63 61 74 65 ; > 023e: 30 1d ; > SEQUENCE [29] > 0240: 06 03 55 1d 0e ; > OBJECT ID [3] 2.5.29.14 (SubjectKeyID) > 0245:04 16 04 14 87 13 66 bc 7a 4d 8e ; OCTET > STRING [22] > 0250: 98 e7 97 fb cc 56 41 27 c8 5e 4c b2 4d ; > 025d:30 1f; > SEQUENCE [31] > 025f: 06 ; > OBJECT ID [3] 2.5.29.35 (AuthorityKeyID) > 0260: 03 55 1d 23 ; > 0264: 04 18 30 16 80 14 1f 21 4f db 10 31 ; OCTET > STRING [24] > 0270: d0 67 83 09 03 d3 cd fc 46 ec cf 1d 8b b4 ; > 027e: 30 0d ; SEQUENCE > [13] > 0280: 06 09 2a 86 48 86 f7 0d 01 01 0b; OBJECT ID > [9] 1.2.840.113549.1.1.11 (SHA256withRSA) > 028b: 05 00 ; NULL > 028d:03 82 01 ; BIT STRING > [257] > 0290: 01 00 3e 2b 5f 32 aa f0 f7 52 2b ba f3 bb 07 ee ; > > ... > > 0390: 6d 94 ; Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Corrected reading of extended length bytes, depth of indefinite length tag-values, added test for indefinite length - Changes: - all: https://git.openjdk.java.net/jdk/pull/268/files - new: https://git.openjdk.java.net/jdk/pull/268/files/25115595..91741d4a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=268=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=268=03-04 Stats: 21 lines in 2 files changed: 18 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/268.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/268/head:pull/268 PR: https://git.openjdk.java.net/jdk/pull/268
