RFR: 8346094: Harden X509CertImpl.getExtensionValue for NPE cases
Konanki Sreenath
duke at openjdk.org
Mon Feb 10 06:46:46 UTC 2025
On Fri, 7 Feb 2025 15:12:08 GMT, Sean Mullan <mullan at openjdk.org> wrote:
> > I'm wondering how necessary this fix is. These are internal classes, only called inside JDK, where some pre-conditions are always met. Unless someone explicitly calls `x509Certimpl.getInfo().setExtensions(null)` (as done in the test), it seems like both the `info` and `extensions` fields should never be null.
> > If you’re concerned about misuse of these methods leading to potential future issues, consider adding comments to clarify their expected usage. You could also include `assert` statements or `Objects.requireNonNull` calls to enforce these preconditions.
>
> I agree with Weijun. The `info` field should never be `null` under normal circumstances. Checking if it is `null` would only make sense if there is a bug somewhere else in the code, and in that case, the bug should be fixed.
I made these changes for the following reasons:
**Null Check on info:** The X509CertImpl ref can be created with a null value for info, so I added a null check to prevent potential issues.
**Consistency Across Internal Classes:** Other internal classes already perform the same null check on info. To maintain consistency, I added this change here as well.
**Null Check on extensions:** The info object can be created without extensions. Since other parts of the code already handle potential null values for extensions, I added this check to maintain uniformity.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23315#issuecomment-2647081463
More information about the security-dev
mailing list