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

Check for integer overflows in Ruby bindings #1572

Merged
merged 2 commits into from
Jun 14, 2023

Conversation

heinrich5991
Copy link
Contributor

@heinrich5991 heinrich5991 commented Jun 1, 2023

The exception thrown is RangeError, like e.g. when doing 256.chr.

> 256.chr
(irb):1:in `chr': 256 out of char range (RangeError)
        from (irb):1:in `<main>'
        from /usr/lib/ruby/gems/3.0.0/gems/irb-1.4.2/exe/irb:11:in `<top (required)>'
        from /usr/bin/irb:25:in `load'
        from /usr/bin/irb:25:in `<main>'

@heinrich5991 heinrich5991 requested a review from a team as a code owner June 1, 2023 10:10
@heinrich5991 heinrich5991 requested review from badboy and removed request for a team June 1, 2023 10:10
@heinrich5991
Copy link
Contributor Author

The method uniffi_in_range is currently placed in the top-level, which is probably not desirable. I couldn't figure out how to move it to the Uniffi<ModuleName> because it has to be accessible from the outside, too. I couldn't name the Uniffi<ModuleName> from fn coerce_rb, so I couldn't just place it in there.

Any ideas how this could be solved?

@heinrich5991
Copy link
Contributor Author

@saks Perhaps you have an idea to the above question? The matrix channel told me that you're the one who authored these bindings.

@saks
Copy link
Contributor

saks commented Jun 9, 2023

@heinrich5991 , thanks for reaching out. I'll get back to you in a few hours.

@saks
Copy link
Contributor

saks commented Jun 10, 2023

@heinrich5991

I like the idea to put this method into some module, the one defined as uniffi namespace should do. I've created this PR (https://github.com/heinrich5991/uniffi-rs/pull/1/files) to illustrate the idea. Please have a look and let me know

heinrich5991 and others added 2 commits June 14, 2023 08:55
The exception thrown is
[`RangeError`](https://docs.ruby-lang.org/en/master/RangeError.html),
like e.g. when doing `256.chr`.

```
> 256.chr
(irb):1:in `chr': 256 out of char range (RangeError)
        from (irb):1:in `<main>'
        from /usr/lib/ruby/gems/3.0.0/gems/irb-1.4.2/exe/irb:11:in `<top (required)>'
        from /usr/bin/irb:25:in `load'
        from /usr/bin/irb:25:in `<main>'
```
@heinrich5991 heinrich5991 force-pushed the pr_more_integer_limits branch from f483110 to ee58fb8 Compare June 14, 2023 06:56
@heinrich5991
Copy link
Contributor Author

@saks

I like the idea to put this method into some module, the one defined as uniffi namespace should do. I've created this PR (https://github.com/heinrich5991/uniffi-rs/pull/1/files) to illustrate the idea. Please have a look and let me know

Thanks for the PR, I added the commit.

@badboy badboy added this to the v0.24.0 milestone Jun 14, 2023
@badboy badboy merged commit 26f77b4 into mozilla:main Jun 14, 2023
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

Successfully merging this pull request may close these issues.

3 participants