Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String#unpack1 returns an array instead of a single string #6134

Closed
lopopolo opened this issue Dec 29, 2023 · 2 comments
Closed

String#unpack1 returns an array instead of a single string #6134

lopopolo opened this issue Dec 29, 2023 · 2 comments

Comments

@lopopolo
Copy link
Contributor

Reproducer

MRI 3.3.0

$ irb
[3.3.0] > puts RUBY_VERSION, RUBY_ENGINE
3.3.0
ruby
=> nil
[3.3.0] > "U2VuZCByZWluZm9yY2VtZW50cw==\n".unpack1("m")
=> "Send reinforcements"

mruby 3.2.0

$ ./bin/mirb
mirb - Embeddable Interactive Ruby Shell

> puts RUBY_VERSION, RUBY_ENGINE
3.2
mruby
 => nil
> "U2VuZCByZWluZm9yY2VtZW50cw==\n".unpack1("m")
 => ["Send reinforcements"]

Issue

The hex decode ends with a continue:

    case PACK_DIR_BASE64:
      srcidx += unpack_base64(mrb, sptr, srclen - srcidx, result);
      continue;

which brings it to the top of the while loop. has_tmpl returns false and the if (single) block which is inside the while loop is never executed.

Fix

$ g diff
diff --git i/artichoke-backend/vendor/mruby/mrbgems/mruby-pack/src/pack.c w/artichoke-backend/vendor/mruby/mrbgems/mruby-pack/src/pack.c
index b5ecdc6479..ab51662d48 100644
--- i/mrbgems/mruby-pack/src/pack.c
+++ w/mrbgems/mruby-pack/src/pack.c
@@ -1575,12 +1575,12 @@ pack_unpack(mrb_state *mrb, mrb_value str, int single)
         count--;
       }
     }
-    if (single) {
-      if (RARRAY_LEN(result) > 0) {
-        return RARRAY_PTR(result)[0];
-      }
-      return mrb_nil_value();
+  }
+  if (single) {
+    if (RARRAY_LEN(result) > 0) {
+      return RARRAY_PTR(result)[0];
     }
+    return mrb_nil_value();
   }
   return result;
 }
@matz matz closed this as completed in 6d8323f Dec 29, 2023
@lopopolo
Copy link
Contributor Author

thanks for taking the patch @matz. I'd also appreciate it if you could add the reproducer I provided as a test. I have frequently had issues with mruby-pack when taking updates to mruby.

@matz
Copy link
Member

matz commented Dec 29, 2023

OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants