Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]
On Fri, 29 Apr 2022 15:23:47 GMT, Sean Mullan wrote:
>> Valerie Peng has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update for getParameters()
>
> src/java.base/share/classes/javax/crypto/Cipher.java line 1053:
>
>> 1051: * The returned parameters may be the same that were used to
>> initialize
>> 1052: * this cipher, or may contain additional default or random
>> parameter
>> 1053: * values used by the underlying cipher implementation. If the
>> required
>
> The PR for https://github.com/openjdk/jdk/pull/8396/ has one difference in
> this sentence in that it says "... values used by the underlying signature
> implementation _if the underlying signature implementation supports returning
> the parameters as {@code AlgorithmParameters}_." Should this also be
> consistent?
>
> Also, I don't think you need to repeat "underlying signature " again above,
> you could just say: ... values used by the underlying signature
> implementation _if the implementation supports returning the parameters as
> {@code AlgorithmParameters}_."
Signature class has that sentence is due to the peculiar case of EdDSA
signature impl where its parameter spec does not correlate to an ASN.1
structure. Since there is no such case for current cipher impls (as far as I
know), that's why I have not added it to Cipher class.
Ok, how about `If the required parameters were not supplied and can be
generated by the cipher, the generated parameters will be returned.`?
-
PR: https://git.openjdk.java.net/jdk/pull/8117
Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]
On Fri, 29 Apr 2022 15:18:34 GMT, Sean Mullan wrote: >> Valerie Peng has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update for getParameters() > > src/java.base/share/classes/javax/crypto/Cipher.java line 1055: > >> 1053: * values used by the underlying cipher implementation. If the >> required >> 1054: * parameters were not supplied and the underlying cipher >> implementation >> 1055: * can generate the parameter values, it will be returned. >> Otherwise, > > "it will be returned" sounds a bit awkward here. I would change this to "the > generated parameter values will be returned". Ok. - PR: https://git.openjdk.java.net/jdk/pull/8117
Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]
On Thu, 28 Apr 2022 19:23:18 GMT, Weijun Wang wrote:
>> Valerie Peng has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update for getParameters()
>
> src/java.base/share/classes/javax/crypto/CipherSpi.java line 449:
>
>> 447: *
>> 448: * Note that when a Cipher object is initialized, it loses all
>> 449: * previously-acquired state. In other words, initializing a Cipher
>> is
>
> Two `{@code Cipher}` above.
Yes, I will look through the whole class (and CipherSpi.java too) regarding the
Cipher vs cipher thing.
-
PR: https://git.openjdk.java.net/jdk/pull/8117
Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]
On Thu, 28 Apr 2022 19:17:08 GMT, Weijun Wang wrote:
>> Valerie Peng has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update for getParameters()
>
> src/java.base/share/classes/javax/crypto/Cipher.java line 1056:
>
>> 1054: * parameters were not supplied and the underlying cipher
>> implementation
>> 1055: * can generate the parameter values, it will be returned.
>> Otherwise,
>> 1056: * {@code null} returned.
>
> Should this be "null is returned"?
Yes, will fix.
> src/java.base/share/classes/javax/crypto/Cipher.java line 1787:
>
>> 1785: * Ensures that Cipher is in a valid state for update() and
>> doFinal()
>> 1786: * calls - should be initialized and in ENCRYPT_MODE or
>> DECRYPT_MODE.
>> 1787: * @throws IllegalStateException if Cipher object is not in valid
>> state
>
> "Cipher" in `{@code}`? Or make it lowercase.
Ok, I will use lowercase.
-
PR: https://git.openjdk.java.net/jdk/pull/8117
Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]
On Thu, 28 Apr 2022 19:11:23 GMT, Valerie Peng wrote:
>> Anyone can help review this javadoc update? The main change is the wording
>> for the method javadoc of
>> Cipher.getParameters()/CipherSpi.engineGetParameters(). The original wording
>> is somewhat restrictive and request is to broaden this to accommodate more
>> scenarios such as when null can be returned.
>> The rest are minor things like add {@code } to class name and null, and
>> remove redundant ".".
>>
>> Will file CSR after the review is close to being wrapped up.
>> Thanks~
>
> Valerie Peng has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update for getParameters()
Changes requested by mullan (Reviewer).
src/java.base/share/classes/javax/crypto/Cipher.java line 1053:
> 1051: * The returned parameters may be the same that were used to
> initialize
> 1052: * this cipher, or may contain additional default or random
> parameter
> 1053: * values used by the underlying cipher implementation. If the
> required
The PR for https://github.com/openjdk/jdk/pull/8396/ has one difference in this
sentence in that it says "... values used by the underlying signature
implementation _if the underlying signature implementation supports returning
the parameters as {@code AlgorithmParameters}_." Should this also be consistent?
Also, I don't think you need to repeat "underlying signature " again above, you
could just say: ... values used by the underlying signature implementation _if
the implementation supports returning the parameters as {@code
AlgorithmParameters}_."
src/java.base/share/classes/javax/crypto/Cipher.java line 1055:
> 1053: * values used by the underlying cipher implementation. If the
> required
> 1054: * parameters were not supplied and the underlying cipher
> implementation
> 1055: * can generate the parameter values, it will be returned.
> Otherwise,
"it will be returned" sounds a bit awkward here. I would change this to "the
generated parameter values will be returned".
-
PR: https://git.openjdk.java.net/jdk/pull/8117
Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]
On Thu, 28 Apr 2022 19:11:23 GMT, Valerie Peng wrote:
>> Anyone can help review this javadoc update? The main change is the wording
>> for the method javadoc of
>> Cipher.getParameters()/CipherSpi.engineGetParameters(). The original wording
>> is somewhat restrictive and request is to broaden this to accommodate more
>> scenarios such as when null can be returned.
>> The rest are minor things like add {@code } to class name and null, and
>> remove redundant ".".
>>
>> Will file CSR after the review is close to being wrapped up.
>> Thanks~
>
> Valerie Peng has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update for getParameters()
src/java.base/share/classes/javax/crypto/Cipher.java line 1056:
> 1054: * parameters were not supplied and the underlying cipher
> implementation
> 1055: * can generate the parameter values, it will be returned.
> Otherwise,
> 1056: * {@code null} returned.
Should this be "null is returned"?
src/java.base/share/classes/javax/crypto/Cipher.java line 1787:
> 1785: * Ensures that Cipher is in a valid state for update() and
> doFinal()
> 1786: * calls - should be initialized and in ENCRYPT_MODE or
> DECRYPT_MODE.
> 1787: * @throws IllegalStateException if Cipher object is not in valid
> state
"Cipher" in `{@code}`? Or make it lowercase.
src/java.base/share/classes/javax/crypto/CipherSpi.java line 449:
> 447: *
> 448: * Note that when a Cipher object is initialized, it loses all
> 449: * previously-acquired state. In other words, initializing a Cipher
> is
Two `{@code Cipher}` above.
-
PR: https://git.openjdk.java.net/jdk/pull/8117
Re: RFR: 8209038: Clarify the javadoc of Cipher.getParameters() [v3]
> Anyone can help review this javadoc update? The main change is the wording
> for the method javadoc of
> Cipher.getParameters()/CipherSpi.engineGetParameters(). The original wording
> is somewhat restrictive and request is to broaden this to accommodate more
> scenarios such as when null can be returned.
> The rest are minor things like add {@code } to class name and null, and
> remove redundant ".".
>
> Will file CSR after the review is close to being wrapped up.
> Thanks~
Valerie Peng has updated the pull request incrementally with one additional
commit since the last revision:
Update for getParameters()
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/8117/files
- new: https://git.openjdk.java.net/jdk/pull/8117/files/9b8b71a6..baae26fe
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8117&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8117&range=01-02
Stats: 8 lines in 2 files changed: 2 ins; 0 del; 6 mod
Patch: https://git.openjdk.java.net/jdk/pull/8117.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/8117/head:pull/8117
PR: https://git.openjdk.java.net/jdk/pull/8117
