Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v3]
On Thu, 12 May 2022 14:14:38 GMT, Weijun Wang wrote:
>> Сергей Цыпанов has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8282662: Revert dubious changes in MethodType
>
> src/java.base/share/classes/sun/security/validator/EndEntityChecker.java line
> 119:
>
>> 117: // TLS key exchange algorithms requiring keyEncipherment key usage
>> 118: private static final Collection KU_SERVER_ENCRYPTION =
>> 119: Arrays.asList("RSA");
>
> I understand that you haven't modified the `KU_SERVER_KEY_AGREEMENT` on line
> 122 because it has 4 elements. However, these 2 nearby lines using different
> styles look a little strange to me. Why restrict the number to 0, 1, and 2?
> If we have defined methods with up to 9 arguments, does that mean the new
> type is suitable for 9 elements?
The reason is that for the cases of length 0, 1, 2 we don't create underlying
array as we do for larger number of elements. In other words when we replace
`Arrays.asList()` with `List.of()` we win only for 0, 1 and 2 elements.
-
PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Fri, 13 May 2022 04:41:03 GMT, ExE Boss wrote: >> 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. > > 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. - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Wed, 11 May 2022 16:30:41 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). > > 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. 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. - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 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 Reviewed. I also added `noreg-trivial` labels to the bug reports. - PR: https://git.openjdk.java.net/jdk/pull/8683
Re: RFR: 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 Marked as reviewed by smarks (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8683
Re: RFR: 8002277: Refactor two PBE classes to simplify maintenance [v8]
> This change refactors the PBES2Core and PKCS12PBECipherCore classes in SunJCE > provider as requested in the bug record. Functionality should remain the same > with a clearer and simplified code/control flow with less lines of code. > This should improve readability and maintenance. I enhanced one existing > regression test to test more scenarios. This test would pass before the > proposed change and continues to pass with the proposed changes. Valerie Peng has updated the pull request incrementally with one additional commit since the last revision: reset ivSpec and minor nit fix. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8521/files - new: https://git.openjdk.java.net/jdk/pull/8521/files/c1893f8b..95dfa948 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8521&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8521&range=06-07 Stats: 8 lines in 1 file changed: 1 ins; 2 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/8521.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8521/head:pull/8521 PR: https://git.openjdk.java.net/jdk/pull/8521
Re: RFR: 8002277: Refactor two PBE classes to simplify maintenance [v7]
On Thu, 12 May 2022 21:31:39 GMT, Valerie Peng wrote:
>> src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java line 244:
>>
>>> 242: iCount = DEFAULT_COUNT;
>>> 243: }
>>> 244: //if (ivSpec == null) { // old behavior always generate
>>
>> How could `ivSpec` be non-null here? IIUC the only answer is from a previous
>> `engineInit`, and it should not be retained. I suggest removing this check
>> plus adding an explicit `ivSpec = null` at the beginning of this method
>> along with `iCount` and `salt`. Those are the only 3 non final instance
>> fields.
>
> Yeah, it's also possible that ivSpec is non-null if getParameters() is called
> before init(). Now that salt and iCount is reset in the beginning of init(),
> ivSpec should be reset too.
Oh, I didn't realize that. So now whenever `init()` is called every old param
(no matter why it was set) is totally wiped. I like this consistent behavior.
-
PR: https://git.openjdk.java.net/jdk/pull/8521
Re: RFR: 8283577: SSLEngine.unwrap on read-only input ByteBuffer
On Wed, 11 May 2022 22:38:04 GMT, Anthony Scarpino wrote: >> test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 162: >> >>> 160: statusServer != HandshakeStatus.NOT_HANDSHAKING); >>> 161: >>> 162: // Read NST >> >> What is NST? > > New Session Ticket Duh, of course! Yay, TLAs!!! (Three Letter Acronyms...) Feel free to expand if you're so inclined. ;) - PR: https://git.openjdk.java.net/jdk/pull/8462
Re: RFR: 8283577: SSLEngine.unwrap on read-only input ByteBuffer
On Wed, 11 May 2022 23:03:27 GMT, Anthony Scarpino wrote: >> test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 172: >> >>> 170: out.clear(); >>> 171: String testString = "ASDF"; >>> 172: in.put(testString.getBytes()).flip(); >> >> If you're going to convert back from UTF_8 later, you should probably >> convert using getBytes(UTF_8) here. > > setting the input to UTF8 isn't a concern. The latter line to decode it > changes it from using the ByteBuffer.toString() to the contents of the > ByteBuffer in a String. You could use the default charsets for encoding and decoding. i.e. in.clear(); receive(server, out.asReadOnlyBuffer(), in); byte[] ba = new byte[in.remaining()]; in.get(ba); String testResult = new String(ba); - PR: https://git.openjdk.java.net/jdk/pull/8462
Re: RFR: 8002277: Refactor two PBE classes to simplify maintenance [v7]
On Thu, 12 May 2022 20:53:00 GMT, Weijun Wang wrote:
>> Valerie Peng has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> trivial syntax fix.
>
> src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java line 244:
>
>> 242: iCount = DEFAULT_COUNT;
>> 243: }
>> 244: //if (ivSpec == null) { // old behavior always generate
>
> How could `ivSpec` be non-null here? IIUC the only answer is from a previous
> `engineInit`, and it should not be retained. I suggest removing this check
> plus adding an explicit `ivSpec = null` at the beginning of this method along
> with `iCount` and `salt`. Those are the only 3 non final instance fields.
Yeah, it's also possible that ivSpec is non-null if getParameters() is called
before init(). Now that salt and iCount is reset in the beginning of init(),
ivSpec should be reset too.
-
PR: https://git.openjdk.java.net/jdk/pull/8521
Re: RFR: 8283577: SSLEngine.unwrap on read-only input ByteBuffer
On Wed, 11 May 2022 22:25:43 GMT, Anthony Scarpino
wrote:
>> test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 1:
>>
>>> 1: /*
>>
>> Wondering why this is in javax/net/ssl/SSLSession instead of
>> sun/security/ssl/SSLCipher.
>
> I can move it.. I created it from another test which happen to be in that
> directory
Ah. If you can, please do. Thanks.
>> test/jdk/javax/net/ssl/SSLSession/ReadOnlyEngine.java line 157:
>>
>>> 155: // Do TLS handshake
>>> 156: do {
>>> 157: statusClient = doHandshake(client, out, in);
>>
>> It's potentially a little inefficient returning after each wrap/unwrap()
>> instead of doing the task right away, but it works. No need to change.
>>
>> Is this style something you copied from another test? The
>> SSLEngineTemplate in the templates directory is what I often use.
>
> I got it from somewhere that I don't remember off hand. I'm just trying to
> get through the handshake.
It was just an observation, no need to change.
-
PR: https://git.openjdk.java.net/jdk/pull/8462
Re: RFR: 8002277: Refactor two PBE classes to simplify maintenance [v7]
On Thu, 12 May 2022 03:28:15 GMT, Valerie Peng wrote:
>> This change refactors the PBES2Core and PKCS12PBECipherCore classes in
>> SunJCE provider as requested in the bug record. Functionality should remain
>> the same with a clearer and simplified code/control flow with less lines of
>> code. This should improve readability and maintenance. I enhanced one
>> existing regression test to test more scenarios. This test would pass before
>> the proposed change and continues to pass with the proposed changes.
>
> Valerie Peng has updated the pull request incrementally with one additional
> commit since the last revision:
>
> trivial syntax fix.
src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java line 244:
> 242: iCount = DEFAULT_COUNT;
> 243: }
> 244: //if (ivSpec == null) { // old behavior always generate
How could `ivSpec` be non-null here? IIUC the only answer is from a previous
`engineInit`, and it should not be retained. I suggest removing this check plus
adding an explicit `ivSpec = null` at the beginning of this method along with
`iCount` and `salt`. Those are the only 3 non final instance fields.
-
PR: https://git.openjdk.java.net/jdk/pull/8521
Re: RFR: 8002277: Refactor two PBE classes to simplify maintenance [v7]
On Thu, 12 May 2022 19:27:20 GMT, Sean Mullan wrote: >> Valerie Peng has updated the pull request incrementally with one additional >> commit since the last revision: >> >> trivial syntax fix. > > src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java line 184: > >> 182: "Iteration count must be a positive number"); >> 183: } >> 184: return iCount == 0? DEFAULT_COUNT : iCount; > > Nit: add space before '?'. Sure - PR: https://git.openjdk.java.net/jdk/pull/8521
Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v4]
On Thu, 12 May 2022 18:23:18 GMT, Sean Mullan wrote:
>> Fixed the nit. Thanks~
>> As for the part about returning the parameters as `{@code
>> AlgorithmParameters}`, it should be covered by current sentence, i.e. `and
>> can be generated by the signature`. Perhaps we don't have to spell out all
>> possible causes where the parameter can't be generated?
>
> That sentence is specifically if the caller did not specify parameters
> though. The previous wording is if the caller did specify parameters (as say
> an `AlgorithmParameterSpec`) and the implementation cannot return them as
> `AlgorithmParameters`. So I think it needs to be mentioned as it is kind of
> surprising if you specify parameters to `init` but `getParameters` returns
> null.
Hmm, would it fall under the "Otherwise, null is returned"? If not, perhaps we
can add back the part about returning AlgorithmParameters as below:
**If the underlying signature implementation supports returning the parameters
as {@code AlgorithmParameters},** the returned parameters may be the same that
were used to initialize
this signature, or may contain additional default or random parameter values
used by the underlying signature scheme. If the required parameters were not
supplied and can be generated by the signature, the generated parameters are
returned. Otherwise, {@code null} is returned.
-
PR: https://git.openjdk.java.net/jdk/pull/8396
Re: RFR: 8002277: Refactor two PBE classes to simplify maintenance [v7]
On Thu, 12 May 2022 03:28:15 GMT, Valerie Peng wrote: >> This change refactors the PBES2Core and PKCS12PBECipherCore classes in >> SunJCE provider as requested in the bug record. Functionality should remain >> the same with a clearer and simplified code/control flow with less lines of >> code. This should improve readability and maintenance. I enhanced one >> existing regression test to test more scenarios. This test would pass before >> the proposed change and continues to pass with the proposed changes. > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > trivial syntax fix. src/java.base/share/classes/com/sun/crypto/provider/PBES2Core.java line 184: > 182: "Iteration count must be a positive number"); > 183: } > 184: return iCount == 0? DEFAULT_COUNT : iCount; Nit: add space before '?'. - PR: https://git.openjdk.java.net/jdk/pull/8521
Re: RFR: 8253176: Signature.getParameters should specify that it can throw UnsupportedOperationException [v4]
On Wed, 11 May 2022 22:12:48 GMT, Valerie Peng wrote:
>> src/java.base/share/classes/java/security/SignatureSpi.java line 399:
>>
>>> 397: * values used by the underlying signature scheme. If the required
>>> 398: * parameters were not supplied and can be generated by the
>>> signature,
>>> 399: * the generated parameters will be returned. Otherwise, {@code
>>> null}
>>
>> Minor wording nit - change "will be" to "are" to be consistent with other
>> wording which uses the present tense. Same comment applies to
>> `Signature.getParameters`.
>>
>> Also, do we no longer need to mention the part "and the underlying signature
>> implementation supports returning the parameters as {@code
>> AlgorithmParameters}"?
>
> Fixed the nit. Thanks~
> As for the part about returning the parameters as `{@code
> AlgorithmParameters}`, it should be covered by current sentence, i.e. `and
> can be generated by the signature`. Perhaps we don't have to spell out all
> possible causes where the parameter can't be generated?
That sentence is specifically if the caller did not specify parameters though.
The previous wording is if the caller did specify parameters (as say an
`AlgorithmParameterSpec`) and the implementation cannot return them as
`AlgorithmParameters`. So I think it needs to be mentioned as it is kind of
surprising if you specify parameters to `init` but `getParameters` returns null.
-
PR: https://git.openjdk.java.net/jdk/pull/8396
Re: RFR: 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 Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8683
Re: RFR: 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 Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8683
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&pr=8683&range=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: 8002277: Refactor two PBE classes to simplify maintenance [v7]
On Thu, 12 May 2022 03:28:15 GMT, Valerie Peng wrote: >> This change refactors the PBES2Core and PKCS12PBECipherCore classes in >> SunJCE provider as requested in the bug record. Functionality should remain >> the same with a clearer and simplified code/control flow with less lines of >> code. This should improve readability and maintenance. I enhanced one >> existing regression test to test more scenarios. This test would pass before >> the proposed change and continues to pass with the proposed changes. > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > trivial syntax fix. LGTM. src/java.base/share/classes/com/sun/crypto/provider/PKCS12PBECipherCore.java line 363: > 361: return core.implGetParameters(); > 362: } > 363: @Override These 3 methods are the same in 3 classes. I wish we had a way to simplify this but I don't know myself. - Marked as reviewed by weijun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8521
Re: RFR: 8284194: Allow empty subject fields in keytool [v3]
> This code change allows one entering "." at a distinguished name prompt to > skip a sub-component when running `keytool -genkeyapir`. Several new resource > strings are added. > > There is no detailed description in `keytool.html`, so I think there's no > need to update it. > > I'll file a CSR to describe the behavior change. > > Here is an example after this change: > > $ keytool -genkeypair -keystore ks -storepass changeit -alias b -keyalg EC > Enter the distinguished name. Provide a single dot (.) to leave a > sub-component empty. > What is your first and last name? > [Unknown]: . > What is the name of your organizational unit? > [Unknown]: . > What is the name of your organization? > [Unknown]: . > What is the name of your City or Locality? > [Unknown]: . > What is the name of your State or Province? > [Unknown]: . > What is the two-letter country code for this unit? > [Unknown]: . > At least one field must be provided. Enter again. > Enter the distinguished name. Provide a single dot (.) to leave a > sub-component empty. > What is your first and last name? > [EMPTY]: Duke > What is the name of your organizational unit? > [EMPTY]: > What is the name of your organization? > [EMPTY]: > What is the name of your City or Locality? > [EMPTY]: > What is the name of your State or Province? > [EMPTY]: > What is the two-letter country code for this unit? > [EMPTY]: > Is CN=Duke correct? > [no]: yes > > Generating 384 bit EC (secp384r1) key pair and self-signed certificate > (SHA384withECDSA) with a validity of 90 days > for: CN=Duke > > In the first round, "." is entered for all fields and keytool rejected it. In > the second round, CN is entered but the others are unchanged (just type > enter, because they are already entered previously). At the end, the name is > "CN=Duke". Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: update the output - Changes: - all: https://git.openjdk.java.net/jdk/pull/8667/files - new: https://git.openjdk.java.net/jdk/pull/8667/files/1894055d..8c592f89 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8667&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8667&range=01-02 Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/8667.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8667/head:pull/8667 PR: https://git.openjdk.java.net/jdk/pull/8667
Re: RFR: 8282662: Use List.of() factory method to reduce memory consumption [v3]
On Thu, 10 Mar 2022 08:52:17 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 dubious changes in MethodType
src/java.base/share/classes/sun/security/validator/EndEntityChecker.java line
119:
> 117: // TLS key exchange algorithms requiring keyEncipherment key usage
> 118: private static final Collection KU_SERVER_ENCRYPTION =
> 119: Arrays.asList("RSA");
I understand that you haven't modified the `KU_SERVER_KEY_AGREEMENT` on line
122 because it has 4 elements. However, these 2 nearby lines using different
styles look a little strange to me. Why restrict the number to 0, 1, and 2? If
we have defined methods with up to 9 arguments, does that mean the new type is
suitable for 9 elements?
-
PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8284194: Allow empty subject fields in keytool [v2]
On Thu, 12 May 2022 13:48:46 GMT, Weijun Wang wrote: > I've already modified the prompt a little before the CSR is finalized. How > about > > ``` > Enter the distinguished name. Provider a single dot (.) to leave a > sub-component empty or press ENTER to use the default value in braces. > ``` > > Hopefully this is not too long, and macOS users know "ENTER" is "return", and > people won't debate on "braces" or "brackets". Ok, I like that. One typo though: s/Provider/Provide > As for the "[EMPTY]" prompt, the user has already entered "." in the first > round and we always remember the inputs as the new default values in the next > round so that they only need to enter the components they want to update. What about just "[]"? I think if you keep EMPTY, you should really define what that keyword means, but that might make the text too verbose. - PR: https://git.openjdk.java.net/jdk/pull/8667
Integrated: 8286423: Destroy password protection in the example code in KeyStore
On Tue, 10 May 2022 04:13:43 GMT, Xue-Lei Andrew Fan wrote: > Hi, > > May I have this simple example update in the KeyStore specification? > > Password protection should be destroyed in the example code in KeyStore > specification. Otherwise, applications may just copy and past the code, and > forget to clean up password protection. > > It's a trivial update, and may not worthy of a CSR. But please let me know > if you would like to have a CSR filed. > > Thanks, > Xuelei This pull request has now been integrated. Changeset: 1904e9d2 Author:Xue-Lei Andrew Fan URL: https://git.openjdk.java.net/jdk/commit/1904e9d280d1cce2deead4d4aa39dda1beb9dff1 Stats: 28 lines in 1 file changed: 14 ins; 13 del; 1 mod 8286423: Destroy password protection in the example code in KeyStore Reviewed-by: weijun - PR: https://git.openjdk.java.net/jdk/pull/8623
Re: RFR: 8284194: Allow empty subject fields in keytool [v2]
On Wed, 11 May 2022 23:40:46 GMT, Weijun Wang wrote: >> This code change allows one entering "." at a distinguished name prompt to >> skip a sub-component when running `keytool -genkeyapir`. Several new >> resource strings are added. >> >> There is no detailed description in `keytool.html`, so I think there's no >> need to update it. >> >> I'll file a CSR to describe the behavior change. >> >> Here is an example after this change: >> >> $ keytool -genkeypair -keystore ks -storepass changeit -alias b -keyalg EC >> Enter the distinguished name. Provide a single dot (.) to leave a >> sub-component empty. >> What is your first and last name? >> [Unknown]: . >> What is the name of your organizational unit? >> [Unknown]: . >> What is the name of your organization? >> [Unknown]: . >> What is the name of your City or Locality? >> [Unknown]: . >> What is the name of your State or Province? >> [Unknown]: . >> What is the two-letter country code for this unit? >> [Unknown]: . >> At least one field must be provided. Enter again. >> Enter the distinguished name. Provide a single dot (.) to leave a >> sub-component empty. >> What is your first and last name? >> [EMPTY]: Duke >> What is the name of your organizational unit? >> [EMPTY]: >> What is the name of your organization? >> [EMPTY]: >> What is the name of your City or Locality? >> [EMPTY]: >> What is the name of your State or Province? >> [EMPTY]: >> What is the two-letter country code for this unit? >> [EMPTY]: >> Is CN=Duke correct? >> [no]: yes >> >> Generating 384 bit EC (secp384r1) key pair and self-signed certificate >> (SHA384withECDSA) with a validity of 90 days >> for: CN=Duke >> >> In the first round, "." is entered for all fields and keytool rejected it. >> In the second round, CN is entered but the others are unchanged (just type >> enter, because they are already entered previously). At the end, the name is >> "CN=Duke". > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > word change I've already modified the prompt a little before the CSR is finalized. How about Enter the distinguished name. Provider a single dot (.) to leave a sub-component empty or press ENTER to use the default value in braces. Hopefully this is not too long, and macOS users know "ENTER" is "return", and people won't debate on "braces" or "brackets". As for the "[EMPTY]" prompt, the user has already entered "." in the first round and we always remember the inputs as the new default values in the next round so that they only need to enter the components they want to update. - PR: https://git.openjdk.java.net/jdk/pull/8667
Re: RFR: 8284194: Allow empty subject fields in keytool [v2]
On Wed, 11 May 2022 23:40:46 GMT, Weijun Wang wrote: >> This code change allows one entering "." at a distinguished name prompt to >> skip a sub-component when running `keytool -genkeyapir`. Several new >> resource strings are added. >> >> There is no detailed description in `keytool.html`, so I think there's no >> need to update it. >> >> I'll file a CSR to describe the behavior change. >> >> Here is an example after this change: >> >> $ keytool -genkeypair -keystore ks -storepass changeit -alias b -keyalg EC >> Enter the distinguished name. Enter a single dot (.) to leave the >> sub-component empty. >> What is your first and last name? >> [Unknown]: . >> What is the name of your organizational unit? >> [Unknown]: . >> What is the name of your organization? >> [Unknown]: . >> What is the name of your City or Locality? >> [Unknown]: . >> What is the name of your State or Province? >> [Unknown]: . >> What is the two-letter country code for this unit? >> [Unknown]: . >> At least one field must be provided. Enter again. >> Enter the distinguished name. Enter a single dot (.) to leave the >> sub-component empty. >> What is your first and last name? >> [EMPTY]: Duke >> What is the name of your organizational unit? >> [EMPTY]: >> What is the name of your organization? >> [EMPTY]: >> What is the name of your City or Locality? >> [EMPTY]: >> What is the name of your State or Province? >> [EMPTY]: >> What is the two-letter country code for this unit? >> [EMPTY]: >> Is CN=Duke correct? >> [no]: yes >> >> Generating 384 bit EC (secp384r1) key pair and self-signed certificate >> (SHA384withECDSA) with a validity of 90 days >> for: CN=Duke >> >> In the first round, "." is entered for all fields and keytool rejected it. >> In the second round, CN is entered but the others are unchanged (just type >> enter, because they are already entered previously). At the end, the name is >> "CN=Duke". > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > word change It might also be helpful to note that hitting return or enter will use the default, ex: "Enter a single dot (.) to leave the sub-component empty or enter return to use the default value in braces." For this: > What is your first and last name? > [EMPTY]: Duke I find the word "EMPTY" here a bit confusing because this is not a default value like "Unknown". It seems to me that it might be more intuitive to just repeat the initial set of prompts using [Unknown] and requiring '.' to be entered, especially since you repeat the part "Enter a single dot (.) to leave the sub-component empty." - PR: https://git.openjdk.java.net/jdk/pull/8667
Integrated: 8286422: Add OIDs for RC2 and Blowfish
On Wed, 11 May 2022 22:35:32 GMT, Weijun Wang wrote: > Add missing OIDs for 2 secret key algorithms. These will be used when storing > secret keys in a PKCS12 keystore. Like DES and DESede, the OIDs were > originally defined for CBC mode cipher algorithms, they are reused here for > key algorithms. > > OpenSSL uses the same OIDs for cipher algorithms. > > 1 3 6 1 4 1 3029 1 2 : BF-CBC: bf-cbc > rsadsi 3 2: RC2-CBC : rc2-cbc > > > Similar to DES, RC2 is only added as an alias so it needs to be specially > treated when getting a secret key entry from a PKCS12 keystore. > > No attempt is made to define these OIDs as aliases to existing cipher > algorithms. This pull request has now been integrated. Changeset: 752ad1c4 Author:Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/752ad1c41093645506dd267f618bd46882d0c674 Stats: 118 lines in 4 files changed: 66 ins; 49 del; 3 mod 8286422: Add OIDs for RC2 and Blowfish Reviewed-by: hchao, ascarpino - PR: https://git.openjdk.java.net/jdk/pull/8668
