Add more bounds checks to Substring

Otherwise it's possible to access more of the underlying string than the
substring covers. Discovered in #24. Not sure if Substring is really
worth it at this point.

Author: robinst

commonmark/src/main/java/org/commonmark/internal/util/Substring.java

@@ -14,12 +14,21 @@ public static CharSequence of(String base, int beginIndex, int endIndex) {
     }
 
     private Substring(String base, int beginIndex, int endIndex) {
+        if (beginIndex < 0) {
+            throw new StringIndexOutOfBoundsException("beginIndex must be at least 0");
+        }
+        if (endIndex < 0) {
+            throw new StringIndexOutOfBoundsException("endIndex must be at least 0");
+        }
+        if (endIndex < beginIndex) {
+            throw new StringIndexOutOfBoundsException("endIndex must not be less than beginIndex");
+        }
+        if (endIndex > base.length()) {
+            throw new StringIndexOutOfBoundsException("endIndex must not be greater than length");
+        }
         this.base = base;
         this.beginIndex = beginIndex;
         this.endIndex = endIndex;
-        if (endIndex > base.length()) {
-            throw new IndexOutOfBoundsException("endIndex must not be greater than length");
-        }
     }
 
     @Override
@@ -29,11 +38,20 @@ public int length() {
 
     @Override
     public char charAt(int index) {
+        if (index < 0 || beginIndex + index >= endIndex) {
+            throw new StringIndexOutOfBoundsException("String index out of range: " + index);
+        }
         return base.charAt(index + beginIndex);
     }
 
     @Override
     public CharSequence subSequence(int start, int end) {
+        if (start < 0 || beginIndex + start > endIndex) {
+            throw new StringIndexOutOfBoundsException("String index out of range: " + start);
+        }
+        if (end < 0 || beginIndex + end > endIndex) {
+            throw new StringIndexOutOfBoundsException("String index out of range: " + end);
+        }
         return new Substring(base, beginIndex + start, beginIndex + end);
     }
 
@@ -49,6 +67,6 @@ public int hashCode() {
 
     @Override
     public boolean equals(Object obj) {
-        return obj == this || (obj instanceof CharSequence && toString().equals(obj));
+        return obj == this || (obj instanceof CharSequence && toString().equals(obj.toString()));
     }
 }

commonmark/src/test/java/org/commonmark/test/SubstringTest.java

@@ -0,0 +1,91 @@
+package org.commonmark.test;
+
+import org.commonmark.internal.util.Substring;
+import org.junit.Test;
+
+import static junit.framework.TestCase.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+public class SubstringTest {
+
+    private final CharSequence substring = Substring.of("abcdefghi", 3, 6);
+
+    @Test
+    public void testConstructEmpty() {
+        assertEquals("", Substring.of("ab", 0, 0).toString());
+        assertEquals("", Substring.of("ab", 1, 1).toString());
+        assertEquals("", Substring.of("ab", 2, 2).toString());
+    }
+
+    @Test(expected = StringIndexOutOfBoundsException.class)
+    public void testConstructBeginIndexNegative() {
+        Substring.of("abc", -1, 0);
+    }
+
+    @Test(expected = StringIndexOutOfBoundsException.class)
+    public void testConstructEndIndexNegative() {
+        Substring.of("abc", 0, -1);
+    }
+
+    @Test(expected = StringIndexOutOfBoundsException.class)
+    public void testConstructEndIndexLessThanBegin() {
+        Substring.of("abc", 1, 0);
+    }
+
+    @Test(expected = StringIndexOutOfBoundsException.class)
+    public void testConstructEndIndexGreaterThanLength() {
+        Substring.of("abc", 1, 4);
+    }
+
+    @Test
+    public void testLength() {
+        assertEquals(3, substring.length());
+    }
+
+    @Test
+    public void testCharAt() {
+        assertEquals('d', substring.charAt(0));
+        assertEquals('e', substring.charAt(1));
+        assertEquals('f', substring.charAt(2));
+    }
+
+    @Test(expected = StringIndexOutOfBoundsException.class)
+    public void testCharAtOutOfBoundsLeft() {
+        substring.charAt(-1);
+    }
+
+    @Test(expected = StringIndexOutOfBoundsException.class)
+    public void testCharAtOutOfBoundsRight() {
+        substring.charAt(3);
+    }
+
+    @Test
+    public void testSubSequence() {
+        assertEquals("d", substring.subSequence(0, 1).toString());
+        assertEquals("e", substring.subSequence(1, 2).toString());
+        assertEquals("f", substring.subSequence(2, 3).toString());
+        assertEquals("def", substring.subSequence(0, 3).toString());
+    }
+
+    @Test(expected = StringIndexOutOfBoundsException.class)
+    public void testSubSequenceOutOfBoundsLeft() {
+        substring.subSequence(-1, 2);
+    }
+
+    @Test(expected = StringIndexOutOfBoundsException.class)
+    public void testSubSequenceOutOfBoundsRight() {
+        substring.subSequence(1, 4);
+    }
+
+    @Test
+    public void testHashCodeEquals() {
+        CharSequence a = Substring.of("abcdefghi", 3, 6);
+        CharSequence b = Substring.of("123def456", 3, 6);
+        CharSequence other = Substring.of("123de", 3, 5);
+        assertEquals(a, b);
+        assertEquals(b, a);
+        assertNotEquals(a, other);
+        assertNotEquals(other, a);
+        assertEquals(a.hashCode(), b.hashCode());
+    }
+}