RFR: 8296072: CertAttrSet::encode and DerEncoder::derEncode should write into DerOutputStream [v3]

Sean Mullan mullan at openjdk.org
Mon Oct 31 18:32:18 UTC 2022


On Fri, 28 Oct 2022 22:04:39 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> The argument of the `CertAttrSet::encode` and `DerEncoder::derEncode` interface methods are modified from `OutputStream` to `DerOutputStream`. All implementations are modified the same way.
>> 
>> `OutputStream` is still used by `sun.security.x509.Extension::encode(OutputStream os)` because it's inherited from `java.security.cert.Extension`. The method is now marked final to avoid accidental override.
>> 
>> In `CertificateExtensions` and `CRLExtensions`, only `Extension::encode(DerOutputStream out)` is called. It used to call `CertAttrSet::encode` for a known extension and `Extension::encode(DerOutputStream out)` for an unknown one. This makes sure the overridden `encode` methods in known extensions are always called. Now that they have the same argument, there is no need for this check.
>
> Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   same for DerEncoder::derEncode
>   
>   only in patch2:
>   unchanged:

A few minor comments.

src/java.base/share/classes/sun/security/pkcs10/PKCS10Attribute.java line 110:

> 108:      * @exception IOException on encoding errors.
> 109:      */
> 110:     public void derEncode(DerOutputStream out) throws IOException {

Add @Override

src/java.base/share/classes/sun/security/pkcs10/PKCS10Attributes.java line 96:

> 94:      * @exception IOException on encoding errors.
> 95:      */
> 96:     public void encode(DerOutputStream out) throws IOException {

Add @Override

src/java.base/share/classes/sun/security/x509/CertAttrSet.java line 67:

> 65:      * @exception IOException on other errors.
> 66:      */
> 67:     void encode(DerOutputStream out)

A bit of a nit, but on line 62, change `OutputStream` to `DerOutputStream` since it is referring to a type. Check the javadoc of all other classes for the same change.

-------------

PR: https://git.openjdk.org/jdk/pull/10906


More information about the security-dev mailing list