Merge pull request #976 from rhenium/ky/err-extras

ossl.c: implement OpenSSL::OpenSSLError#detailed_message

Author: rhenium

ext/openssl/extconf.rb

@@ -38,8 +38,12 @@
 
 $defs.push("-D""OPENSSL_SUPPRESS_DEPRECATED")
 
+# Missing in TruffleRuby
+have_func("rb_call_super_kw(0, NULL, 0)", "ruby.h")
+# Ruby 3.1
 have_func("rb_io_descriptor", "ruby/io.h")
-have_func("rb_io_maybe_wait(0, Qnil, Qnil, Qnil)", "ruby/io.h") # Ruby 3.1
+have_func("rb_io_maybe_wait(0, Qnil, Qnil, Qnil)", "ruby/io.h")
+# Ruby 3.2
 have_func("rb_io_timeout", "ruby/io.h")
 
 Logging::message "=== Checking for system dependent stuff... ===\n"

ext/openssl/ossl.c

@@ -254,12 +254,17 @@ ossl_to_der_if_possible(VALUE obj)
 /*
  * Errors
  */
+static ID id_i_errors;
+
+static void collect_errors_into(VALUE ary);
+
 VALUE
 ossl_make_error(VALUE exc, VALUE str)
 {
     unsigned long e;
     const char *data;
     int flags;
+    VALUE errors = rb_ary_new();
 
     if (NIL_P(str))
         str = rb_str_new(NULL, 0);
@@ -276,10 +281,12 @@ ossl_make_error(VALUE exc, VALUE str)
         rb_str_cat_cstr(str, msg ? msg : "(null)");
         if (flags & ERR_TXT_STRING && data)
             rb_str_catf(str, " (%s)", data);
-        ossl_clear_error();
+        collect_errors_into(errors);
     }
 
-    return rb_exc_new_str(exc, str);
+    VALUE obj = rb_exc_new_str(exc, str);
+    rb_ivar_set(obj, id_i_errors, errors);
+    return obj;
 }
 
 void
@@ -300,13 +307,12 @@ ossl_raise(VALUE exc, const char *fmt, ...)
     rb_exc_raise(ossl_make_error(exc, err));
 }
 
-void
-ossl_clear_error(void)
+static void
+collect_errors_into(VALUE ary)
 {
-    if (dOSSL == Qtrue) {
+    if (dOSSL == Qtrue || !NIL_P(ary)) {
         unsigned long e;
         const char *file, *data, *func, *lib, *reason;
-        char append[256] = "";
         int line, flags;
 
 #ifdef HAVE_ERR_GET_ERROR_ALL
@@ -318,20 +324,66 @@ ossl_clear_error(void)
             lib = ERR_lib_error_string(e);
             reason = ERR_reason_error_string(e);
 
+            VALUE str = rb_sprintf("error:%08lX:%s:%s:%s", e, lib ? lib : "",
+                                   func ? func : "", reason ? reason : "");
             if (flags & ERR_TXT_STRING) {
                 if (!data)
                     data = "(null)";
-                snprintf(append, sizeof(append), " (%s)", data);
+                rb_str_catf(str, " (%s)", data);
             }
-            rb_warn("error on stack: error:%08lX:%s:%s:%s%s", e, lib ? lib : "",
-                    func ? func : "", reason ? reason : "", append);
+
+            if (dOSSL == Qtrue)
+                rb_warn("error on stack: %"PRIsVALUE, str);
+            if (!NIL_P(ary))
+                rb_ary_push(ary, str);
         }
     }
     else {
         ERR_clear_error();
     }
 }
 
