Re: RFR: 8284105: Update security libraries to use sealed classes [v3]
> Please review these changes to update the security libraries to use sealed > classes. See JEP 409 for more details on sealed classes. > > No CSR is required as all the changes are to internal classes. A few classes > that did not have subclasses were simply marked final instead of sealed. Sean Mullan 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 four additional commits since the last revision: - Make KrbAsRep final. - Merge - Make some classes package-private instead of sealed. - Update security libraries to use sealed classes. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8165/files - new: https://git.openjdk.java.net/jdk/pull/8165/files/c27b1ba0..2a1ab2b8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8165=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8165=01-02 Stats: 4066 lines in 394 files changed: 1983 ins; 791 del; 1292 mod Patch: https://git.openjdk.java.net/jdk/pull/8165.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8165/head:pull/8165 PR: https://git.openjdk.java.net/jdk/pull/8165
Re: RFR: 8284105: Update security libraries to use sealed classes [v2]
On Fri, 8 Apr 2022 20:07:32 GMT, Sean Mullan wrote:
>> Please review these changes to update the security libraries to use sealed
>> classes. See JEP 409 for more details on sealed classes.
>>
>> No CSR is required as all the changes are to internal classes. A few classes
>> that did not have subclasses were simply marked final instead of sealed.
>
> Sean Mullan has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Make some classes package-private instead of sealed.
Only one comment. Others look fine.
src/java.security.jgss/share/classes/sun/security/krb5/KrbTgsRep.java line 43:
> 41: * Kerberos client.
> 42: */
> 43: final class KrbTgsRep extends KrbKdcRep {
Make `KrbAsRep` final also to be symmetric.
-
Marked as reviewed by weijun (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8165
Re: RFR: 8284105: Update security libraries to use sealed classes [v2]
> Please review these changes to update the security libraries to use sealed > classes. See JEP 409 for more details on sealed classes. > > No CSR is required as all the changes are to internal classes. A few classes > that did not have subclasses were simply marked final instead of sealed. Sean Mullan has updated the pull request incrementally with one additional commit since the last revision: Make some classes package-private instead of sealed. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8165/files - new: https://git.openjdk.java.net/jdk/pull/8165/files/52aa0ad6..c27b1ba0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8165=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8165=00-01 Stats: 93 lines in 8 files changed: 42 ins; 38 del; 13 mod Patch: https://git.openjdk.java.net/jdk/pull/8165.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8165/head:pull/8165 PR: https://git.openjdk.java.net/jdk/pull/8165
Re: RFR: 8284105: Update security libraries to use sealed classes [v2]
On Fri, 8 Apr 2022 16:11:27 GMT, Weijun Wang wrote:
>> Sean Mullan has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Make some classes package-private instead of sealed.
>
> src/java.base/share/classes/sun/security/rsa/RSASignature.java line 50:
>
>> 48: * @author Andreas Sterbenz
>> 49: */
>> 50: public abstract class RSASignature extends SignatureSpi {
>
> We can probably move the `RSASignature.encodeSignature` method to `RSAUtil`
> and this class can be package private.
Ok. I'll also move `RSASignature.decodeSignature` to `RSAUtil` to maintain
symmetry even though it isn't called outside the package.
> src/java.base/share/classes/sun/security/util/math/intpoly/IntegerPolynomial.java
> line 64:
>
>> 62: */
>> 63:
>> 64: public abstract class IntegerPolynomial implements IntegerFieldModuloP {
>
> Although we only have several implementations, I think this class is meant to
> be freely extendable for whatever new modulus.
We can always add new ones to the permits clause later.
-
PR: https://git.openjdk.java.net/jdk/pull/8165
Re: RFR: 8284105: Update security libraries to use sealed classes
On Fri, 8 Apr 2022 16:17:19 GMT, Weijun Wang wrote: > It looks `KrbTgsRep.java`, `Krb5ProxyCredential.java`, `Builder.java`, > `Vertex.java`, `Validator.java`, and `RSAKeyPairGenerator.java` can all be > made package private. Good point, although I would prefer to leave `Validator` as public for now until we are more sure of the compatibility risk as there have been external dependencies on it. It may be useful to apply the `sealed` modifier to package-private classes, but for this RFE that is not the goal, so I will remove the `sealed` modifier from these classes (if applicable) when I make them package-private. - PR: https://git.openjdk.java.net/jdk/pull/8165
Re: RFR: 8284105: Update security libraries to use sealed classes
On Fri, 8 Apr 2022 13:40:37 GMT, Sean Mullan wrote: > Please review these changes to update the security libraries to use sealed > classes. See JEP 409 for more details on sealed classes. > > No CSR is required as all the changes are to internal classes. A few classes > that did not have subclasses were simply marked final instead of sealed. It looks safe to me as if compiling and test passed. - Marked as reviewed by xuelei (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8165
Re: RFR: 8284105: Update security libraries to use sealed classes
On Fri, 8 Apr 2022 13:40:37 GMT, Sean Mullan wrote:
> Please review these changes to update the security libraries to use sealed
> classes. See JEP 409 for more details on sealed classes.
>
> No CSR is required as all the changes are to internal classes. A few classes
> that did not have subclasses were simply marked final instead of sealed.
It looks `KrbTgsRep.java`, `Krb5ProxyCredential.java`, `Builder.java`,
`Vertex.java`, `Validator.java`, and `RSAKeyPairGenerator.java` can all be made
package private.
src/java.base/share/classes/sun/security/rsa/RSASignature.java line 50:
> 48: * @author Andreas Sterbenz
> 49: */
> 50: public abstract class RSASignature extends SignatureSpi {
We can probably move the `RSASignature.encodeSignature` method to `RSAUtil` and
this class can be package private.
src/java.base/share/classes/sun/security/util/math/intpoly/IntegerPolynomial.java
line 64:
> 62: */
> 63:
> 64: public abstract class IntegerPolynomial implements IntegerFieldModuloP {
Although we only have several implementations, I think this class is meant to
be freely extendable for whatever new modulus.
-
PR: https://git.openjdk.java.net/jdk/pull/8165
Re: RFR: 8284105: Update security libraries to use sealed classes
On Fri, 8 Apr 2022 13:40:37 GMT, Sean Mullan wrote: > Please review these changes to update the security libraries to use sealed > classes. See JEP 409 for more details on sealed classes. > > No CSR is required as all the changes are to internal classes. A few classes > that did not have subclasses were simply marked final instead of sealed. Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8165
