src: deduplicate X509 getter implementations

Reorder arguments of internal helper functions such that their order is consistent across X509 property getters. Add ReturnPropertyThroughBIO() and ReturnProperty(). Use these new helpers to deduplicate code across various X509 property getters. PR-URL: #48563 Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Alba Mendez <[email protected]>
nodejs · Jul 13, 2023 · df363d0 · df363d0
1 parent 08c6287
commit df363d0
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 100 deletions.
diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc
@@ -492,7 +492,7 @@ MaybeLocal<Value> GetModulusString(
 }
 }  // namespace
 
-MaybeLocal<Object> GetRawDERCertificate(Environment* env, X509* cert) {
+MaybeLocal<Value> GetRawDERCertificate(Environment* env, X509* cert) {
   int size = i2d_X509(cert, nullptr);
 
   std::unique_ptr<BackingStore> bs;
@@ -872,10 +872,9 @@ bool SafeX509InfoAccessPrint(const BIOPointer& out, X509_EXTENSION* ext) {
   return ok;
 }
 
-v8::MaybeLocal<v8::Value> GetSubjectAltNameString(
-    Environment* env,
-    const BIOPointer& bio,
-    X509* cert) {
+v8::MaybeLocal<v8::Value> GetSubjectAltNameString(Environment* env,
+                                                  X509* cert,
+                                                  const BIOPointer& bio) {
   int index = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1);
   if (index < 0)
     return Undefined(env->isolate());
@@ -891,10 +890,9 @@ v8::MaybeLocal<v8::Value> GetSubjectAltNameString(
   return ToV8Value(env, bio);
 }
 
-v8::MaybeLocal<v8::Value> GetInfoAccessString(
-    Environment* env,
-    const BIOPointer& bio,
-    X509* cert) {
+v8::MaybeLocal<v8::Value> GetInfoAccessString(Environment* env,
+                                              X509* cert,
+                                              const BIOPointer& bio) {
   int index = X509_get_ext_by_NID(cert, NID_info_access, -1);
   if (index < 0)
     return Undefined(env->isolate());
@@ -910,10 +908,9 @@ v8::MaybeLocal<v8::Value> GetInfoAccessString(
   return ToV8Value(env, bio);
 }
 
-MaybeLocal<Value> GetIssuerString(
-    Environment* env,
-    const BIOPointer& bio,
-    X509* cert) {
+MaybeLocal<Value> GetIssuerString(Environment* env,
+                                  X509* cert,
+                                  const BIOPointer& bio) {
   X509_NAME* issuer_name = X509_get_issuer_name(cert);
   if (X509_NAME_print_ex(
           bio.get(),
@@ -927,10 +924,9 @@ MaybeLocal<Value> GetIssuerString(
   return ToV8Value(env, bio);
 }
 
-MaybeLocal<Value> GetSubject(
-    Environment* env,
-    const BIOPointer& bio,
-    X509* cert) {
+MaybeLocal<Value> GetSubject(Environment* env,
+                             X509* cert,
+                             const BIOPointer& bio) {
   if (X509_NAME_print_ex(
           bio.get(),
           X509_get_subject_name(cert),
@@ -1283,11 +1279,11 @@ MaybeLocal<Object> X509ToObject(
       !Set<Value>(context,
                   info,
                   env->subjectaltname_string(),
-                  GetSubjectAltNameString(env, bio, cert)) ||
+                  GetSubjectAltNameString(env, cert, bio)) ||
       !Set<Value>(context,
                   info,
                   env->infoaccess_string(),
-                  GetInfoAccessString(env, bio, cert)) ||
+                  GetInfoAccessString(env, cert, bio)) ||
       !Set<Boolean>(context, info, env->ca_string(), is_ca)) {
     return MaybeLocal<Object>();
   }
@@ -1390,18 +1386,14 @@ MaybeLocal<Object> X509ToObject(
                   info,
                   env->fingerprint512_string(),
                   GetFingerprintDigest(env, EVP_sha512(), cert)) ||
-      !Set<Value>(context,
-                  info,
-                  env->ext_key_usage_string(),
-                  GetKeyUsage(env, cert)) ||
+      !Set<Value>(
+          context, info, env->ext_key_usage_string(), GetKeyUsage(env, cert)) ||
       !Set<Value>(context,
                   info,
                   env->serial_number_string(),
                   GetSerialNumber(env, cert)) ||
-      !Set<Object>(context,
-                   info,
-                   env->raw_string(),
-                   GetRawDERCertificate(env, cert))) {
+      !Set<Value>(
+          context, info, env->raw_string(), GetRawDERCertificate(env, cert))) {
     return MaybeLocal<Object>();
   }
 

diff --git a/src/crypto/crypto_common.h b/src/crypto/crypto_common.h
@@ -118,30 +118,26 @@ v8::MaybeLocal<v8::Value> GetCurrentCipherVersion(Environment* env,
 
 v8::MaybeLocal<v8::Value> GetSerialNumber(Environment* env, X509* cert);
 
-v8::MaybeLocal<v8::Object> GetRawDERCertificate(Environment* env, X509* cert);
+v8::MaybeLocal<v8::Value> GetRawDERCertificate(Environment* env, X509* cert);
 
 v8::Local<v8::Value> ToV8Value(Environment* env, const BIOPointer& bio);
 bool SafeX509SubjectAltNamePrint(const BIOPointer& out, X509_EXTENSION* ext);
 
-v8::MaybeLocal<v8::Value> GetSubject(
-    Environment* env,
-    const BIOPointer& bio,
-    X509* cert);
+v8::MaybeLocal<v8::Value> GetSubject(Environment* env,
+                                     X509* cert,
+                                     const BIOPointer& bio);
 
-v8::MaybeLocal<v8::Value> GetIssuerString(
-    Environment* env,
-    const BIOPointer& bio,
-    X509* cert);
+v8::MaybeLocal<v8::Value> GetIssuerString(Environment* env,
+                                          X509* cert,
+                                          const BIOPointer& bio);
 
-v8::MaybeLocal<v8::Value> GetSubjectAltNameString(
-    Environment* env,
-    const BIOPointer& bio,
-    X509* cert);
+v8::MaybeLocal<v8::Value> GetSubjectAltNameString(Environment* env,
+                                                  X509* cert,
+                                                  const BIOPointer& bio);
 
-v8::MaybeLocal<v8::Value> GetInfoAccessString(
-    Environment* env,
-    const BIOPointer& bio,
-    X509* cert);
+v8::MaybeLocal<v8::Value> GetInfoAccessString(Environment* env,
+                                              X509* cert,
+                                              const BIOPointer& bio);
 
 }  // namespace crypto
 }  // namespace node

diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc
@@ -203,97 +203,62 @@ void X509Certificate::Parse(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(cert);
 }
 
-void X509Certificate::Subject(const FunctionCallbackInfo<Value>& args) {
+template <MaybeLocal<Value> Property(
+    Environment* env, X509* cert, const BIOPointer& bio)>
+static void ReturnPropertyThroughBIO(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
   BIOPointer bio(BIO_new(BIO_s_mem()));
   CHECK(bio);
   Local<Value> ret;
-  if (GetSubject(env, bio, cert->get()).ToLocal(&ret))
+  if (Property(env, cert->get(), bio).ToLocal(&ret))
     args.GetReturnValue().Set(ret);
 }
 
+void X509Certificate::Subject(const FunctionCallbackInfo<Value>& args) {
+  ReturnPropertyThroughBIO<GetSubject>(args);
+}
+
 void X509Certificate::Issuer(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  X509Certificate* cert;
-  ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
-  BIOPointer bio(BIO_new(BIO_s_mem()));
-  CHECK(bio);
-  Local<Value> ret;
-  if (GetIssuerString(env, bio, cert->get()).ToLocal(&ret))
-    args.GetReturnValue().Set(ret);
+  ReturnPropertyThroughBIO<GetIssuerString>(args);
 }
 
 void X509Certificate::SubjectAltName(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  X509Certificate* cert;
-  ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
-  BIOPointer bio(BIO_new(BIO_s_mem()));
-  CHECK(bio);
-  Local<Value> ret;
-  if (GetSubjectAltNameString(env, bio, cert->get()).ToLocal(&ret))
-    args.GetReturnValue().Set(ret);
+  ReturnPropertyThroughBIO<GetSubjectAltNameString>(args);
 }
 
 void X509Certificate::InfoAccess(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  X509Certificate* cert;
-  ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
-  BIOPointer bio(BIO_new(BIO_s_mem()));
-  CHECK(bio);
-  Local<Value> ret;
-  if (GetInfoAccessString(env, bio, cert->get()).ToLocal(&ret))
-    args.GetReturnValue().Set(ret);
+  ReturnPropertyThroughBIO<GetInfoAccessString>(args);
 }
 
 void X509Certificate::ValidFrom(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  X509Certificate* cert;
-  ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
-  BIOPointer bio(BIO_new(BIO_s_mem()));
-  CHECK(bio);
-  Local<Value> ret;
-  if (GetValidFrom(env, cert->get(), bio).ToLocal(&ret))
-    args.GetReturnValue().Set(ret);
+  ReturnPropertyThroughBIO<GetValidFrom>(args);
 }
 
 void X509Certificate::ValidTo(const FunctionCallbackInfo<Value>& args) {
+  ReturnPropertyThroughBIO<GetValidTo>(args);
+}
+
+template <MaybeLocal<Value> Property(Environment* env, X509* cert)>
+static void ReturnProperty(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
-  BIOPointer bio(BIO_new(BIO_s_mem()));
-  CHECK(bio);
   Local<Value> ret;
-  if (GetValidTo(env, cert->get(), bio).ToLocal(&ret))
-    args.GetReturnValue().Set(ret);
+  if (Property(env, cert->get()).ToLocal(&ret)) args.GetReturnValue().Set(ret);
 }
 
 void X509Certificate::KeyUsage(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  X509Certificate* cert;
-  ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
-  Local<Value> ret;
-  if (GetKeyUsage(env, cert->get()).ToLocal(&ret))
-    args.GetReturnValue().Set(ret);
+  ReturnProperty<GetKeyUsage>(args);
 }
 
 void X509Certificate::SerialNumber(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  X509Certificate* cert;
-  ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
-  Local<Value> ret;
-  if (GetSerialNumber(env, cert->get()).ToLocal(&ret))
-    args.GetReturnValue().Set(ret);
+  ReturnProperty<GetSerialNumber>(args);
 }
 
 void X509Certificate::Raw(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  X509Certificate* cert;
-  ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
-  Local<Value> ret;
-  if (GetRawDERCertificate(env, cert->get()).ToLocal(&ret))
-    args.GetReturnValue().Set(ret);
+  ReturnProperty<GetRawDERCertificate>(args);
 }
 
 void X509Certificate::PublicKey(const FunctionCallbackInfo<Value>& args) {