From dd9e0d5a194aafda10b8cca529372a40aa5d118c Mon Sep 17 00:00:00 2001 From: Nick Hill Date: Sun, 12 May 2019 22:03:32 -0700 Subject: [PATCH] AsciiString.lastIndexOf(...) is implemented incorrectly (#9103) Motivation @xiaoheng1 reported incorrect behaviour of AsciiString.lastIndexOf in #9099. Upon closer inspection it appears that it was never implemented correctly and searches between the provided index and the end of the string similar to indexOf(...), rather than between the provided index and the beginning of the string as the javadoc states (and in line with java.lang.String). Modifications Fix AsciiString.lastIndexOf implementation and corresponding unit tests to behave the same as the equivalent String methods. Result Fixes #9099 --- .../main/java/io/netty/util/AsciiString.java | 15 ++++++--------- .../netty/util/AsciiStringCharacterTest.java | 18 +++++++++--------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/common/src/main/java/io/netty/util/AsciiString.java b/common/src/main/java/io/netty/util/AsciiString.java index 056c55df07..9b1427923c 100644 --- a/common/src/main/java/io/netty/util/AsciiString.java +++ b/common/src/main/java/io/netty/util/AsciiString.java @@ -744,7 +744,7 @@ public final class AsciiString implements CharSequence, Comparable */ public int lastIndexOf(CharSequence string) { // Use count instead of count - 1 so lastIndexOf("") answers count - return lastIndexOf(string, length()); + return lastIndexOf(string, length); } /** @@ -759,23 +759,20 @@ public final class AsciiString implements CharSequence, Comparable */ public int lastIndexOf(CharSequence subString, int start) { final int subCount = subString.length(); + start = Math.min(start, length - subCount); if (start < 0) { - start = 0; - } - if (subCount <= 0) { - return start < length ? start : length; - } - if (subCount > length - start) { return INDEX_NOT_FOUND; } + if (subCount == 0) { + return start; + } final char firstChar = subString.charAt(0); if (firstChar > MAX_CHAR_VALUE) { return INDEX_NOT_FOUND; } final byte firstCharAsByte = c2b0(firstChar); - final int end = offset + start; - for (int i = offset + length - subCount; i >= end; --i) { + for (int i = offset + start; i >= 0; --i) { if (value[i] == firstCharAsByte) { int o1 = i, o2 = 0; while (++o2 < subCount && b2c(value[++o1]) == subString.charAt(o2)) { diff --git a/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java b/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java index 76bd7dd72b..824979150c 100644 --- a/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java +++ b/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java @@ -361,15 +361,15 @@ public class AsciiStringCharacterTest { @Test public void testLastIndexOfCharSequence() { assertEquals(0, new AsciiString("abcd").lastIndexOf("abcd", 0)); - assertEquals(0, new AsciiString("abcd").lastIndexOf("abc", 0)); - assertEquals(1, new AsciiString("abcd").lastIndexOf("bcd", 0)); - assertEquals(1, new AsciiString("abcd").lastIndexOf("bc", 0)); - assertEquals(5, new AsciiString("abcdabcd").lastIndexOf("bcd", 0)); - assertEquals(0, new AsciiString("abcd", 1, 2).lastIndexOf("bc", 0)); - assertEquals(0, new AsciiString("abcd", 1, 3).lastIndexOf("bcd", 0)); - assertEquals(1, new AsciiString("abcdabcd", 4, 4).lastIndexOf("bcd", 0)); + assertEquals(0, new AsciiString("abcd").lastIndexOf("abc", 4)); + assertEquals(1, new AsciiString("abcd").lastIndexOf("bcd", 4)); + assertEquals(1, new AsciiString("abcd").lastIndexOf("bc", 4)); + assertEquals(5, new AsciiString("abcdabcd").lastIndexOf("bcd", 10)); + assertEquals(0, new AsciiString("abcd", 1, 2).lastIndexOf("bc", 2)); + assertEquals(0, new AsciiString("abcd", 1, 3).lastIndexOf("bcd", 3)); + assertEquals(1, new AsciiString("abcdabcd", 4, 4).lastIndexOf("bcd", 4)); assertEquals(3, new AsciiString("012345").lastIndexOf("345", 3)); - assertEquals(3, new AsciiString("012345").lastIndexOf("345", 0)); + assertEquals(3, new AsciiString("012345").lastIndexOf("345", 6)); // Test with empty string assertEquals(0, new AsciiString("abcd").lastIndexOf("", 0)); @@ -381,7 +381,7 @@ public class AsciiStringCharacterTest { assertEquals(-1, new AsciiString("abcdbc").lastIndexOf("bce", 0)); assertEquals(-1, new AsciiString("abcd", 1, 3).lastIndexOf("abc", 0)); assertEquals(-1, new AsciiString("abcd", 1, 2).lastIndexOf("bd", 0)); - assertEquals(-1, new AsciiString("012345").lastIndexOf("345", 4)); + assertEquals(-1, new AsciiString("012345").lastIndexOf("345", 2)); assertEquals(-1, new AsciiString("012345").lastIndexOf("abc", 3)); assertEquals(-1, new AsciiString("012345").lastIndexOf("abc", 0)); assertEquals(-1, new AsciiString("012345").lastIndexOf("abcdefghi", 0));