String positions and indices are clamped before they reach a pointer or a count

src/lib.ts

@@ -677,14 +677,24 @@ namespace __String {
         return newString;
     }      
 
-    export function endsWith(this: string, searchString: string, endPosition = this.length): boolean {
+    export function endsWith(this: string, searchString: string, endPosition: int = this.length): boolean {
         // a null check: "" is falsy, and an empty search string is found
         if (searchString == null)
         {
             return false;
         }
 
-        const lenstr = endPosition;
+        // the end is clamped to [0, length]: past the end the suffix was read after the terminator
+        let lenstr = endPosition;
+        if (lenstr < 0)
+        {
+            lenstr = 0;
+        }
+        else if (lenstr > this.length)
+        {
+            lenstr = this.length;
+        }
+
         const lensuffix = searchString.length;
         if (lensuffix > lenstr)
         {
@@ -694,12 +704,24 @@ namespace __String {
         return strncmp(<string>Ref(this[lenstr - lensuffix]), searchString, lensuffix) == 0;
     }    
 
-    export function includes(this: string, searchString: string, position = 0): boolean {    
-        if (position >= this.length) 
+    export function includes(this: string, searchString: string, position = 0): boolean {
+        // a null check: "" is falsy, and an empty search string is found
+        if (searchString == null)
         {
             return false;
         }
 
+        // the position is clamped to [0, length]: a negative one read before the string, and at
+        // the end only "" is found, at the terminator
+        if (position < 0)
+        {
+            position = 0;
+        }
+        else if (position > this.length)
+        {
+            position = this.length;
+        }
+
         return strstr(<string>Ref(this[position]), searchString) != null;
     }      
 
@@ -710,11 +732,16 @@ namespace __String {
             return -1;
         }
 
-        if (position >= this.length) 
+        // the position is clamped to [0, length], as in includes; at the end only "" is found
+        if (position < 0)
         {
-            return this.length;
+            position = 0;
         }
-
+        else if (position > this.length)
+        {
+            position = this.length;
+        }
+
         const found = strstr(<string>Ref(this[position]), searchString);
         if (found == null)
         {
@@ -729,10 +756,22 @@ namespace __String {
         return true;
     }    
 
-    export function lastIndexOf(this: string, searchString: string, position = this.length): int {    
-        if (!searchString)
+    export function lastIndexOf(this: string, searchString: string, position: int = this.length): int {
+        // the position is clamped to [0, length] first: past the end the search read after the
+        // terminator, and "" is found at the clamped position
+        if (position < 0)
         {
-            return position;
+            position = 0;
+        }
+        else if (position > this.length)
+        {
+            position = this.length;
+        }
+
+        // a null check: "" is falsy
+        if (searchString == null)
+        {
+            return -1;
         }
 
         const searchStringLen = searchString.length;
@@ -741,11 +780,6 @@ namespace __String {
             return position;
         }
 
-        if (position < 0) 
-        {
-            position = 0;
-        }
-
         for (let i = position; i >= 0; i--)
         {
             const found = strncmp(<string>Ref(this[i]), searchString, searchStringLen);
@@ -916,7 +950,7 @@ namespace __String {
         return regexp.search(this);
     }
 
-    export function slice(this: string, indexStart: int, indexEnd = this.length): string {
+    export function slice(this: string, indexStart: int, indexEnd: int = this.length): string {
 
         if (indexStart < 0) {
             if (-this.length <= indexStart) {
@@ -936,7 +970,13 @@ namespace __String {
             }
         } else if (indexEnd >= this.length) {
             indexEnd = this.length;
-        }        
+        }
+
+        // an end at or before the start is empty - slice does not swap, as substring does; the
+        // count was negative and the copy ran past the result (#27)
+        if (indexEnd <= indexStart) {
+            return "";
+        }
 
         const count = indexEnd - indexStart;
         const newString = "".clone().resize(count);
@@ -1003,10 +1043,16 @@ namespace __String {
             return false;
         }
 
+        // a negative position is 0: it read before the string
+        if (position < 0)
+        {
+            position = 0;
+        }
+
         return strncmp(<string>Ref(this[position]), searchString, lensuffix) == 0;
     }    
 
-    export function substring(this: string, indexStart: int, indexEnd = this.length): string {
+    export function substring(this: string, indexStart: int, indexEnd: int = this.length): string {
         if (indexStart < 0) {
             indexStart = 0;
         } else if (indexStart >= this.length) {
@@ -1070,36 +1116,36 @@ namespace __String {
         return this;
     }    
 
-    export function trim(this: string): string {
-        let start = 0;
-        for (let i = 0; i < this.length; i++) {
-            if (!isspace(this[i])) { start = i; break; }
+    // the first character that is not a space, or the length when every one is
+    function trimmedStart(this: string): int {
+        for (let i: int = 0; i < this.length; i++) {
+            if (!isspace(this[i])) return i;
         }
 
-        let end = this.length - 1;
-        for (let i = this.length - 1; i >= 0; i--) {
-            if (!isspace(this[i])) { end = i; break; }
+        return this.length;
+    }
+
+    // one past the last character that is not a space, or 0 when every one is: substring's end is
+    // exclusive, and the end used to be the last kept character, so it was dropped
+    function trimmedEnd(this: string): int {
+        for (let i: int = this.length - 1; i >= 0; i--) {
+            if (!isspace(this[i])) return i + 1;
         }
 
-        return this.substring(start, end);
+        return 0;
     }
 
-    export function trimStart(this: string): string {
-        let start = 0;
-        for (let i = 0; i < this.length; i++) {
-            if (!isspace(this[i])) { start = i; break; }
-        }
+    export function trim(this: string): string {
+        const start = this.trimmedStart();
+        return start == this.length ? "" : this.substring(start, this.trimmedEnd());
+    }
 
-        return this.substring(start);
+    export function trimStart(this: string): string {
+        return this.substring(this.trimmedStart());
     }    
 
     export function trimEnd(this: string): string {
-        let end = this.length - 1;
-        for (let i = this.length - 1; i >= 0; i--) {
-            if (!isspace(this[i])) { end = i; break; }
-        }
-
-        return this.substring(0, end);
+        return this.substring(0, this.trimmedEnd());
     }    
 
     export function valueOf(this: string): string {

tests/string_bounds.ts

@@ -0,0 +1,73 @@
+// A position or index outside the string is clamped, as in JavaScript, before it becomes a pointer
+// into the string or a byte count: slice with its end before its start computed a negative count
+// and copied past the result (#27); a negative position read before the string; a position past
+// the end read after its terminator. trim, which dropped the last kept character, is here too.
+function main() {
+    const s = "abcdef";
+
+    // slice: an end at or before the start is empty - slice does not swap, substring does
+    assert(s.slice(4, 1) == "", "slice(4, 1)");
+    assert(s.slice(-1, -3) == "", "slice(-1, -3)");
+    assert(s.slice(3, 3) == "", "slice(3, 3)");
+    assert(s.slice(2, 4) == "cd", "slice(2, 4)");
+    assert(s.slice(-3) == "def", "slice(-3)");
+    assert(s.slice(-10, 2) == "ab", "slice(-10, 2)");
+    assert(s.slice(4, 100) == "ef", "slice(4, 100)");
+    assert(s.slice(10) == "", "slice(10)");
+    assert(s.substring(4, 1) == "bcd", "substring(4, 1) swaps");
+
+    // a negative end counts from the end: the parameter was the type of `length`, unsigned, so a
+    // negative end became huge and was clamped to the length
+    assert(s.slice(1, -3) == "bc", "slice(1, -3)");
+    assert(s.slice(-1, 3) == "", "slice(-1, 3)");
+    assert(s.substring(1, -3) == "a", "substring(1, -3) is substring(0, 1)");
+    assert(!s.endsWith("ab", -1), "endsWith('ab', -1)");
+
+    // startsWith: a negative position is 0
+    assert(s.startsWith("ab", -5), "startsWith('ab', -5)");
+    assert(!s.startsWith("b", -5), "startsWith('b', -5)");
+
+    // endsWith: the end position is clamped to [0, length]
+    assert(s.endsWith("ef", 100), "endsWith('ef', 100)");
+    assert(s.endsWith("ab", 2), "endsWith('ab', 2)");
+    assert(!s.endsWith("a", -5), "endsWith('a', -5)");
+    assert(s.endsWith("", -5), "endsWith('', -5)");
+
+    // includes: a negative position is 0, one past the end finds only ""
+    assert(s.includes("a", -5), "includes('a', -5)");
+    assert(!s.includes("f", 10), "includes('f', 10)");
+    assert(s.includes("", 10), "includes('', 10)");
+
+    // indexOf: as includes, and past the end nothing but "" is found
+    assert(s.indexOf("b", -5) == 1, "indexOf('b', -5)");
+    assert(s.indexOf("x", 10) == -1, "indexOf('x', 10)");
+    assert(s.indexOf("f", 10) == -1, "indexOf('f', 10)");
+    assert(s.indexOf("", 10) == 6, "indexOf('', 10)");
+
+    // lastIndexOf: the position is clamped to [0, length]
+    assert(s.lastIndexOf("a", 100) == 0, "lastIndexOf('a', 100)");
+    assert(s.lastIndexOf("f", 100) == 5, "lastIndexOf('f', 100)");
+    assert(s.lastIndexOf("", 100) == 6, "lastIndexOf('', 100)");
+    assert(s.lastIndexOf("a", -5) == 0, "lastIndexOf('a', -5)");
+    assert(s.lastIndexOf("b", -5) == -1, "lastIndexOf('b', -5)");
+
+    // padStart / padEnd: an empty pad string pads nothing
+    assert("x".padStart(5, "") == "x", "padStart(5, '')");
+    assert("x".padEnd(5, "") == "x", "padEnd(5, '')");
+    assert("x".padStart(3, "ab") == "abx", "padStart(3, 'ab')");
+    assert("x".padEnd(4, "ab") == "xaba", "padEnd(4, 'ab')");
+
+    // trim: substring's end is exclusive, and the last kept character was passed as the end
+    assert("  ab  ".trim() == "ab", "trim");
+    assert("ab".trim() == "ab", "trim, nothing to trim");
+    assert("   ".trim() == "", "trim, all spaces");
+    assert("".trim() == "", "trim, empty");
+    assert("  ab  ".trimStart() == "ab  ", "trimStart");
+    assert("   ".trimStart() == "", "trimStart, all spaces");
+    assert("  ab  ".trimEnd() == "  ab", "trimEnd");
+    assert("ab".trimEnd() == "ab", "trimEnd, nothing to trim");
+    assert("   ".trimEnd() == "", "trimEnd, all spaces");
+    assert("".trimEnd() == "", "trimEnd, empty");
+
+    console.log("ALL DONE");
+}