Skip to content

Commit ed5d339

Browse files
committed
Fix bounds checking issues with MatchData#[Integer, Integer].
1 parent 79ec0de commit ed5d339

File tree

3 files changed

+25
-4
lines changed

3 files changed

+25
-4
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ New features:
66
Bug fixes:
77

88
* Fix `rb_id2name` to ensure the native string will have the same lifetime as the id (#2630, @aardvark179).
9+
* Fix `MatchData#[]` exception when passing a length argument larger than the number of match values (#2636, @nirvdrum).
10+
* Fix `MatchData#[]` exception when supplying a large negative index along with a length argument (@nirvdrum).
911

1012
Compatibility:
1113

spec/ruby/core/matchdata/element_reference_spec.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616
it "supports accessors [start, length]" do
1717
/(.)(.)(\d+)(\d)/.match("THX1138.")[1, 2].should == %w|H X|
1818
/(.)(.)(\d+)(\d)/.match("THX1138.")[-3, 2].should == %w|X 113|
19+
20+
# negative index is larger than the number of match values
21+
/(.)(.)(\d+)(\d)/.match("THX1138.")[-30, 2].should == nil
22+
23+
# length argument larger than number of match values is capped to match value length
24+
/(.)(.)(\d+)(\d)/.match("THX1138.")[3, 10].should == %w|113 8|
1925
end
2026

2127
it "supports ranges [start..end]" do

src/main/java/org/truffleruby/core/regexp/MatchDataNodes.java

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,14 +285,27 @@ protected Object getIndex(RubyMatchData matchData, int index, NotProvided length
285285
}
286286

287287
@Specialization
288-
protected RubyArray getIndex(RubyMatchData matchData, int index, int length,
289-
@Cached ConditionProfile normalizedIndexProfile) {
290-
// TODO BJF 15-May-2015 Need to handle negative indexes and lengths and out of bounds
288+
protected Object getIndex(RubyMatchData matchData, int index, int length,
289+
@Cached ConditionProfile normalizedIndexProfile,
290+
@Cached ConditionProfile indexOutOfBoundsProfile,
291+
@Cached ConditionProfile tooLargeTotalProfile) {
291292
final Object[] values = getValuesNode.execute(matchData);
293+
292294
if (normalizedIndexProfile.profile(index < 0)) {
293295
index += values.length;
296+
297+
if (indexOutOfBoundsProfile.profile(index < 0)) {
298+
return nil;
299+
}
300+
}
301+
302+
int endIndex = index + length;
303+
if (tooLargeTotalProfile.profile(endIndex > values.length)) {
304+
endIndex = values.length;
294305
}
295-
final Object[] store = Arrays.copyOfRange(values, index, index + length);
306+
307+
final Object[] store = Arrays.copyOfRange(values, index, endIndex);
308+
296309
return createArray(store);
297310
}
298311

0 commit comments

Comments
 (0)