Skip to content

Commit 3b0fb88

Browse files
committed
Fix String#slice with negative offset
This fix resolves the issue with String#slice(index, offset) called with negative offset. #slice should return `nil` when offset is negative but returned a String itself: ```ruby "foobar".slice(0, -1) #=> "foobar" ```
1 parent 84048ba commit 3b0fb88

File tree

3 files changed

+22
-5
lines changed

3 files changed

+22
-5
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Compatibility:
1111
* Implement `StringScanner#{peek_byte,scan_byte,scan_integer,named_captures}` methods (#3788, @andrykonchin).
1212
* Support String patterns in `StringScanner#{exist?,scan_until,skip_until,check_until,search_full}` methods (@andrykonchin).
1313
* Implement `ObjectSpace::WeakKeyMap` (#3681, @andrykonchin).
14+
* Fix `String#slice` called with negative offset (@andrykonchin).
1415

1516
Performance:
1617

spec/ruby/core/string/shared/slice.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,13 @@
119119
"hello there".send(@method, -4,-3).should == nil
120120
end
121121

122+
platform_is pointer_size: 64 do
123+
it "returns nil if the length is negative big value" do
124+
"hello there".send(@method, 4, -(1 << 31)).should == nil
125+
"hello there".send(@method, 4, -(1 << 63)).should == nil
126+
end
127+
end
128+
122129
it "calls to_int on the given index and the given length" do
123130
"hello".send(@method, 0.5, 1).should == "h"
124131
"hello".send(@method, 0.5, 2.5).should == "he"

src/main/java/org/truffleruby/core/string/StringNodes.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,13 @@ Object slice(Object string, int start, int length) {
575575
}
576576

577577
@Specialization
578-
Object slice(Object string, long start, long length) {
578+
Object slice(Object string, long start, long length,
579+
@Cached @Shared InlinedBranchProfile negativeLengthProfile) {
580+
if (length < 0) {
581+
negativeLengthProfile.enter(this);
582+
return nil;
583+
}
584+
579585
int lengthInt = (int) length;
580586
if (lengthInt != length) {
581587
lengthInt = Integer.MAX_VALUE; // go to end of string
@@ -589,8 +595,9 @@ Object slice(Object string, long start, long length) {
589595

590596
@Specialization(guards = "wasProvided(length)")
591597
Object slice(Object string, long start, Object length,
592-
@Cached @Shared ToLongNode toLongNode) {
593-
return slice(string, start, toLongNode.execute(this, length));
598+
@Cached @Shared ToLongNode toLongNode,
599+
@Cached @Shared InlinedBranchProfile negativeLengthProfile) {
600+
return slice(string, start, toLongNode.execute(this, length), negativeLengthProfile);
594601
}
595602

596603
@Specialization(
@@ -600,8 +607,10 @@ Object slice(Object string, long start, Object length,
600607
"isNotRubyString(start)",
601608
"wasProvided(length)" })
602609
Object slice(Object string, Object start, Object length,
603-
@Cached @Shared ToLongNode toLongNode) {
604-
return slice(string, toLongNode.execute(this, start), toLongNode.execute(this, length));
610+
@Cached @Shared ToLongNode toLongNode,
611+
@Cached @Shared InlinedBranchProfile negativeLengthProfile) {
612+
return slice(string, toLongNode.execute(this, start), toLongNode.execute(this, length),
613+
negativeLengthProfile);
605614
}
606615

607616
// endregion

0 commit comments

Comments
 (0)