+void
+ossl_clear_error(void)
+{
+    collect_errors_into(Qnil);
+}
+
+/*
+ * call-seq:
+ *    ossl_error.detailed_message(**) -> string
+ *
+ * Returns the exception message decorated with the captured \OpenSSL error
+ * queue entries.
+ */
+static VALUE
+osslerror_detailed_message(int argc, VALUE *argv, VALUE self)
+{
+    VALUE str;
+#ifdef HAVE_RB_CALL_SUPER_KW
+    // Ruby >= 3.2
+    if (RTEST(rb_funcall(rb_eException, rb_intern("method_defined?"), 1,
+                         ID2SYM(rb_intern("detailed_message")))))
+        str = rb_call_super_kw(argc, argv, RB_PASS_CALLED_KEYWORDS);
+    else
+#endif
+        str = rb_funcall(self, rb_intern("message"), 0);
+    VALUE errors = rb_attr_get(self, id_i_errors);
+
+    // OpenSSLError was not created by ossl_make_error()
+    if (!RB_TYPE_P(errors, T_ARRAY))
+        return str;
+
+    str = rb_str_resurrect(str);
+    rb_str_catf(str, "\nOpenSSL error queue reported %ld errors:",
+                RARRAY_LEN(errors));
+    for (long i = 0; i < RARRAY_LEN(errors); i++) {
+        VALUE err = RARRAY_AREF(errors, i);
+        rb_str_catf(str, "\n%"PRIsVALUE, err);
+    }
+    return str;
+}
+
 /*
  * call-seq:
  *   OpenSSL.errors -> [String...]
@@ -1009,10 +1061,26 @@ Init_openssl(void)
 
     rb_global_variable(&eOSSLError);
     /*
-     * Generic error,
-     * common for all classes under OpenSSL module
+     * Generic error class for OpenSSL. All error classes in this library
+     * inherit from this class.
+     *
+     * This class indicates that an error was reported by the underlying
+     * \OpenSSL library.
+     */
+    eOSSLError = rb_define_class_under(mOSSL, "OpenSSLError", rb_eStandardError);
+    /*
+     * \OpenSSL error queue entries captured at the time the exception was
+     * raised. The same information is printed to stderr if OpenSSL.debug is
+     * set to +true+.
+     *
+     * This is an array of zero or more strings, ordered from the oldest to the
+     * newest. The format of the strings is not stable and may vary across
+     * versions of \OpenSSL or versions of this Ruby extension.
+     *
+     * See also the man page ERR_get_error(3).
      */
-    eOSSLError = rb_define_class_under(mOSSL,"OpenSSLError",rb_eStandardError);
+    rb_attr(eOSSLError, rb_intern_const("errors"), 1, 0, 0);
+    rb_define_method(eOSSLError, "detailed_message", osslerror_detailed_message, -1);
 
     /*
      * Init debug core
@@ -1028,6 +1096,7 @@ Init_openssl(void)
      * Get ID of to_der
      */
     ossl_s_to_der = rb_intern("to_der");
+    id_i_errors = rb_intern("@errors");
 
     /*
      * Init components

test/openssl/test_ossl.rb

@@ -66,16 +66,27 @@ def test_memcmp_timing
   end if ENV["OSSL_TEST_ALL"] == "1"
 
   def test_error_data
-    # X509V3_EXT_nconf_nid() called from OpenSSL::X509::ExtensionFactory#create_ext is a function
-    # that uses ERR_raise_data() to append additional information about the error.
+    # X509V3_EXT_nconf_nid() called from
+    # OpenSSL::X509::ExtensionFactory#create_ext is a function that uses
+    # ERR_raise_data() to append additional information about the error.
     #
     # The generated message should look like:
     #     "subjectAltName = IP:not.a.valid.ip.address: bad ip address (value=not.a.valid.ip.address)"
     #     "subjectAltName = IP:not.a.valid.ip.address: error in extension (name=subjectAltName, value=IP:not.a.valid.ip.address)"
+    #
+    # The string inside parentheses is the ERR_TXT_STRING data, and is appended
+    # by ossl_make_error(), so we check it here.
     ef = OpenSSL::X509::ExtensionFactory.new
-    assert_raise_with_message(OpenSSL::X509::ExtensionError, /value=(IP:)?not.a.valid.ip.address\)/) {
+    e = assert_raise(OpenSSL::X509::ExtensionError) {
       ef.create_ext("subjectAltName", "IP:not.a.valid.ip.address")
     }
+    assert_match(/not.a.valid.ip.address\)\z/, e.message)
+
+    # We currently craft the strings based on ERR_error_string()'s style:
+    #     error:<error code in hex>:<library>:<function>:<reason> (data)
+    assert_instance_of(Array, e.errors)
+    assert_match(/\Aerror:.*not.a.valid.ip.address\)\z/, e.errors.last)
+    assert_include(e.detailed_message, "not.a.valid.ip.address")
   end
 end