ruby-ldap icon indicating copy to clipboard operation
ruby-ldap copied to clipboard

Ruby 1.8.X Breaking Change: rb_hash_dup

Open Nejuf opened this issue 9 years ago • 10 comments

@@ -144,21 +174,13 @@ rb_ldap_entry_get_attributes (VALUE self)
 VALUE
 rb_ldap_entry_to_hash (VALUE self)
 {
-  VALUE attrs = rb_ldap_entry_get_attributes (self);
-  VALUE hash = rb_hash_new ();
-  VALUE attr, vals;
-  int i;
-
-  Check_Type (attrs, T_ARRAY);
-  rb_hash_aset (hash, rb_tainted_str_new2 ("dn"),
-               rb_ary_new3 (1, rb_ldap_entry_get_dn (self)));
-  for (i = 0; i < RARRAY_LEN (attrs); i++)
-    {
-      attr = rb_ary_entry (attrs, i);
-      vals = rb_ldap_entry_get_values (self, attr);
-      rb_hash_aset (hash, attr, vals);
-    }
+  RB_LDAPENTRY_DATA *edata;
+  VALUE hash, dn_ary;

+  GET_LDAPENTRY_DATA (self, edata);
+  hash = rb_hash_dup(edata->attr);
+  dn_ary = rb_ary_new3(1, edata->dn);
+  rb_hash_aset(hash, rb_tainted_str_new2("dn"), dn_ary);
   return hash;

v0.9.17 #32 introduced a breaking change for Ruby 1.8.X. The rb_hash_dup method was not added until Ruby 1.9. Below is the resulting error.

symbol lookup error: /opt/ruby-enterprise-1.8.7-2011.03/lib/ruby/gems/1.8/gems/ruby-ldap-0.9.17/lib/ldap.so: undefined symbol: rb_hash_dup

Nejuf avatar Feb 27 '16 02:02 Nejuf

Could you please try version from branch gh-35?

bearded avatar Feb 28 '16 17:02 bearded

Thanks for the fast response. I think the only system I can test the fix on is a production server, so it could be 1-3 weeks before I can confirm.

Nejuf avatar Mar 03 '16 02:03 Nejuf

@hsuenaga, could you please take a look into this problem?

bearded avatar Mar 03 '16 08:03 bearded

@Nejuf, it may not work at all, be prepared :)

bearded avatar Mar 03 '16 20:03 bearded

I'm unable to reproduce the problem when compiling against master running ree-1.8.7-p2011.03 (installed from deb package) on Ubuntu Trusty 14.04. For grins, I successfully installed branch gh-35 as well.

@Nejuf can you include the OS and ruby patchlevel?

As an aside, there's a newer version ree-1.8.7-2012.02 available that you could maybe test with? Would seem wise to use the latest (and more secure) versions of 1.8.7, especially in production.

dacamp avatar Mar 03 '16 22:03 dacamp

I don't know about ruby 1.8.X internal APIs. I will create a patch to back out #32 on old ruby systems to prevent compilation errors. Please wait for 2 or 3 days.

Finally, the system stack problem described in #32 must be fixed, but it needs more time to learn the ruby 1.8.X...

hsuenaga avatar Mar 04 '16 02:03 hsuenaga

I sent PR #36 to fix compile error only. The native extension can be compiled using ruby 1.8.x, but the extension has the system stack problem if you are using ruby 1.8.x or lower. If you are using ruby 1.9.x or higher, nothing changed.

I will learn the ruby 1.8.x internal APIs, but it sould takes long time. If you want to fix the compile error quickly, please apply the PR #36. If you can wait for the final fix, you can ignore the PR #36.

hsuenaga avatar Mar 04 '16 10:03 hsuenaga

PR #36 is released as version 0.9.18.

bearded avatar Mar 04 '16 11:03 bearded

Thanks, guys! I tried branch gh-35 and it worked. Sorry, I hadn't checked my notifications in a while and didn't realize you had another version to test with. When we next update the server, I'll test v0.9.18.

Nejuf avatar Mar 18 '16 17:03 Nejuf

@Nejuf, this issue may be closed?

bearded avatar Jun 20 '16 19:06 bearded