pkey: make PKey.read raise PKey::PKeyError rather than ArgumentErrortopic/pkey-read-pkey-error

PKey.read is a generic method to load an arbitrary PKey structure from a
PEM or DER encoded String. Each PKey classes's constructor also can load
from a String, but the behavior on error is different. While they raises
its own exception (are subclasses of PKey::PKeyError), PKey.read raises
ArgumentError.  [Bug #11774]

5 files changed, 14 insertions, 5 deletions

diff --git a/NEWS b/NEWS
index d25b7426..1ff36e23 100644
--- a/NEWS
+++ b/NEWS
@@ -35,6 +35,10 @@ Backward compatibility notes
 * RC4 cipher suites are removed from OpenSSL::SSL::SSLContext::DEFAULT_PARAMS.
   RC4 is now considered to be weak. [GH ruby/openssl#50]
 
+* OpenSSL::PKey.read raises OpenSSL::PKey::PKeyError instead of ArgumentError
+  for consistency with OpenSSL::PKey::{DH,DSA,RSA,EC}#new.
+  [Bug #11774] [GH ruby/openssl#55]
+
 Updates since Ruby 2.3
 ----------------------
 
@@ -79,6 +83,10 @@ Updates since Ruby 2.3
     OpenSSL::PKey::DSA#set_pqg, #set_key, OpenSSL::PKey::DH#set_pqg and #set_key
     are added.
 
+  - OpenSSL::PKey.read raises OpenSSL::PKey::PKeyError instead of ArgumentError
+    for consistency with OpenSSL::PKey::{DH,DSA,RSA,EC}#new.
+    [Bug #11774] [GH ruby/openssl#55]
+
 * OpenSSL::Random
 
   - OpenSSL::Random.pseudo_bytes is deprecated, and not defined when built with
diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c
index 4ae8e14e..ee0575d8 100644
--- a/ext/openssl/ossl_pkey.c
+++ b/ext/openssl/ossl_pkey.c
@@ -158,7 +158,8 @@ ossl_pkey_new_from_data(int argc, VALUE *argv, VALUE self)
 
     BIO_free(bio);
     if (!pkey)
-	ossl_raise(rb_eArgError, "Could not parse PKey");
+	ossl_raise(ePKeyError, "Could not parse PKey");
+
     return ossl_pkey_new(pkey);
 }
 
diff --git a/test/test_pkey_dsa.rb b/test/test_pkey_dsa.rb
index 9c29c034..522cdeed 100644
--- a/test/test_pkey_dsa.rb
+++ b/test/test_pkey_dsa.rb
@@ -221,7 +221,7 @@ YNMbNw==
   def test_export_password_funny
     key = OpenSSL::TestUtils::TEST_KEY_DSA256
     pem = key.export(OpenSSL::Cipher.new('AES-128-CBC'), "pass\0wd")
-    assert_raise(ArgumentError) do
+    assert_raise(OpenSSL::PKey::PKeyError) do
       OpenSSL::PKey.read(pem, "pass")
     end
     key2 = OpenSSL::PKey.read(pem, "pass\0wd")
diff --git a/test/test_pkey_ec.rb b/test/test_pkey_ec.rb
index 4498b2b8..bf2985a0 100644
--- a/test/test_pkey_ec.rb
+++ b/test/test_pkey_ec.rb
@@ -233,7 +233,7 @@ class OpenSSL::TestEC < OpenSSL::TestCase
   def test_export_password_funny
     key = OpenSSL::TestUtils::TEST_KEY_EC_P256V1
     pem = key.export(OpenSSL::Cipher.new('AES-128-CBC'), "pass\0wd")
-    assert_raise(ArgumentError) do
+    assert_raise(OpenSSL::PKey::PKeyError) do
       OpenSSL::PKey.read(pem, "pass")
     end
     key2 = OpenSSL::PKey.read(pem, "pass\0wd")
diff --git a/test/test_pkey_rsa.rb b/test/test_pkey_rsa.rb
index 49e8ceac..c062a6a4 100644
--- a/test/test_pkey_rsa.rb
+++ b/test/test_pkey_rsa.rb
@@ -260,7 +260,7 @@ AwEAAQ==
   def test_read_private_key_pem_pw_exception
     pem = OpenSSL::TestUtils::TEST_KEY_RSA1024.to_pem(OpenSSL::Cipher.new('AES-128-CBC'), 'secret')
     # it raises an ArgumentError from PEM reading. The exception raised inside are ignored for now.
-    assert_raise(ArgumentError) do
+    assert_raise(OpenSSL::PKey::PKeyError) do
       OpenSSL::PKey.read(pem) do
         raise RuntimeError
       end
@@ -285,7 +285,7 @@ AwEAAQ==
     end
     # password containing NUL byte
     pem = key.export(OpenSSL::Cipher.new('AES-128-CBC'), "pass\0wd")
-    assert_raise(ArgumentError) do
+    assert_raise(OpenSSL::PKey::PKeyError) do
       OpenSSL::PKey.read(pem, "pass")
     end
     key2 = OpenSSL::PKey.read(pem, "pass\0wd")