Merge #2726

2726: Only send valid UTF-8 over the network r=def- a=heinrich5991



Co-authored-by: heinrich5991 <heinrich5991@gmail.com>

CMakeLists.txt

@@ -2083,6 +2083,7 @@ if(GTEST_FOUND OR DOWNLOAD_GTEST)
     json.cpp
     mapbugs.cpp
     name_ban.cpp
+    packer.cpp
     prng.cpp
     str.cpp
     strip_path_and_extension.cpp

src/engine/shared/packer.cpp

@@ -33,36 +33,34 @@ void CPacker::AddString(const char *pStr, int Limit)
 	if(m_Error)
 		return;
 
-	//
+	if(Limit <= 0)
-	if(Limit > 0)
 	{
-		while(*pStr && Limit != 0)
+		Limit = PACKER_BUFFER_SIZE;
-		{
-			*m_pCurrent++ = *pStr++;
-			Limit--;
-
-			if(m_pCurrent >= m_pEnd)
-			{
-				m_Error = 1;
-				break;
-			}
-		}
-		*m_pCurrent++ = 0;
 	}
-	else
+	while(*pStr && Limit != 0)
 	{
-		while(*pStr)
+		int Codepoint = str_utf8_decode(&pStr);
+		if(Codepoint == -1)
 		{
-			*m_pCurrent++ = *pStr++;
+			Codepoint = 0xfffd; // Unicode replacement character.
-
-			if(m_pCurrent >= m_pEnd)
-			{
-				m_Error = 1;
-				break;
-			}
 		}
-		*m_pCurrent++ = 0;
+		char aGarbage[4];
+		int Length = str_utf8_encode(aGarbage, Codepoint);
+		if(Limit < Length)
+		{
+			break;
+		}
+		// Ensure space for the null termination.
+		if(m_pEnd - m_pCurrent < Length + 1)
+		{
+			m_Error = 1;
+			break;
+		}
+		Length = str_utf8_encode((char *)m_pCurrent, Codepoint);
+		m_pCurrent += Length;
+		Limit -= Length;
 	}
+	*m_pCurrent++ = 0;
 }
 
 void CPacker::AddRaw(const void *pData, int Size)

src/engine/shared/packer.h

@@ -7,11 +7,12 @@
 
 class CPacker
 {
+public:
 	enum
 	{
 		PACKER_BUFFER_SIZE=1024*2
 	};
-
+private:
 	unsigned char m_aBuffer[PACKER_BUFFER_SIZE];
 	unsigned char *m_pCurrent;
 	unsigned char *m_pEnd;

src/test/packer.cpp

@@ -0,0 +1,64 @@
+#include "test.h"
+#include <gtest/gtest.h>
+
+#include <base/system.h>
+#include <engine/shared/packer.h>
+
+// pExpected is NULL if an error is expected
+static void ExpectAddString5(const char *pString, int Limit, const char *pExpected)
+{
+	static char ZEROS[CPacker::PACKER_BUFFER_SIZE] = {0};
+	static const int OFFSET = CPacker::PACKER_BUFFER_SIZE - 5;
+	CPacker Packer;
+	Packer.Reset();
+	Packer.AddRaw(ZEROS, OFFSET);
+	Packer.AddString(pString, Limit);
+
+	EXPECT_EQ(pExpected == 0, Packer.Error());
+	if(pExpected)
+	{
+		// Include null termination.
+		int ExpectedLength = str_length(pExpected) + 1;
+		EXPECT_EQ(ExpectedLength, Packer.Size() - OFFSET);
+		if(ExpectedLength == Packer.Size() - OFFSET)
+		{
+			EXPECT_STREQ(pExpected, (const char *)Packer.Data() + OFFSET);
+		}
+	}
+}
+
+TEST(Packer, AddString)
+{
+	ExpectAddString5("", 0, "");
+	ExpectAddString5("a", 0, "a");
+	ExpectAddString5("abcd", 0, "abcd");
+	ExpectAddString5("abcde", 0, 0);
+}
+
+TEST(Packer, AddStringLimit)
+{
+	ExpectAddString5("", 1, "");
+	ExpectAddString5("a", 1, "a");
+	ExpectAddString5("aa", 1, "a");
+	ExpectAddString5("ä", 1, "");
+
+	ExpectAddString5("", 10, "");
+	ExpectAddString5("a", 10, "a");
+	ExpectAddString5("abcd", 10, "abcd");
+	ExpectAddString5("abcde", 10, 0);
+
+	ExpectAddString5("äöü", 5, "äö");
+	ExpectAddString5("äöü", 6, 0);
+}
+
+TEST(Packer, AddStringBroken)
+{
+	ExpectAddString5("\x80", 0, "<EFBFBD>");
+	ExpectAddString5("\x80\x80", 0, 0);
+	ExpectAddString5("a\x80", 0, "a<EFBFBD>");
+	ExpectAddString5("\x80""a", 0, "<EFBFBD>a");
+	ExpectAddString5("\x80", 1, "");
+	ExpectAddString5("\x80\x80", 3, "<EFBFBD>");
+	ExpectAddString5("\x80\x80", 5, "<EFBFBD>");
+	ExpectAddString5("\x80\x80", 6, 0);
+}