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

Regexp#match{?} with nil raises TypeError as String, Symbol #1506

Merged
merged 8 commits into from
Oct 17, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/cgi/cookie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def initialize(name = "", *value)
@expires = nil
if name.kind_of?(String)
@name = name
%r|^(.*/)|.match(ENV["SCRIPT_NAME"])
%r|^(.*/)|.match(ENV["SCRIPT_NAME"].to_s)
@path = ($1 or "")
@secure = false
@httponly = false
Expand All @@ -91,7 +91,7 @@ def initialize(name = "", *value)
if options["path"]
@path = options["path"]
else
%r|^(.*/)|.match(ENV["SCRIPT_NAME"])
%r|^(.*/)|.match(ENV["SCRIPT_NAME"].to_s)
@path = ($1 or "")
end
@domain = options["domain"]
Expand Down
6 changes: 3 additions & 3 deletions lib/cgi/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def _header_for_hash(options) #:nodoc:
private :_header_for_hash

def nph? #:nodoc:
return /IIS\/(\d+)/.match($CGI_ENV['SERVER_SOFTWARE']) && $1.to_i < 5
return /IIS\/(\d+)/.match($CGI_ENV['SERVER_SOFTWARE'].to_s) && $1.to_i < 5
end

def _header_for_modruby(buf) #:nodoc:
Expand Down Expand Up @@ -606,7 +606,7 @@ def create_body(is_large) #:nodoc:
return body
end
def unescape_filename? #:nodoc:
user_agent = $CGI_ENV['HTTP_USER_AGENT']
user_agent = $CGI_ENV['HTTP_USER_AGENT'].to_s
return /Mac/i.match(user_agent) && /Mozilla/i.match(user_agent) && !/MSIE/i.match(user_agent)
end

Expand Down Expand Up @@ -648,7 +648,7 @@ def read_from_cmdline
# Reads query parameters in the @params field, and cookies into @cookies.
def initialize_query()
if ("POST" == env_table['REQUEST_METHOD']) and
%r|\Amultipart/form-data.*boundary=\"?([^\";,]+)\"?|.match(env_table['CONTENT_TYPE'])
%r|\Amultipart/form-data.*boundary=\"?([^\";,]+)\"?|.match(env_table['CONTENT_TYPE'].to_s)
current_max_multipart_length = @max_multipart_length.respond_to?(:call) ? @max_multipart_length.call : @max_multipart_length
raise StandardError.new("too large multipart data.") if env_table['CONTENT_LENGTH'].to_i > current_max_multipart_length
boundary = $1.dup
Expand Down
2 changes: 1 addition & 1 deletion re.c
Original file line number Diff line number Diff line change
Expand Up @@ -3284,6 +3284,7 @@ rb_reg_match_m(int argc, VALUE *argv, VALUE re)
pos = 0;
}

str = SYMBOL_P(str) ? rb_sym2str(str) : StringValue(str);
pos = reg_match_pos(re, &str, pos);
if (pos < 0) {
rb_backref_set(Qnil);
Expand Down Expand Up @@ -3329,7 +3330,6 @@ rb_reg_match_p(VALUE re, VALUE str, long pos)
const UChar *start, *end;
int tmpreg;

if (NIL_P(str)) return Qfalse;
str = SYMBOL_P(str) ? rb_sym2str(str) : StringValue(str);
if (pos) {
if (pos < 0) {
Expand Down
21 changes: 4 additions & 17 deletions spec/ruby/core/regexp/match_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
it "returns nil if there is no match" do
/xyz/.send(@method,"abxyc").should be_nil
end

it "returns nil if the object is nil" do
/\w+/.send(@method, nil).should be_nil
end
end

describe "Regexp#=~" do
it_behaves_like :regexp_match, :=~

it "returns nil if the object is nil" do
(/\w+/ =~ nil).should be_nil
end

it "returns the index of the first character of the matching region" do
(/(.)(.)(.)/ =~ "abc").should == 0
end
Expand Down Expand Up @@ -91,15 +91,6 @@
end
end

it "resets $~ if passed nil" do
# set $~
/./.match("a")
$~.should be_kind_of(MatchData)

/1/.match(nil)
$~.should be_nil
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to add a ruby_version_is ""..."2.6" guard instead of removing, since the behavior continues to apply to older versions.
Could you also add specs for the new behavior?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addresses in 1041650 ~


it "raises TypeError when the given argument cannot be coarce to String" do
f = 1
lambda { /foo/.match(f)[0] }.should raise_error(TypeError)
Expand Down Expand Up @@ -133,10 +124,6 @@
/str/i.match?('string', 0).should be_true
/str/i.match?('string', 1).should be_false
end

it "returns false when given nil" do
/./.match?(nil).should be_false
end
end
end

Expand Down
4 changes: 2 additions & 2 deletions test/ruby/test_regexp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ def test_equal
end

def test_match
assert_nil(//.match(nil))
assert_raise(TypeError) { //.match(nil) }
assert_equal("abc", /.../.match(:abc)[0])
assert_raise(TypeError) { /.../.match(Object.new)[0] }
assert_equal("bc", /../.match('abc', 1)[0])
Expand All @@ -555,10 +555,10 @@ def test_match_p
/backref/ =~ 'backref'
# must match here, but not in a separate method, e.g., assert_send,
# to check if $~ is affected or not.
assert_equal(false, //.match?(nil))
assert_equal(true, //.match?(""))
assert_equal(true, /.../.match?(:abc))
assert_raise(TypeError) { /.../.match?(Object.new) }
assert_raise(TypeError) { //.match?(nil) }
assert_equal(true, /b/.match?('abc'))
assert_equal(true, /b/.match?('abc', 1))
assert_equal(true, /../.match?('abc', 1))
Expand Down
5 changes: 5 additions & 0 deletions test/ruby/test_string.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2469,6 +2469,7 @@ def o.==(other) "" == other end

def test_match_method
assert_equal("bar", "foobarbaz".match(/bar/).to_s)
assert_raise(TypeError) { "".match(nil) }

o = Regexp.new('foo')
def o.match(x, y, z); x + y + z; end
Expand Down Expand Up @@ -2524,6 +2525,10 @@ def test_match_p_string
assert_equal('backref', $&)
end

def test_match_p_nil
assert_raise(TypeError) { ''.match?(nil) }
end

def test_clear
s = "foo" * 100
s.clear
Expand Down
5 changes: 5 additions & 0 deletions test/ruby/test_symbol.rb
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ def o.=~(x); x + "bar"; end

def test_match_method
assert_equal("bar", :"foobarbaz".match(/bar/).to_s)
assert_raise(TypeError) { :"".match(nil) }

o = Regexp.new('foo')
def o.match(x, y, z); x + y + z; end
Expand Down Expand Up @@ -384,6 +385,10 @@ def test_match_p_string
assert_equal('backref', $&)
end

def test_match_p_nil
assert_raise(TypeError) { :''.match?(nil) }
end

def test_symbol_popped
assert_nothing_raised { eval('a = 1; :"#{ a }"; 1') }
end
Expand Down