On Fri, 7 Feb 2025 15:12:08 GMT, Sean Mullan <mul...@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
