Make string concat, copy and move work on non-terminated strings

Previously, these methods took a nul-terminated string and appended it to the current buffer. The length argument (or length of the passed String object) was used to allocate memory, but was not used when actually copying the string. This meant that: - If the passed length was too short, or the string passed in was not nul-terminated, the buffer would overflow. - If the string passed in contained embedded nul characters (i.e, among the first length characters), any characters after the embedded nul would not be copied (but len would be updated to indicate they were). In practice, neither of the above would occur, since the length passed is always the known length of the string, usually as returned by strlen. However, to make this method public, and to allow using this concat method to pass in strings that are not nul-terminated, it should be changed to be more robust. This commit changes the method to use memcpy instead of strcpy, copying exactly the number of bytes passed in. For the current calls to this method, which pass a nul-terminated string, without embedded nul characters and a correct length, this behaviour should not change. However, this concat method can now also be used in the two cases mentioned above. Non-nul-terminated strings now work as expected and for strings with embedded newlines the entire string is copied as-is, instead of leaving uninitialized memory after the embedded nul byte. Note that a lot of operations will still only see the string up to the embedded nul byte, but that's not really fixable unless we reimplement functions like strcmp.
arduino · aentinger · Jan 25, 2021 · Mar 16, 2014 · Mar 16, 2014 · Mar 16, 2014
commit 59dd9950ff797a803fb449983ae9fb3e1c436406
diff --git a/api/String.cpp b/api/String.cpp
@@ -192,7 +192,7 @@ String & String::copy(const char *cstr, unsigned int length)
 		return *this;
 	}
 	len = length;
-	strcpy(buffer, cstr);
+	memcpy(buffer, cstr, length);
 	return *this;
 }
 
@@ -212,7 +212,7 @@ void String::move(String &rhs)
 {
 	if (buffer) {
 		if (rhs && capacity >= rhs.len) {
-			strcpy(buffer, rhs.buffer);
+			memcpy(buffer, rhs.buffer, rhs.len);
 			len = rhs.len;
 			rhs.len = 0;
 			return;
@@ -284,7 +284,7 @@ unsigned char String::concat(const char *cstr, unsigned int length)
 	if (!cstr) return 0;
 	if (length == 0) return 1;
 	if (!reserve(newlen)) return 0;
-	strcpy(buffer + len, cstr);
+	memcpy(buffer + len, cstr, length);
 	len = newlen;
 	return 1;
 }