Skip to content

Commit

Permalink
[GR-18163] Fix non-String argument conversion in Regexp.new
Browse files Browse the repository at this point in the history
PullRequest: truffleruby/3462
  • Loading branch information
andrykonchin committed Aug 22, 2022
2 parents 4e6b7fd + 338ecc7 commit f9ae7a9
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 27 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Compatibility:
* Fix exception message when there are missing or extra keyword arguments - it contains all the missing/extra keywords now (#1522, @andrykonchin).
* Always terminate native strings with enough `\0` bytes (#2704, @eregon).
* Support `#dup` and `#clone` on foreign strings (@eregon).
* Fix `Regexp.new` to coerce non-String arguments (#2705, @andrykonchin).

Performance:

Expand Down
4 changes: 4 additions & 0 deletions spec/ruby/core/regexp/compile_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@
describe "Regexp.compile given a Regexp" do
it_behaves_like :regexp_new_regexp, :compile
end

describe "Regexp.new given a non-String/Regexp" do
it_behaves_like :regexp_new_non_string_or_regexp, :compile
end
16 changes: 4 additions & 12 deletions spec/ruby/core/regexp/new_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,9 @@

describe "Regexp.new given a Regexp" do
it_behaves_like :regexp_new_regexp, :new
it_behaves_like :regexp_new_string_binary, :compile
it_behaves_like :regexp_new_string_binary, :new
end

describe "Regexp.new given an Integer" do
it "raises a TypeError" do
-> { Regexp.new(1) }.should raise_error(TypeError)
end
end

describe "Regexp.new given a Float" do
it "raises a TypeError" do
-> { Regexp.new(1.0) }.should raise_error(TypeError)
end
end
describe "Regexp.new given a non-String/Regexp" do
it_behaves_like :regexp_new_non_string_or_regexp, :new
end
36 changes: 36 additions & 0 deletions spec/ruby/core/regexp/shared/new.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,32 @@ class RegexpSpecsSubclassTwo < Regexp; end
end
end

describe :regexp_new_non_string_or_regexp, shared: true do
it "calls #to_str method for non-String/Regexp argument" do
obj = Object.new
def obj.to_str() "a" end

Regexp.send(@method, obj).should == /a/
end

it "raises TypeError if there is no #to_str method for non-String/Regexp argument" do
obj = Object.new
-> { Regexp.send(@method, obj) }.should raise_error(TypeError, "no implicit conversion of Object into String")

-> { Regexp.send(@method, 1) }.should raise_error(TypeError, "no implicit conversion of Integer into String")
-> { Regexp.send(@method, 1.0) }.should raise_error(TypeError, "no implicit conversion of Float into String")
-> { Regexp.send(@method, :symbol) }.should raise_error(TypeError, "no implicit conversion of Symbol into String")
-> { Regexp.send(@method, []) }.should raise_error(TypeError, "no implicit conversion of Array into String")
end

it "raises TypeError if #to_str returns non-String value" do
obj = Object.new
def obj.to_str() [] end

-> { Regexp.send(@method, obj) }.should raise_error(TypeError, /can't convert Object to String/)
end
end

describe :regexp_new_string, shared: true do
it "uses the String argument as an unescaped literal to construct a Regexp object" do
Regexp.send(@method, "^hi{2,3}fo.o$").should == /^hi{2,3}fo.o$/
Expand Down Expand Up @@ -97,6 +123,16 @@ class RegexpSpecsSubclassTwo < Regexp; end
(r.options & Regexp::EXTENDED).should_not == 0
end

it "does not try to convert the second argument to Integer with #to_int method call" do
ScratchPad.clear
obj = Object.new
def obj.to_int() ScratchPad.record(:called) end

Regexp.send(@method, "Hi", obj)

ScratchPad.recorded.should == nil
end

ruby_version_is ""..."3.2" do
it "treats any non-Integer, non-nil, non-false second argument as IGNORECASE" do
r = Regexp.send(@method, 'Hi', Object.new)
Expand Down
4 changes: 0 additions & 4 deletions spec/tags/core/regexp/compile_tags.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1 @@
fails:Regexp.compile given a String ignores the third argument if it is 'e' or 'euc' (case-insensitive)
fails:Regexp.compile given a String ignores the third argument if it is 's' or 'sjis' (case-insensitive)
fails:Regexp.compile given a String ignores the third argument if it is 'u' or 'utf8' (case-insensitive)
fails:Regexp.compile given a Regexp does not honour options given as additional arguments
fails(immutable regexp):Regexp.compile works by default for subclasses with overridden #initialize
4 changes: 0 additions & 4 deletions spec/tags/core/regexp/new_tags.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1 @@
fails:Regexp.new given a String ignores the third argument if it is 'e' or 'euc' (case-insensitive)
fails:Regexp.new given a String ignores the third argument if it is 's' or 'sjis' (case-insensitive)
fails:Regexp.new given a String ignores the third argument if it is 'u' or 'utf8' (case-insensitive)
fails:Regexp.new given a Regexp does not honour options given as additional arguments
fails(immutable regexp):Regexp.new works by default for subclasses with overridden #initialize
24 changes: 17 additions & 7 deletions src/main/ruby/truffleruby/core/regexp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,22 +130,32 @@ def self.union(*patterns)
end
Truffle::Graal.always_split(method(:union))

def self.new(pattern, opts=nil, lang=nil)
def self.new(pattern, opts=undefined, encoding=nil)
if Primitive.object_kind_of?(pattern, Regexp)
warn 'flags ignored' unless Primitive.undefined?(opts)
opts = pattern.options
pattern = pattern.source
elsif Primitive.object_kind_of?(pattern, Integer) or Primitive.object_kind_of?(pattern, Float)
raise TypeError, "can't convert #{pattern.class} into String"
elsif Primitive.object_kind_of?(opts, Integer)
else
pattern = Truffle::Type.rb_convert_type pattern, String, :to_str
end

if Primitive.object_kind_of?(opts, Integer)
opts = opts & (OPTION_MASK | KCODE_MASK) if opts > 0
elsif opts
elsif !Primitive.undefined?(opts) && opts
opts = IGNORECASE
else
opts = 0
end

code = lang[0] if lang
opts |= NOENCODING if code == ?n or code == ?N
if encoding
encoding = Truffle::Type.rb_convert_type encoding, String, :to_str
code = encoding[0]
if code == ?n or code == ?N
opts |= NOENCODING
else
warn "encoding option is ignored - #{encoding}"
end
end

Primitive.regexp_compile pattern, opts # may be overridden by subclasses
end
Expand Down

0 comments on commit f9ae7a9

Please sign in to comment.