Repository: netty/netty
Issue: ArrayIndexOutOfBoundsException in AsciiString().indexOf()
Labels: defect

### Actual behavior
Calling the method indexOf() in a special case throws ArrayIndexOutOfBoundsException.

java.lang.ArrayIndexOutOfBoundsException: 6
	at io.netty.util.AsciiString.indexOf(AsciiString.java:693)
	at ByteBufsTests.indexOfSuccess(ByteBufsTests.java:46)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
...

### Steps to reproduce
Run the following 2 lines of JUnit code, I hope this is enough for reproduction.

### Minimal yet complete reproducer code (or URL to code)
Assert.assertEquals("Test success in 4.1.(22|23).Final", 3, new AsciiString("012345").indexOf("345", 3));
Assert.assertEquals("Test fails with 'ArrayIndexOutOfBoundsException: 6' in 4.1.23.Final", -1, new AsciiString("012345").indexOf("abc", 3));

### Netty version
4.1.23.Final

### JVM version (e.g. `java -version`)
java version "1.8.0_66"
Java(TM) SE Runtime Environment (build 1.8.0_66-b17)
Java HotSpot(TM) 64-Bit Server VM (build 25.66-b17, mixed mode)

### OS version (e.g. `uname -a`)
Win7

Issue comments:
@Stefan-Weber yes thanks... working on a patch

Pull request review comments:
I think it will be more correct:
```java
final int len = offset + length - subCount;
```
that is, do not add the `start` to the `len`.
Agreed. There is also a bounds checking issue with the lastIndexOf below. See PR https://github.com/netty/netty/pull/7866
sounds good ... let me close this one

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/AsciiString.java b/common/src/main/java/io/netty/util/AsciiString.java
index 17ef64f3287..621db211e52 100644
--- a/common/src/main/java/io/netty/util/AsciiString.java
+++ b/common/src/main/java/io/netty/util/AsciiString.java
@@ -689,7 +689,7 @@ public int indexOf(CharSequence subString, int start) {
         }
         final byte firstCharAsByte = c2b0(firstChar);
         final int len = offset + start + length - subCount;
-        for (int i = start + offset; i <= len; ++i) {
+        for (int i = start + offset; i <= len && i < length; ++i) {
             if (value[i] == firstCharAsByte) {
                 int o1 = i, o2 = 0;
                 while (++o2 < subCount && b2c(value[++o1]) == subString.charAt(o2)) {
@@ -773,7 +773,7 @@ public int lastIndexOf(CharSequence subString, int start) {
         }
         final byte firstCharAsByte = c2b0(firstChar);
         final int end = offset + start;
-        for (int i = offset + start + length - subCount; i >= end; --i) {
+        for (int i = length - subCount; i >= end; --i) {
             if (value[i] == firstCharAsByte) {
                 int o1 = i, o2 = 0;
                 while (++o2 < subCount && b2c(value[++o1]) == subString.charAt(o2)) {


Test patch:
diff --git a/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java b/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java
index 60686f18a8c..e80f4424dfb 100644
--- a/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java
+++ b/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java
@@ -385,4 +385,26 @@ public void testSubStringHashCode() {
         //two "123"s
         assertEquals(AsciiString.hashCode("123"), AsciiString.hashCode("a123".substring(1)));
     }
+
+    @Test
+    public void testIndexOf() {
+        Assert.assertEquals(3, new AsciiString("012345").indexOf("345", 3));
+        Assert.assertEquals(3, new AsciiString("012345").indexOf("345", 0));
+        Assert.assertEquals(-1, new AsciiString("012345").indexOf("345", 4));
+        Assert.assertEquals(-1, new AsciiString("012345").indexOf("abc", 3));
+        Assert.assertEquals(-1, new AsciiString("012345").indexOf("abc", 0));
+        Assert.assertEquals(-1, new AsciiString("012345").indexOf("abcdefghi", 0));
+        Assert.assertEquals(-1, new AsciiString("012345").indexOf("abcdefghi", 4));
+    }
+
+    @Test
+    public void testLastIndexOf() {
+        Assert.assertEquals(3, new AsciiString("012345").lastIndexOf("345", 3));
+        Assert.assertEquals(3, new AsciiString("012345").lastIndexOf("345", 0));
+        Assert.assertEquals(-1, new AsciiString("012345").lastIndexOf("345", 4));
+        Assert.assertEquals(-1, new AsciiString("012345").lastIndexOf("abc", 3));
+        Assert.assertEquals(-1, new AsciiString("012345").lastIndexOf("abc", 0));
+        Assert.assertEquals(-1, new AsciiString("012345").lastIndexOf("abcdefghi", 0));
+        Assert.assertEquals(-1, new AsciiString("012345").lastIndexOf("abcdefghi", 4));
+    }
 }