From cb85e03d728e8e24be6b9b05070643779948785a 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 0bd09deb2f..7a0f46c530 100644 --- a/common/src/main/java/io/netty/util/AsciiString.java +++ b/common/src/main/java/io/netty/util/AsciiString.java @@ -746,7 +746,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); } /** @@ -761,23 +761,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 c2a835df66..deff4267d6 100644 --- a/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java +++ b/common/src/test/java/io/netty/util/AsciiStringCharacterTest.java @@ -356,15 +356,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)); @@ -376,7 +376,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));