Add a few more TODOs for functions that should be const but aren't
Mostly so we don't forget to const them when X509_NAME is finally fixed.
Bug: 42290269
Change-Id: I78a8b31d9b846669db1ba2e98133203e5be24949
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81748
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 0cd8716..837c0d7 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -88,9 +88,8 @@
// |i2d_X509_AUX|) are not preserved. Additionally, if |x509| is incomplete,
// this function may fail.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |crl| was
-// mutated.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |crl| was mutated.
OPENSSL_EXPORT X509 *X509_dup(X509 *x509);
// X509_free decrements |x509|'s reference count and, if zero, releases memory
@@ -110,9 +109,8 @@
// i2d_X509 marshals |x509| as a DER-encoded X.509 Certificate (RFC 5280), as
// described in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |x509| was
-// mutated.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |x509| was mutated.
OPENSSL_EXPORT int i2d_X509(X509 *x509, uint8_t **outp);
// X509_VERSION_* are X.509 version numbers. Note the numerical values of all
@@ -429,8 +427,8 @@
// as equal. This function should only be used with |X509| objects that were
// parsed from bytes and never mutated.
//
-// TODO(https://crbug.com/boringssl/407): This function is const, but it is not
-// always thread-safe, notably if |a| and |b| were mutated.
+// TODO(crbug.com/42290269): This function is const, but it is not always
+// thread-safe, notably if |a| and |b| were mutated.
OPENSSL_EXPORT int X509_cmp(const X509 *a, const X509 *b);
@@ -564,7 +562,7 @@
// ASN.1 element. Directly embedding the output in a larger ASN.1 structure will
// not behave correctly.
//
-// TODO(crbug.com/boringssl/407): |x509| should be const.
+// TODO(crbug.com/42290269): |x509| should be const.
OPENSSL_EXPORT int i2d_X509_AUX(X509 *x509, uint8_t **outp);
// d2i_X509_AUX parses up to |length| bytes from |*inp| as a DER-encoded X.509
@@ -675,9 +673,8 @@
// function works by serializing the structure, so if |crl| is incomplete, it
// may fail.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |crl| was
-// mutated.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |crl| was mutated.
OPENSSL_EXPORT X509_CRL *X509_CRL_dup(X509_CRL *crl);
// X509_CRL_free decrements |crl|'s reference count and, if zero, releases
@@ -692,9 +689,8 @@
// i2d_X509_CRL marshals |crl| as a X.509 CertificateList (RFC 5280), as
// described in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |crl| was
-// mutated.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |crl| was mutated.
OPENSSL_EXPORT int i2d_X509_CRL(X509_CRL *crl, uint8_t **outp);
// X509_CRL_match compares |a| and |b| and returns zero if they are equal, a
@@ -1068,9 +1064,8 @@
// function works by serializing the structure, so if |req| is incomplete, it
// may fail.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |req| was
-// mutated.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |req| was mutated.
OPENSSL_EXPORT X509_REQ *X509_REQ_dup(X509_REQ *req);
// X509_REQ_free releases memory associated with |req|.
@@ -1084,9 +1079,8 @@
// i2d_X509_REQ marshals |req| as a CertificateRequest (RFC 2986), as described
// in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |req| was
-// mutated.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |req| was mutated.
OPENSSL_EXPORT int i2d_X509_REQ(X509_REQ *req, uint8_t **outp);
// X509_REQ_VERSION_1 is the version constant for |X509_REQ| objects. No other
@@ -1352,24 +1346,22 @@
// i2d_X509_NAME marshals |in| as a DER-encoded X.509 Name (RFC 5280), as
// described in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |in| was
-// mutated.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |in| was mutated.
OPENSSL_EXPORT int i2d_X509_NAME(X509_NAME *in, uint8_t **outp);
// X509_NAME_dup returns a newly-allocated copy of |name|, or NULL on error.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |name| was
-// mutated.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |name| was mutated.
OPENSSL_EXPORT X509_NAME *X509_NAME_dup(X509_NAME *name);
// X509_NAME_cmp compares |a| and |b|'s canonicalized forms. It returns zero if
// they are equal, one if |a| sorts after |b|, -1 if |b| sorts after |a|, and -2
// on error.
//
-// TODO(https://crbug.com/boringssl/407): This function is const, but it is not
-// always thread-safe, notably if |name| was mutated.
+// TODO(crbug.com/42290269): This function is const, but it is not always
+// thread-safe, notably if |name| was mutated.
//
// TODO(https://crbug.com/boringssl/355): The -2 return is very inconvenient to
// pass to a sorting function. Can we make this infallible? In the meantime,
@@ -1386,17 +1378,15 @@
// Avoid this function and prefer |i2d_X509_NAME|. It is one of the reasons
// |X509_NAME| functions, including this one, are not consistently thread-safe
// or const-correct. Depending on the resolution of
-// https://crbug.com/boringssl/407, this function may be removed or cause poor
-// performance.
+// crbug.com/42290269, this function may be removed or cause poor performance.
OPENSSL_EXPORT int X509_NAME_get0_der(X509_NAME *name, const uint8_t **out_der,
size_t *out_der_len);
// X509_NAME_set makes a copy of |name|. On success, it frees |*xn|, sets |*xn|
// to the copy, and returns one. Otherwise, it returns zero.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |name| was
-// mutated.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |name| was mutated.
OPENSSL_EXPORT int X509_NAME_set(X509_NAME **xn, X509_NAME *name);
// X509_NAME_entry_count returns the number of entries in |name|.
@@ -2093,18 +2083,18 @@
// i2d_GENERAL_NAME marshals |in| as a DER-encoded X.509 GeneralName (RFC 5280),
// as described in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |in| is an
-// directoryName and the |X509_NAME| has been modified.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |in| is an directoryName and
+// the |X509_NAME| has been modified.
OPENSSL_EXPORT int i2d_GENERAL_NAME(GENERAL_NAME *in, uint8_t **outp);
// GENERAL_NAME_dup returns a newly-allocated copy of |gen|, or NULL on error.
// This function works by serializing the structure, so it will fail if |gen| is
// empty.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if |gen| is an
-// directoryName and the |X509_NAME| has been modified.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if |gen| is an directoryName and
+// the |X509_NAME| has been modified.
OPENSSL_EXPORT GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *gen);
// GENERAL_NAMES_new returns a new, empty |GENERAL_NAMES|, or NULL on error.
@@ -2121,9 +2111,9 @@
// i2d_GENERAL_NAMES marshals |in| as a DER-encoded SEQUENCE OF GeneralName, as
// described in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): This function should be const and
-// thread-safe but is currently neither in some cases, notably if some element
-// of |in| is an directoryName and the |X509_NAME| has been modified.
+// TODO(crbug.com/42290269): This function should be const and thread-safe but
+// is currently neither in some cases, notably if some element of |in| is an
+// directoryName and the |X509_NAME| has been modified.
OPENSSL_EXPORT int i2d_GENERAL_NAMES(GENERAL_NAMES *in, uint8_t **outp);
// OTHERNAME_new returns a new, empty |OTHERNAME|, or NULL on error.
@@ -2229,8 +2219,8 @@
// i2d_AUTHORITY_KEYID marshals |akid| as a DER-encoded AuthorityKeyIdentifier
// (RFC 5280), as described in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): |akid| is not const because it
-// contains an |X509_NAME|.
+// TODO(crbug.com/42290269): |akid| is not const because it contains an
+// |X509_NAME|.
OPENSSL_EXPORT int i2d_AUTHORITY_KEYID(AUTHORITY_KEYID *akid, uint8_t **outp);
@@ -2322,8 +2312,8 @@
// i2d_AUTHORITY_INFO_ACCESS marshals |aia| as a DER-encoded
// AuthorityInfoAccessSyntax (RFC 5280), as described in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): |aia| is not const because it
-// contains an |X509_NAME|.
+// TODO(crbug.com/42290269): |aia| is not const because it contains an
+// |X509_NAME|.
OPENSSL_EXPORT int i2d_AUTHORITY_INFO_ACCESS(AUTHORITY_INFO_ACCESS *aia,
uint8_t **outp);
@@ -2397,8 +2387,8 @@
// i2d_CRL_DIST_POINTS marshals |crldp| as a DER-encoded CRLDistributionPoints
// (RFC 5280), as described in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): |crldp| is not const because it
-// contains an |X509_NAME|.
+// TODO(crbug.com/42290269): |crldp| is not const because it contains an
+// |X509_NAME|.
OPENSSL_EXPORT int i2d_CRL_DIST_POINTS(CRL_DIST_POINTS *crldp, uint8_t **outp);
// A ISSUING_DIST_POINT_st, aka |ISSUING_DIST_POINT|, represents a
@@ -2431,8 +2421,8 @@
// i2d_ISSUING_DIST_POINT marshals |idp| as a DER-encoded
// IssuingDistributionPoint (RFC 5280), as described in |i2d_SAMPLE|.
//
-// TODO(https://crbug.com/boringssl/407): |idp| is not const because it
-// contains an |X509_NAME|.
+// TODO(crbug.com/42290269): |idp| is not const because it contains an
+// |X509_NAME|.
OPENSSL_EXPORT int i2d_ISSUING_DIST_POINT(ISSUING_DIST_POINT *idp,
uint8_t **outp);
@@ -3705,9 +3695,8 @@
// there will be hash collisions. It also depends on an OpenSSL-specific
// canonicalization process.
//
-// TODO(https://crbug.com/boringssl/407): This should be const and thread-safe
-// but currently is neither, notably if |name| was modified from its parsed
-// value.
+// TODO(crbug.com/42290269): This should be const and thread-safe but currently
+// is neither, notably if |name| was modified from its parsed value.
OPENSSL_EXPORT uint32_t X509_NAME_hash(X509_NAME *name);
// X509_NAME_hash_old returns a hash of |name|, or zero on error. This is the
@@ -3718,9 +3707,8 @@
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions.
//
-// TODO(https://crbug.com/boringssl/407): This should be const and thread-safe
-// but currently is neither, notably if |name| was modified from its parsed
-// value.
+// TODO(crbug.com/42290269): This should be const and thread-safe but currently
+// is neither, notably if |name| was modified from its parsed value.
OPENSSL_EXPORT uint32_t X509_NAME_hash_old(X509_NAME *name);
// X509_STORE_set_default_paths configures |store| to read from some "default"
@@ -4439,6 +4427,9 @@
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions. It also depends on an OpenSSL-specific
// canonicalization process.
+//
+// TODO(crbug.com/42290269): This should be const and thread-safe but currently
+// is neither, notably if |x509| was modified from its parsed value.
OPENSSL_EXPORT uint32_t X509_issuer_name_hash(X509 *x509);
// X509_subject_name_hash returns the hash of |x509|'s subject name with
@@ -4448,6 +4439,9 @@
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions. It also depends on an OpenSSL-specific
// canonicalization process.
+//
+// TODO(crbug.com/42290269): This should be const and thread-safe but currently
+// is neither, notably if |x509| was modified from its parsed value.
OPENSSL_EXPORT uint32_t X509_subject_name_hash(X509 *x509);
// X509_issuer_name_hash_old returns the hash of |x509|'s issuer name with
@@ -4456,6 +4450,9 @@
// This hash is specific to the |X509_LOOKUP_add_dir| filesystem format and is
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions.
+//
+// TODO(crbug.com/42290269): This should be const and thread-safe but currently
+// is neither, notably if |x509| was modified from its parsed value.
OPENSSL_EXPORT uint32_t X509_issuer_name_hash_old(X509 *x509);
// X509_subject_name_hash_old returns the hash of |x509|'s usjbect name with
@@ -4464,6 +4461,9 @@
// This hash is specific to the |X509_LOOKUP_add_dir| filesystem format and is
// not suitable for general-purpose X.509 name processing. It is very short, so
// there will be hash collisions.
+//
+// TODO(crbug.com/42290269): This should be const and thread-safe but currently
+// is neither, notably if |x509| was modified from its parsed value.
OPENSSL_EXPORT uint32_t X509_subject_name_hash_old(X509 *x509);
@@ -4582,7 +4582,7 @@
// This function returning one does not indicate that |x509| is trusted, only
// that it is eligible to be a CA.
//
-// TODO(crbug.com/boringssl/407): |x509| should be const.
+// TODO(crbug.com/42290269): |x509| should be const.
OPENSSL_EXPORT int X509_check_ca(X509 *x509);
// X509_check_issued checks if |issuer| and |subject|'s name, authority key
@@ -4593,13 +4593,13 @@
// intended to prune the set of possible issuer certificates during
// path-building.
//
-// TODO(crbug.com/boringssl/407): Both parameters should be const.
+// TODO(crbug.com/42290269): Both parameters should be const.
OPENSSL_EXPORT int X509_check_issued(X509 *issuer, X509 *subject);
// NAME_CONSTRAINTS_check checks if |x509| satisfies name constraints in |nc|.
// It returns |X509_V_OK| on success and some |X509_V_ERR_*| constant on error.
//
-// TODO(crbug.com/boringssl/407): Both parameters should be const.
+// TODO(crbug.com/42290269): Both parameters should be const.
OPENSSL_EXPORT int NAME_CONSTRAINTS_check(X509 *x509, NAME_CONSTRAINTS *nc);
// X509_check_host checks if |x509| matches the DNS name |chk|. It returns one
@@ -4679,7 +4679,7 @@
// This function only searches for trusted issuers. It does not consider
// untrusted intermediates passed in to |X509_STORE_CTX_init|.
//
-// TODO(crbug.com/boringssl/407): |x509| should be const.
+// TODO(crbug.com/42290269): |x509| should be const.
OPENSSL_EXPORT int X509_STORE_CTX_get1_issuer(X509 **out_issuer,
X509_STORE_CTX *ctx, X509 *x509);
@@ -4713,7 +4713,7 @@
// NULL on error. The caller must release the result with |sk_X509_pop_free| and
// |X509_free| when done.
//
-// TODO(crbug.com/boringssl/407): |name| should be const.
+// TODO(crbug.com/42290269): |name| should be const.
OPENSSL_EXPORT STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx,
X509_NAME *name);
@@ -4722,7 +4722,7 @@
// The caller must release the result with |sk_X509_CRL_pop_free| and
// |X509_CRL_free| when done.
//
-// TODO(crbug.com/boringssl/407): |name| should be const.
+// TODO(crbug.com/42290269): |name| should be const.
OPENSSL_EXPORT STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *ctx,
X509_NAME *name);
@@ -4740,7 +4740,7 @@
// case, this function returns an arbitrary match. Use
// |X509_STORE_CTX_get1_certs| or |X509_STORE_CTX_get1_crls| instead.
//
-// TODO(crbug.com/boringssl/407): |name| should be const.
+// TODO(crbug.com/42290269): |name| should be const.
OPENSSL_EXPORT int X509_STORE_CTX_get_by_subject(X509_STORE_CTX *ctx, int type,
X509_NAME *name,
X509_OBJECT *ret);
