Merge pull request #792 from rhenium/ky/asn1-obj-fix-eq

asn1: fix ObjectId#==
ruby · Sep 5, 2024 · 1b5d41f · 1b5d41f
2 parents c96e666 + c67e16e
commit 1b5d41f
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 29 deletions.
diff --git a/ext/openssl/ossl_asn1.c b/ext/openssl/ossl_asn1.c
@@ -1297,30 +1297,6 @@ ossl_asn1obj_get_ln(VALUE self)
     return ret;
 }
 
-/*
- *  call-seq:
- *     oid == other_oid => true or false
- *
- *  Returns +true+ if _other_oid_ is the same as _oid_
- */
-static VALUE
-ossl_asn1obj_eq(VALUE self, VALUE other)
-{
-    VALUE valSelf, valOther;
-    int nidSelf, nidOther;
-
-    valSelf = ossl_asn1_get_value(self);
-    valOther = ossl_asn1_get_value(other);
-
-    if ((nidSelf = OBJ_txt2nid(StringValueCStr(valSelf))) == NID_undef)
-	ossl_raise(eASN1Error, "OBJ_txt2nid");
-
-    if ((nidOther = OBJ_txt2nid(StringValueCStr(valOther))) == NID_undef)
-	ossl_raise(eASN1Error, "OBJ_txt2nid");
-
-    return nidSelf == nidOther ? Qtrue : Qfalse;
-}
-
 static VALUE
 asn1obj_get_oid_i(VALUE vobj)
 {
@@ -1365,6 +1341,25 @@ ossl_asn1obj_get_oid(VALUE self)
     return str;
 }
 
+/*
+ *  call-seq:
+ *     oid == other_oid => true or false
+ *
+ *  Returns +true+ if _other_oid_ is the same as _oid_.
+ */
+static VALUE
+ossl_asn1obj_eq(VALUE self, VALUE other)
+{
+    VALUE oid1, oid2;
+
+    if (!rb_obj_is_kind_of(other, cASN1ObjectId))
+        return Qfalse;
+
+    oid1 = ossl_asn1obj_get_oid(self);
+    oid2 = ossl_asn1obj_get_oid(other);
+    return rb_str_equal(oid1, oid2);
+}
+
 #define OSSL_ASN1_IMPL_FACTORY_METHOD(klass) \
 static VALUE ossl_asn1_##klass(int argc, VALUE *argv, VALUE self)\
 { return rb_funcall3(cASN1##klass, rb_intern("new"), argc, argv); }

diff --git a/test/openssl/test_asn1.rb b/test/openssl/test_asn1.rb
@@ -331,7 +331,9 @@ def test_object_identifier
       pend "OBJ_obj2txt() not working (LibreSSL?)" if $!.message =~ /OBJ_obj2txt/
       raise
     end
+  end
 
+  def test_object_identifier_equality
     aki = [
       OpenSSL::ASN1::ObjectId.new("authorityKeyIdentifier"),
       OpenSSL::ASN1::ObjectId.new("X509v3 Authority Key Identifier"),
@@ -346,17 +348,22 @@ def test_object_identifier
 
     aki.each do |a|
       aki.each do |b|
-        assert a == b
+        assert_equal true, a == b
       end
 
       ski.each do |b|
-        refute a == b
+        assert_equal false, a == b
       end
     end
 
-    assert_raise(TypeError) {
-      OpenSSL::ASN1::ObjectId.new("authorityKeyIdentifier") == nil
-    }
+    obj1 = OpenSSL::ASN1::ObjectId.new("1.2.34.56789.10")
+    obj2 = OpenSSL::ASN1::ObjectId.new("1.2.34.56789.10")
+    obj3 = OpenSSL::ASN1::ObjectId.new("1.2.34.56789.11")
+    omit "OID 1.2.34.56789.10 is registered" if obj1.sn
+    assert_equal true, obj1 == obj2
+    assert_equal false, obj1 == obj3
+
+    assert_equal false, OpenSSL::ASN1::ObjectId.new("authorityKeyIdentifier") == nil
   end
 
   def test_sequence