Fix String::replace() and clean up SSO setLen

Fixes esp8266#5883 and supercedes esp8266#5890

The replace() function was using len() while in the middle of buffer
operations.  In SSO mode len() is not stored separately and is a call to
strlen(), which may not be legal if you're in the middle of overwriting
the SSO buffer, as was the case in ::replace when the replacement string
was longer than the find string.  This caused potential garbage at the
end of the string when accessed.  Instead, just cache the length in a
local while doing the operation.

Also make setLength() explicitly write a 0 in the SSO buffer at the
specific offset.  It's probably not needed, but is safe to do and makes
logical sense.

Add in test cases from esp8266#5890 as well as some new ones that fail on the
unmodified core.

Author: earlephilhower

cores/esp8266/WString.cpp

@@ -759,9 +759,10 @@ void String::replace(const String& find, const String& replace) {
         while(index >= 0 && (index = lastIndexOf(find, index)) >= 0) {
             readFrom = wbuffer() + index + find.len();
             memmove(readFrom + diff, readFrom, len() - (readFrom - buffer()));
-            setLen(len() + diff);
-            wbuffer()[len()] = 0;
+	    int newLen = len() + diff;
             memcpy(wbuffer() + index, replace.buffer(), replace.len());
+            setLen(newLen);
+            wbuffer()[newLen] = 0;
             index--;
         }
     }

cores/esp8266/WString.h

@@ -264,7 +264,7 @@ class String {
         inline unsigned int len() const { return sso() ? strlen(sso_buf) : ptr.len; }
         inline unsigned int capacity() const { return sso() ? SSOSIZE - 1 : ptr.cap; }
         inline void setSSO(bool sso) { sso_buf[SSOSIZE - 1] = sso ? 0x00 : 0xff; }
-        inline void setLen(int len) { if (!sso()) ptr.len = len; }
+        inline void setLen(int len) { if (!sso()) ptr.len = len; else sso_buf[len] = 0; }
         inline void setCapacity(int cap) { if (!sso()) ptr.cap = cap; }
 	inline void setBuffer(char *buff) { if (!sso()) ptr.buff = buff; }
         // Buffer accessor functions

tests/host/core/test_string.cpp

@@ -83,6 +83,12 @@ TEST_CASE("String constructors", "[core][String]")
     REQUIRE(ssh == "3.14159_abcd");
     String flash = (F("hello from flash"));
     REQUIRE(flash == "hello from flash");
+    const char textarray[6] = {'h', 'e', 'l', 'l', 'o', 0};
+    String hello(textarray);
+    REQUIRE(hello == "hello");
+    String hello2;
+    hello2 = textarray;
+    REQUIRE(hello2 == "hello");
 }
 
 TEST_CASE("String concantenation", "[core][String]")
@@ -360,4 +366,58 @@ TEST_CASE("String SSO works", "[core][String]")
     REQUIRE(s == "0123456789abcdefghijklmnopqrstu");
     REQUIRE(s.length() == 31);
   }
+  s = "0123456789abcde";
+  s = s.substring(s.indexOf('a'));
+  REQUIRE(s == "abcde");
+  REQUIRE(s.length() == 5);
+}
+
+#include <new>
+void repl(const String& key, const String& val, String& s, boolean useURLencode)
+{
+    s.replace(key, val);
+}
+
+
+TEST_CASE("String SSO handles junk in memory", "[core][String]")
+{
+  // We fill the SSO space with garbage then construct an object in it and check consistency
+  // This is NOT how you want to use Strings outside of this testing!
+  unsigned char space[16];
+  String *s = (String*)space;
+  memset(space, 0xff, 16);
+  new(s) String;
+  REQUIRE(*s == "");
+
+  // Tests from #5883
+  bool useURLencode = false;
+  const char euro[4] = {(char)0xe2, (char)0x82, (char)0xac, 0}; // Unicode euro symbol
+  const char yen[3]   = {(char)0xc2, (char)0xa5, 0}; // Unicode yen symbol
+
+  memset(space, 0xff, 16);
+  new(s) String("%ssid%");
+  repl(("%ssid%"), "MikroTik", *s, useURLencode);
+  REQUIRE(*s == "MikroTik");
+
+  memset(space, 0xff, 16);
+  new(s) String("{E}");
+  repl(("{E}"), euro, *s, useURLencode);
+  REQUIRE(*s == "€");
+  memset(space, 0xff, 16);
+  new(s) String("&euro;");
+  repl(("&euro;"), euro, *s, useURLencode);
+  REQUIRE(*s == "€");
+  memset(space, 0xff, 16);
+  new(s) String("{Y}");
+  repl(("{Y}"), yen, *s, useURLencode);
+  REQUIRE(*s == "¥");
+  memset(space, 0xff, 16);
+  new(s) String("&yen;");
+  repl(("&yen;"), yen, *s, useURLencode);
+  REQUIRE(*s == "¥");
+
+  memset(space, 0xff, 16);
+  new(s) String("%sysname%");
+  repl(("%sysname%"), "CO2_defect", *s, useURLencode);
+  REQUIRE(*s == "CO2_defect");
 }