Critical bug in `scan_args::get_kwargs`

[gjtorikian]

Sorry for the bombastic title, but I think the severity of the bug I've encountered is pretty high.

Essentially, it seems that get_kwargs is not converting Values correctly. Given the following Ruby code:

MagnusKwargsBug::Selector.new(match_text_within: "foo")

Backed by the following arg parsing in Magnus:

    #[allow(clippy::let_unit_value)]
    fn scan_parse_args(args: &[Value]) -> Result<(Option<String>, Option<String>), Error> {
        let args = scan_args::scan_args(args)?;
        let _: () = args.required;
        let _: () = args.optional;
        let _: () = args.splat;
        let _: () = args.trailing;
        let _: () = args.block;

        let kw = scan_args::get_kwargs::<_, (), (Option<String>, Option<String>), ()>(
            args.keywords,
            &[],
            &["match_element", "match_text_within"],
        )?;
        let (match_element, match_text_within) = kw.optional;

        Ok((match_element, match_text_within))
    }

The following error occurs at runtime in the scan_args::get_kwargs function:

  1) Error:
TestMagnusKwargsBug#test_that_it_can_accept_kwargs:
TypeError: no implicit conversion of Float into String
    /Users/gjtorikian/Development/magnus_kwargs_bug/test/test_magnus_kwargs_bug.rb:7:in `new'
    /Users/gjtorikian/Development/magnus_kwargs_bug/test/test_magnus_kwargs_bug.rb:7:in `test_that_it_can_accept_kwargs'

I'm not sure why or how a Float is being interpreted, rather than the defined String.

Here's a reproducible use case: https://github.com/gjtorikian/magnus_kwargs_bug; after cloning the repo, run bundle exec rake compile test.

[matsadler]

Thanks for the bug report, and thank you for the great test case, made it much easier to figure out what was going on with a nice reproduction like that.

I'd made a mistake in the use of the underlying rb_get_kwargs function from Ruby's C API.

Ruby has an internal undef value that is only safe to use in like 3 places in the entire API, rb_get_kwargs was returning this value and I was mistakenly passing this on to the type conversion code. It just so happens that Ruby's API function for checking an object is / converting to a string, rb_str_to_str, gives "no implicit conversion of Float into String" when you give it undef. Other types I tested with segfaulted in their conversion functions.

I've fixed this in bb27c6e, and put out a 0.4.1 release with this fix.

[gjtorikian]

Thank you for the quick fix!

[gjtorikian]

@matsadler Ah, I think there's an errant dbg! call: https://github.com/matsadler/magnus/commit/bb27c6e79855f3d6335e8f2b9810d7c8658fe14f#diff-f4bb75ac712f5e9fe5cacea36a361bf6838554b9d0595044e24eac27aae88b6aR524

[matsadler]

Damnit, I thought I got them all, I guess that was hiding amongst the other changes when I reviewed them.

Thanks for spotting that. There's a 0.4.2 release out now. Lets hope I didn't make anymore mistakes.