x509name: fix handling of X509_NAME_{oneline,print_ex}() return valueky/x509name-to-s-empty

X509_NAME_print_ex() behaves differently depending on the passed flags.
When XN_FLAG_COMPAT is specified, it returns either 1 on success or 0
on error. Otherwise, it returns the byte size written or -1 on error.
This means 0 return is not necessarily an error.

Also, X509_NAME_oneline() return value needs to be checked as it may
fail with a NULL return.

Fixes: https://github.com/ruby/openssl/issues/200

2 files changed, 34 insertions, 6 deletions

diff --git a/ext/openssl/ossl_x509name.c b/ext/openssl/ossl_x509name.c
index 16a1b257..f064cf65 100644
--- a/ext/openssl/ossl_x509name.c
+++ b/ext/openssl/ossl_x509name.c
@@ -239,14 +239,12 @@ ossl_x509name_to_s_old(VALUE self)
 {
     X509_NAME *name;
     char *buf;
-    VALUE str;
 
     GetX509Name(self, name);
     buf = X509_NAME_oneline(name, NULL, 0);
-    str = rb_str_new2(buf);
-    OPENSSL_free(buf);
-
-    return str;
+    if (!buf)
+	ossl_raise(eX509NameError, "X509_NAME_oneline");
+    return ossl_buf2str(buf, rb_long2int(strlen(buf)));
 }
 
 static VALUE
@@ -254,12 +252,14 @@ x509name_print(VALUE self, unsigned long iflag)
 {
     X509_NAME *name;
     BIO *out;
+    int ret;
 
     GetX509Name(self, name);
     out = BIO_new(BIO_s_mem());
     if (!out)
 	ossl_raise(eX509NameError, NULL);
-    if (!X509_NAME_print_ex(out, name, 0, iflag)) {
+    ret = X509_NAME_print_ex(out, name, 0, iflag);
+    if (ret < 0 || iflag == XN_FLAG_COMPAT && ret == 0) {
 	BIO_free(out);
 	ossl_raise(eX509NameError, "X509_NAME_print_ex");
     }
diff --git a/test/test_x509name.rb b/test/test_x509name.rb
index f7a60c3a..f8fabafe 100644
--- a/test/test_x509name.rb
+++ b/test/test_x509name.rb
@@ -322,6 +322,34 @@ class OpenSSL::TestX509Name < OpenSSL::TestCase
     assert_equal("Namiki", ary[5][1])
   end
 
+  def test_to_s
+    dn = [
+      ["DC", "org"],
+      ["DC", "ruby-lang"],
+      ["CN", "フー, バー"],
+    ]
+    name = OpenSSL::X509::Name.new
+    dn.each { |x| name.add_entry(*x) }
+
+    assert_equal "/DC=org/DC=ruby-lang/" \
+      "CN=\\xE3\\x83\\x95\\xE3\\x83\\xBC, \\xE3\\x83\\x90\\xE3\\x83\\xBC",
+      name.to_s
+    # OpenSSL escapes characters with MSB by default
+    assert_equal \
+      "CN=\\E3\\83\\95\\E3\\83\\BC\\, \\E3\\83\\90\\E3\\83\\BC," \
+      "DC=ruby-lang,DC=org",
+      name.to_s(OpenSSL::X509::Name::RFC2253)
+    assert_equal "DC = org, DC = ruby-lang, " \
+      "CN = \"\\E3\\83\\95\\E3\\83\\BC, \\E3\\83\\90\\E3\\83\\BC\"",
+      name.to_s(OpenSSL::X509::Name::ONELINE)
+
+    empty = OpenSSL::X509::Name.new
+    assert_equal "", empty.to_s
+    assert_equal "", empty.to_s(OpenSSL::X509::Name::COMPAT)
+    assert_equal "", empty.to_s(OpenSSL::X509::Name::RFC2253)
+    assert_equal "", empty.to_s(OpenSSL::X509::Name::ONELINE)
+  end
+
   def test_equals2
     n1 = OpenSSL::X509::Name.parse 'CN=a'
     n2 = OpenSSL::X509::Name.parse 'CN=a'