Use rb_protect and Ruby internals to generate strings

Author: anakinj

ext/openssl/ossl_pkey.c

@@ -471,13 +471,14 @@ static const struct pkey_from_data_alias fcc_aliases[] = {
 };
 
 struct pkey_from_data_arg {
+    VALUE options;
     OSSL_PARAM_BLD *param_bld;
     const OSSL_PARAM *settable_params;
     const struct pkey_from_data_alias *aliases;
 };
 
 static int
-add_parameter_to_builder(VALUE key, VALUE value, VALUE arg) {
+add_data_to_builder(VALUE key, VALUE value, VALUE arg) {
     if(NIL_P(value))
         return ST_CONTINUE;
 
@@ -500,55 +501,53 @@ add_parameter_to_builder(VALUE key, VALUE value, VALUE arg) {
             case OSSL_PARAM_INTEGER:
             case OSSL_PARAM_UNSIGNED_INTEGER:
                 if(!OSSL_PARAM_BLD_push_BN(params->param_bld, key_ptr, GetBNPtr(value))) {
-                    OSSL_PARAM_BLD_free(params->param_bld);
                     ossl_raise(ePKeyError, "OSSL_PARAM_BLD_push_BN");
                 }
                 break;
             case OSSL_PARAM_UTF8_STRING:
                 StringValue(value);
                 if(!OSSL_PARAM_BLD_push_utf8_string(params->param_bld, key_ptr, RSTRING_PTR(value), RSTRING_LENINT(value))) {
-                    OSSL_PARAM_BLD_free(params->param_bld);
                     ossl_raise(ePKeyError, "OSSL_PARAM_BLD_push_utf8_string");
                 }
                 break;
 
             case OSSL_PARAM_OCTET_STRING:
                 StringValue(value);
                 if(!OSSL_PARAM_BLD_push_octet_string(params->param_bld, key_ptr, RSTRING_PTR(value), RSTRING_LENINT(value))) {
-                    OSSL_PARAM_BLD_free(params->param_bld);
                     ossl_raise(ePKeyError, "OSSL_PARAM_BLD_push_octet_string");
                 }
                 break;
             case OSSL_PARAM_UTF8_PTR:
             case OSSL_PARAM_OCTET_PTR:
-                OSSL_PARAM_BLD_free(params->param_bld);
                 ossl_raise(ePKeyError, "Unsupported parameter \"%s\", handling of OSSL_PARAM_UTF8_PTR and OSSL_PARAM_OCTET_PTR not implemented", key_ptr);
                 break;
             }
 
             return ST_CONTINUE;
         }
     }
-    OSSL_PARAM_BLD_free(params->param_bld);
 
-    char message_buffer[512] = { 0 };
-    char *cur = message_buffer;
-    char *end = message_buffer + sizeof(message_buffer);
+    VALUE supported_parameters = rb_ary_new();
+
     for (const OSSL_PARAM *settable_params = params->settable_params; settable_params->key != NULL; settable_params++) {
-        const char *fmt = cur == message_buffer ? "%s" : ", %s";
-        if (cur > end)
-            break;
-        cur += snprintf(cur, end-cur, fmt, settable_params->key);
+        rb_ary_push(supported_parameters, rb_str_new_cstr(settable_params->key));
     }
 
     for(int i = 0; strlen(params->aliases[i].alias) > 0; i++) {
-        const char *fmt = cur == message_buffer ? "%s" : ", %s";
-        if (cur > end)
-            break;
-        cur += snprintf(cur, end-cur, fmt, params->aliases[i].alias);
+        rb_ary_push(supported_parameters, rb_str_new_cstr(params->aliases[i].alias));
     }
 
-    ossl_raise(ePKeyError, "Invalid parameter \"%s\". Supported parameters: \"%s\"", key_ptr, message_buffer);
+    ossl_raise(ePKeyError, "Invalid parameter \"%s\". Supported parameters: %"PRIsVALUE, key_ptr, rb_ary_join(supported_parameters, rb_str_new2(", ")));
+}
+
+static VALUE
+add_data_to_builder_protected(VALUE value)
+{
+    struct pkey_from_data_arg *args = (void *)value;
+
+    rb_hash_foreach(args->options, &add_data_to_builder, (VALUE) args);
+
+    return Qnil;
 }
 
 static VALUE
@@ -567,6 +566,7 @@ pkey_from_data(int argc, VALUE *argv, VALUE self)
     struct pkey_from_data_arg from_data_args = { 0 };
 
     from_data_args.param_bld = OSSL_PARAM_BLD_new();
+    from_data_args.options = options;
 
     if (from_data_args.param_bld == NULL) {
         EVP_PKEY_CTX_free(ctx);
@@ -577,6 +577,7 @@ pkey_from_data(int argc, VALUE *argv, VALUE self)
 
     if (from_data_args.settable_params == NULL) {
         EVP_PKEY_CTX_free(ctx);
+        OSSL_PARAM_BLD_free(from_data_args.param_bld);
         ossl_raise(ePKeyError, "EVP_PKEY_fromdata_settable");
     }
 
@@ -585,7 +586,14 @@ pkey_from_data(int argc, VALUE *argv, VALUE self)
     else
         from_data_args.aliases = fcc_aliases;
 
-    rb_hash_foreach(options, &add_parameter_to_builder, (VALUE) &from_data_args);
+    int state;
+    rb_protect(add_data_to_builder_protected, (VALUE) &from_data_args, &state);
+
+    if(state) {
+        EVP_PKEY_CTX_free(ctx);
+        OSSL_PARAM_BLD_free(from_data_args.param_bld);
+        rb_jump_tag(state);
+    }
 
     OSSL_PARAM *params = OSSL_PARAM_BLD_to_param(from_data_args.param_bld);
     OSSL_PARAM_BLD_free(from_data_args.param_bld);
@@ -612,6 +620,7 @@ pkey_from_data(int argc, VALUE *argv, VALUE self)
 
     return ossl_pkey_new(pkey);
 }
+
 #endif
 
 /*
@@ -686,11 +695,11 @@ ossl_pkey_s_generate_key(int argc, VALUE *argv, VALUE self)
 static VALUE
 ossl_pkey_s_from_data(int argc, VALUE *argv, VALUE self)
 {
-    #if OSSL_OPENSSL_PREREQ(3, 0, 0)
+#if OSSL_OPENSSL_PREREQ(3, 0, 0)
     return pkey_from_data(argc, argv, self);
-    #else
+#else
     rb_raise(ePKeyError, "OpenSSL::PKey.from_data requires OpenSSL 3.0 or later");
-    #endif
+#endif
 }
 
 /*

test/openssl/test_pkey.rb

@@ -413,7 +413,7 @@ def test_s_from_data_dsa_with_gem_specific_keys
     end
 
     def test_s_from_data_dsa_with_invalid_parameter
-      assert_raise_with_message(OpenSSL::PKey::PKeyError, /Invalid parameter "invalid"/) do
+      assert_raise_with_message(OpenSSL::PKey::PKeyError, /Invalid parameter "invalid". Supported parameters: p, q, g, j/) do
         OpenSSL::PKey.from_data("DSA", invalid: 1234)
       end
     end