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
Fix segmentation fault of StringScanner#charpos
when String#byteslice
returns non string value [Bug #17756]
#20
Conversation
StringScanner#charpos
when target.byteslice
returns non string valueStringScanner#charpos
when target.byteslice
returns non string value [Bug #17756]
ext/strscan/strscan.c
Outdated
@@ -450,6 +450,7 @@ strscan_get_charpos(VALUE self) | |||
GET_SCANNER(self, p); | |||
|
|||
substr = rb_funcall(p->str, id_byteslice, 2, INT2FIX(0), LONG2NUM(p->curr)); | |||
Check_Type(substr, T_STRING); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to rather use rb_enc_strlen()
, which takes char *
directly? It seems like a waste to create a String object just for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! Thank you for letting me know, I didn't know the feature 😅
How about like this 02bfa2c ?
Tests passed in Ruby 3.0.0. But I'm a newbie in C, so I'm afraid something wrong 🙏
#20 (comment) Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
#20 (comment) Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
test/strscan/test_stringscanner.rb
Outdated
def test_charpos_not_use_byteslice | ||
string = +'ruby' | ||
scanner = create_string_scanner(string) | ||
pre = Module.new do | ||
def byteslice(*args) | ||
end | ||
end | ||
string.singleton_class.prepend(pre) | ||
assert_equal(0, scanner.charpos) | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this tests are really need for now?
It aims regression tests, but I feel a bit meaningless since #20 (comment), 02bfa2c 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you think this change? 1531d35 🤔
StringScanner#charpos
when target.byteslice
returns non string value [Bug #17756]StringScanner#charpos
when String#byteslice
returns non string value [Bug #17756]
Thanks! |
Thank you! |
ref: https://bugs.ruby-lang.org/issues/17756
This code makes segmentation fault.
The crash report.