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 length was not correct, or the string passed in was not nul-terminated, the buffer would overflow. - If the string passed in contained embedded nul characters (e.g, in 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 allow using this concat method to pass in strings that are not nul-terminated, its behaviour should change. 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 · matthijskooijman · Mar 16, 2014 · Mar 16, 2014 · Mar 16, 2014 · Mar 16, 2014
commit c63b861241eed1340ad56c0d05be9b65b90c91cb
diff --git a/hardware/arduino/avr/cores/arduino/WString.cpp b/hardware/arduino/avr/cores/arduino/WString.cpp
@@ -174,7 +174,7 @@ String & String::copy(const char *cstr, unsigned int length)
 		return *this;
 	}
 	len = length;
-	strcpy(buffer, cstr);
+	memcpy(buffer, cstr, length);
 	return *this;
 }

@@ -194,7 +194,7 @@ void String::move(String &rhs)
 {
 	if (buffer) {
 		if (capacity >= rhs.len) {
-			strcpy(buffer, rhs.buffer);
+			memcpy(buffer, rhs.buffer, rhs.len);
 			len = rhs.len;
 			rhs.len = 0;
 			return;
@@ -266,7 +266,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;
 }

diff --git a/hardware/arduino/sam/cores/arduino/WString.cpp b/hardware/arduino/sam/cores/arduino/WString.cpp
@@ -176,7 +176,7 @@ String & String::copy(const char *cstr, unsigned int length)
 		return *this;
 	}
 	len = length;
-	strcpy(buffer, cstr);
+	memcpy(buffer, cstr, length);
 	return *this;
 }

@@ -196,7 +196,7 @@ void String::move(String &rhs)
 {
 	if (buffer) {
 		if (capacity >= rhs.len) {
-			strcpy(buffer, rhs.buffer);
+			memcpy(buffer, rhs.buffer, rhs.len);
 			len = rhs.len;
 			rhs.len = 0;
 			return;
@@ -268,7 +268,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;
 }