From b43fd8e0803a2a7796ebf944b3fa1f53b405d4c9 Mon Sep 17 00:00:00 2001 From: rhe Date: Wed, 18 May 2016 02:55:45 +0000 Subject: [PATCH] openssl: fix equality test methods of OpenSSL::BN * ext/openssl/ossl_bn.c (try_convert_to_bnptr): Extracted from GetBNPtr(). This doesn't raise exception but returns NULL on error. (GetBNPtr): Raise TypeError if conversion fails. (ossl_bn_eq): Implement BN#==. (ossl_bn_eql): #eql? should not raise TypeError even if the argument is not compatible with BN. (ossl_bn_hash): Implement BN#hash. * ext/openssl/ossl_bn.c (Init_ossl_bn): Define #== and #hash. * test/openssl/test_bn.rb: Test BN#eql?, #== and #hash git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@55047 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 14 +++++++ ext/openssl/ossl_bn.c | 90 +++++++++++++++++++++++++++++++++++------ test/openssl/test_bn.rb | 16 ++++++-- 3 files changed, 103 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index c3ffe672df..25d6d25176 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +Wed May 18 11:19:59 2016 Kazuki Yamaguchi + + * ext/openssl/ossl_bn.c (try_convert_to_bnptr): Extracted from + GetBNPtr(). This doesn't raise exception but returns NULL on error. + (GetBNPtr): Raise TypeError if conversion fails. + (ossl_bn_eq): Implement BN#==. + (ossl_bn_eql): #eql? should not raise TypeError even if the argument + is not compatible with BN. + (ossl_bn_hash): Implement BN#hash. + + * ext/openssl/ossl_bn.c (Init_ossl_bn): Define #== and #hash. + + * test/openssl/test_bn.rb: Test BN#eql?, #== and #hash + Wed May 18 10:17:41 2016 Nobuyoshi Nakada * include/ruby/ruby.h (RB_INTEGER_TYPE_P): new macro and diff --git a/ext/openssl/ossl_bn.c b/ext/openssl/ossl_bn.c index fba86cdd44..0af6c68466 100644 --- a/ext/openssl/ossl_bn.c +++ b/ext/openssl/ossl_bn.c @@ -82,8 +82,8 @@ ossl_bn_new(const BIGNUM *bn) return obj; } -BIGNUM * -GetBNPtr(VALUE obj) +static BIGNUM * +try_convert_to_bnptr(VALUE obj) { BIGNUM *bn = NULL; VALUE newobj; @@ -100,14 +100,20 @@ GetBNPtr(VALUE obj) } SetBN(newobj, bn); /* Handle potencial mem leaks */ break; - case T_NIL: - break; - default: - ossl_raise(rb_eTypeError, "Cannot convert into OpenSSL::BN"); } return bn; } +BIGNUM * +GetBNPtr(VALUE obj) +{ + BIGNUM *bn = try_convert_to_bnptr(obj); + if (!bn) + ossl_raise(rb_eTypeError, "Cannot convert into OpenSSL::BN"); + + return bn; +} + /* * Private */ @@ -841,18 +847,75 @@ BIGNUM_CMP(ucmp) /* * call-seq: - * big.eql?(obj) => true or false + * bn == obj => true or false + * + * Returns +true+ only if +obj+ has the same value as +bn+. Contrast this + * with OpenSSL::BN#eql?, which requires obj to be OpenSSL::BN. + */ +static VALUE +ossl_bn_eq(VALUE self, VALUE other) +{ + BIGNUM *bn1, *bn2; + + GetBN(self, bn1); + /* BNPtr may raise, so we can't use here */ + bn2 = try_convert_to_bnptr(other); + + if (bn2 && !BN_cmp(bn1, bn2)) { + return Qtrue; + } + return Qfalse; +} + +/* + * call-seq: + * bn.eql?(obj) => true or false * * Returns true only if obj is a - * Bignum with the same value as big. Contrast this + * OpenSSL::BN with the same value as big. Contrast this + * with OpenSSL::BN#==, which performs type conversions. */ static VALUE ossl_bn_eql(VALUE self, VALUE other) { - if (ossl_bn_cmp(self, other) == INT2FIX(0)) { - return Qtrue; + BIGNUM *bn1, *bn2; + + if (!rb_obj_is_kind_of(other, cBN)) + return Qfalse; + GetBN(self, bn1); + GetBN(other, bn2); + + return BN_cmp(bn1, bn2) ? Qfalse : Qtrue; +} + +/* + * call-seq: + * bn.hash => Integer + * + * Returns a hash code for this object. + * + * See also Object#hash. + */ +static VALUE +ossl_bn_hash(VALUE self) +{ + BIGNUM *bn; + VALUE hash; + unsigned char *buf; + int len; + + GetBN(self, bn); + len = BN_num_bytes(bn); + buf = xmalloc(len); + if (BN_bn2bin(bn, buf) != len) { + xfree(buf); + ossl_raise(eBNError, NULL); } - return Qfalse; + + hash = INT2FIX(rb_memhash(buf, len)); + xfree(buf); + + return hash; } /* @@ -982,8 +1045,9 @@ Init_ossl_bn(void) rb_define_alias(cBN, "<=>", "cmp"); rb_define_method(cBN, "ucmp", ossl_bn_ucmp, 1); rb_define_method(cBN, "eql?", ossl_bn_eql, 1); - rb_define_alias(cBN, "==", "eql?"); - rb_define_alias(cBN, "===", "eql?"); + rb_define_method(cBN, "hash", ossl_bn_hash, 0); + rb_define_method(cBN, "==", ossl_bn_eq, 1); + rb_define_alias(cBN, "===", "=="); rb_define_method(cBN, "zero?", ossl_bn_is_zero, 0); rb_define_method(cBN, "one?", ossl_bn_is_one, 0); /* is_word */ diff --git a/test/openssl/test_bn.rb b/test/openssl/test_bn.rb index 848d1feb43..415bd74c79 100644 --- a/test/openssl/test_bn.rb +++ b/test/openssl/test_bn.rb @@ -43,10 +43,18 @@ class OpenSSL::TestBN < Test::Unit::TestCase assert_equal(true, OpenSSL::BN.new((2 ** 127 - 1).to_s(16), 16).prime?(1)) end - def test_cmp_nil - bn = OpenSSL::BN.new('1') - assert_equal(false, bn == nil) - assert_equal(true, bn != nil) + def test_cmp + bn1 = OpenSSL::BN.new('1') + bn2 = OpenSSL::BN.new('1') + bn3 = OpenSSL::BN.new('2') + assert_equal(false, bn1 == nil) + assert_equal(true, bn1 != nil) + assert_equal(true, bn1 == bn2) + assert_equal(false, bn1 == bn3) + assert_equal(true, bn1.eql?(bn2)) + assert_equal(false, bn1.eql?(bn3)) + assert_equal(bn1.hash, bn2.hash) + assert_not_equal(bn3.hash, bn1.hash) end end