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&pr=8165&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8165&range=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&pr=8165&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8165&range=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
RFR: 8284105: Update security libraries to use sealed classes
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. - Commit messages: - Update security libraries to use sealed classes. Changes: https://git.openjdk.java.net/jdk/pull/8165/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8165&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8284105 Stats: 50 lines in 20 files changed: 8 ins; 0 del; 42 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
