[LANG-786] StringUtils equals() relies on undefined behavior; thanks to Daniel Trebbien

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/lang/trunk@1234915 13f79535-47bb-0310-9956-ffa450edef68

pom.xml

@@ -394,6 +394,9 @@
     <contributor>
       <name>Masato Tezuka</name>
     </contributor>
+    <contributor>
+      <name>Daniel Trebbien</name>
+    </contributor>
     <contributor>
       <name>Jeff Varszegi</name>
     </contributor>

src/main/java/org/apache/commons/lang3/StringUtils.java

@@ -758,7 +758,8 @@ public class StringUtils {
     // Equals
     //-----------------------------------------------------------------------
     /**
-     * <p>Compares two CharSequences, returning {@code true} if they are equal.</p>
+     * <p>Compares two CharSequences, returning {@code true} if they represent
+     * equal sequences of characters.</p>
      *
      * <p>{@code null}s are handled without exceptions. Two {@code null}
      * references are considered to be equal. The comparison is case sensitive.</p>
@@ -771,20 +772,28 @@ public class StringUtils {
      * StringUtils.equals("abc", "ABC") = false
      * </pre>
      *
-     * @see java.lang.String#equals(Object)
+     * @see java.lang.CharSequence#equals(Object)
-     * @param cs1  the first CharSequence, may be null
+     * @param cs1  the first CharSequence, may be {@code null}
-     * @param cs2  the second CharSequence, may be null
+     * @param cs2  the second CharSequence, may be {@code null}
-     * @return {@code true} if the CharSequences are equal, case sensitive, or
+     * @return {@code true} if the CharSequences are equal (case-sensitive), or both {@code null}
-     *  both {@code null}
      * @since 3.0 Changed signature from equals(String, String) to equals(CharSequence, CharSequence)
      */
     public static boolean equals(CharSequence cs1, CharSequence cs2) {
-        return cs1 == null ? cs2 == null : cs1.equals(cs2);
+        if (cs1 == cs2) {
+            return true;
+        }
+        if (cs1 == null || cs2 == null) {
+            return false;
+        }
+        if (cs1 instanceof String && cs2 instanceof String) {
+            return cs1.equals(cs2);
+        }
+        return CharSequenceUtils.regionMatches(cs1, false, 0, cs2, 0, Math.max(cs1.length(), cs2.length()));
     }
 
     /**
-     * <p>Compares two CharSequences, returning {@code true} if they are equal ignoring
+     * <p>Compares two CharSequences, returning {@code true} if they represent
-     * the case.</p>
+     * equal sequences of characters, ignoring case.</p>
      *
      * <p>{@code null}s are handled without exceptions. Two {@code null}
      * references are considered equal. Comparison is case insensitive.</p>

src/test/java/org/apache/commons/lang3/StringUtilsEqualsIndexOfTest.java

@@ -19,6 +19,8 @@ package org.apache.commons.lang3;
 import java.util.Locale;
 
 import junit.framework.TestCase;
+import org.hamcrest.core.IsNot;
+import static org.junit.Assert.assertThat;
 
 /**
  * Unit tests {@link org.apache.commons.lang3.StringUtils} - Substring methods
@@ -438,14 +440,75 @@ public class StringUtilsEqualsIndexOfTest extends TestCase {
         assertTrue( StringUtils.containsWhitespace("\n") );
     }
 
+    // The purpose of this class is to test StringUtils#equals(CharSequence, CharSequence)
+    // with a CharSequence implementation whose equals(Object) override requires that the
+    // other object be an instance of CustomCharSequence, even though, as char sequences,
+    // `seq` may equal the other object.
+    private static class CustomCharSequence implements CharSequence {
+        private CharSequence seq;
+
+        public CustomCharSequence(CharSequence seq) {
+            this.seq = seq;
+        }
+
+        public char charAt(int index) {
+            return seq.charAt(index);
+        }
+
+        public int length() {
+            return seq.length();
+        }
+
+        public CharSequence subSequence(int start, int end) {
+            return new CustomCharSequence(seq.subSequence(start, end));
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (obj == null || !(obj instanceof CustomCharSequence)) {
+                return false;
+            }
+            CustomCharSequence other = (CustomCharSequence) obj;
+            return seq.equals(other.seq);
+        }
+
+        public String toString() {
+            return seq.toString();
+        }
+    }
+
+    public void testCustomCharSequence() {
+        assertThat((CharSequence) new CustomCharSequence(FOO), IsNot.<CharSequence>not(FOO));
+        assertThat((CharSequence) FOO, IsNot.<CharSequence>not(new CustomCharSequence(FOO)));
+        assertEquals(new CustomCharSequence(FOO), new CustomCharSequence(FOO));
+    }
+
     public void testEquals() {
-        assertEquals(true, StringUtils.equals(null, null));
+        final CharSequence fooCs = FOO, barCs = BAR, foobarCs = FOOBAR;
-        assertEquals(true, StringUtils.equals(FOO, FOO));
+        assertTrue(StringUtils.equals(null, null));
-        assertEquals(true, StringUtils.equals(FOO, new String(new char[] { 'f', 'o', 'o' })));
+        assertTrue(StringUtils.equals(fooCs, fooCs));
-        assertEquals(false, StringUtils.equals(FOO, new String(new char[] { 'f', 'O', 'O' })));
+        assertTrue(StringUtils.equals(fooCs, (CharSequence) new StringBuilder(FOO)));
-        assertEquals(false, StringUtils.equals(FOO, BAR));
+        assertTrue(StringUtils.equals(fooCs, (CharSequence) new String(new char[] { 'f', 'o', 'o' })));
-        assertEquals(false, StringUtils.equals(FOO, null));
+        assertTrue(StringUtils.equals(fooCs, (CharSequence) new CustomCharSequence(FOO)));
-        assertEquals(false, StringUtils.equals(null, FOO));
+        assertTrue(StringUtils.equals((CharSequence) new CustomCharSequence(FOO), fooCs));
+        assertFalse(StringUtils.equals(fooCs, (CharSequence) new String(new char[] { 'f', 'O', 'O' })));
+        assertFalse(StringUtils.equals(fooCs, barCs));
+        assertFalse(StringUtils.equals(fooCs, null));
+        assertFalse(StringUtils.equals(null, fooCs));
+        assertFalse(StringUtils.equals(fooCs, foobarCs));
+        assertFalse(StringUtils.equals(foobarCs, fooCs));
+    }
+
+    public void testEqualsOnStrings() {
+        assertTrue(StringUtils.equals(null, null));
+        assertTrue(StringUtils.equals(FOO, FOO));
+        assertTrue(StringUtils.equals(FOO, new String(new char[] { 'f', 'o', 'o' })));
+        assertFalse(StringUtils.equals(FOO, new String(new char[] { 'f', 'O', 'O' })));
+        assertFalse(StringUtils.equals(FOO, BAR));
+        assertFalse(StringUtils.equals(FOO, null));
+        assertFalse(StringUtils.equals(null, FOO));
+        assertFalse(StringUtils.equals(FOO, FOOBAR));
+        assertFalse(StringUtils.equals(FOOBAR, FOO));
     }
 
     public void testEqualsIgnoreCase() {