RFR: 8296143: CertAttrSet's set/get mechanism is not type-safe [v6]

Weijun Wang weijun at openjdk.org
Tue Nov 8 19:48:48 UTC 2022


> The major change is to remove the `get` and `set` methods in various `CertAttrSet` child classes and change them to `setXyz` and `getXyz` methods. The `Xyz` words might come from the field name or the attribute name. For example, `X509CertInfo` now has `setExtensions` and `setValidity` instead of `set("extensions", exts)` and `set("validity", validity)`. This also has the benefit to remove a lot of try-catch blocks on `IOException`s on "unknown attributes" because everything is known now. At the same time, all the identifier name and attribute names are removed from `CertAttrSet` child classes. The only left is `NAME` in extensions since it's still used as keys in `CertificateExtensions`.
> 
> Besides assigning a new value to an internal field, the original `set` methods might also re-encode by calling `encodeThis`, invalidate the cached encoding (in `X509CertInfo`), or check for read-only flag (in `X509CertImp`). Newly added `setXyz` methods are doing the same. This is one place that future new setter methods should remember.
> 
> Most `get` implementations simply return an internal field. One exception in `X509CertImpl` is that when getting something inside the `X509CertInfo`, it wraps exceptions into a new `CertificateParsingException`. This is actually related to the way `CertificateExtensions::get` is implemented where an exception is thrown when an extension does not exist. `CertificateExtensions::getExtension` has been rewritten to follow the `CRLExtensions::getExtension` style where `null` is returned in this case.
> 
> The only method left in `CertAttrSet` is `encode`, and it no longer throws a `CertificateException`.
> 
> Several classes do have their attributes, and still has get/set methods. This includes `CertificateExtensions`, `CRLExtensions`, `ReasonFlags`, `KeyUsageExtension`, and `NetscapeCertTypeExtensions`. Some methods are renamed to be clearer. For example, in `CertificateExtensions`, we have `getExtension` instead of `get`.
> 
> There are no more `AttributeNameEnumeration.java` and `X509AttributeName.java`.

Weijun Wang has updated the pull request incrementally with one additional commit since the last revision:

  more cleanup

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/10959/files
  - new: https://git.openjdk.org/jdk/pull/10959/files/b4453f66..a62ef66d

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10959&range=05
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10959&range=04-05

  Stats: 24 lines in 8 files changed: 0 ins; 20 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/10959.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10959/head:pull/10959

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


More information about the security-dev mailing list