commit 2c1c9fa5cbb123555f38947af3c3ad3f29132dda
author David Benjamin <davidben@google.com> Mon Mar 15 18:18:34 2021 -0400
committer Adam Langley <agl@google.com> Tue Mar 16 16:58:31 2021 +0000
tree c71a24493b17fe29bbc9ee6621d2ff730fb739ff
parent 1eae29777c4309eb3bca73702b84becaebb71dc5

Better document nullable X.509 getters.

When it is and isn't safe to assume an X509 field is non-NULL seems to
cause some confusion. (I often get requests to add NULL checks when
rewriting calling code.)

X.509 has surprisingly few optional fields, and we generally say
pointers are non-NULL unless documented. But that only works if we
remember to mention the nullable ones.

Change-Id: I18b57a17c9d57c377ea2227347e423f574389818
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46185
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/x509.h

diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 24c589b..1550b2c 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -536,9 +536,12 @@
 // instead.
 OPENSSL_EXPORT int X509_set_notAfter(X509 *x509, const ASN1_TIME *tm);
 
-// X509_get0_uids sets |*out_issuer_uid| and |*out_subject_uid| to non-owning
-// pointers to the issuerUID and subjectUID fields, respectively, of |x509|.
-// Either output pointer may be NULL to skip the field.
+// X509_get0_uids sets |*out_issuer_uid| to a non-owning pointer to the
+// issuerUID field of |x509|, or NULL if |x509| has no issuerUID. It similarly
+// outputs |x509|'s subjectUID field to |*out_subject_uid|.
+//
+// Callers may pass NULL to either |out_issuer_uid| or |out_subject_uid| to
+// ignore the corresponding field.
 OPENSSL_EXPORT void X509_get0_uids(const X509 *x509,
                                    const ASN1_BIT_STRING **out_issuer_uid,
                                    const ASN1_BIT_STRING **out_subject_uid);
@@ -615,14 +618,16 @@
 // const-correct for legacy reasons.
 OPENSSL_EXPORT X509_NAME *X509_CRL_get_issuer(const X509_CRL *crl);
 
-// X509_CRL_get_REVOKED returns the list of revoked certificates in |crl|.
+// X509_CRL_get_REVOKED returns the list of revoked certificates in |crl|, or
+// NULL if |crl| omits it.
 //
 // TOOD(davidben): This function was originally a macro, without clear const
 // semantics. It should take a const input and give const output, but the latter
 // would break existing callers. For now, we match upstream.
 OPENSSL_EXPORT STACK_OF(X509_REVOKED) *X509_CRL_get_REVOKED(X509_CRL *crl);
 
-// X509_CRL_get0_extensions returns |crl|'s extension list.
+// X509_CRL_get0_extensions returns |crl|'s extension list, or NULL if |crl|
+// omits it.
 OPENSSL_EXPORT const STACK_OF(X509_EXTENSION) *X509_CRL_get0_extensions(
     const X509_CRL *crl);
 
@@ -1146,6 +1151,8 @@
 OPENSSL_EXPORT int X509_set_pubkey(X509 *x, EVP_PKEY *pkey);
 OPENSSL_EXPORT EVP_PKEY *X509_get_pubkey(X509 *x);
 OPENSSL_EXPORT ASN1_BIT_STRING *X509_get0_pubkey_bitstr(const X509 *x);
+
+// X509_get0_extensions returns |x|'s extension list, or NULL if |x| omits it.
 OPENSSL_EXPORT const STACK_OF(X509_EXTENSION) *X509_get0_extensions(
     const X509 *x);
 
@@ -1285,7 +1292,8 @@
 OPENSSL_EXPORT int X509_REVOKED_set_revocationDate(X509_REVOKED *revoked,
                                                    const ASN1_TIME *tm);
 
-// X509_REVOKED_get0_extensions returns |r|'s extensions.
+// X509_REVOKED_get0_extensions returns |r|'s extensions list, or NULL if |r|
+// omits it.
 OPENSSL_EXPORT const STACK_OF(X509_EXTENSION) *X509_REVOKED_get0_extensions(
     const X509_REVOKED *r);