Merge pull request #36 from dreamstalker/_34_tests

Fixed #34: Implemented safe strcpy()

rehlds/engine/common.cpp

@@ -33,6 +33,12 @@ char serverinfo[MAX_INFO_STRING];
 char gpszVersionString[32];
 char gpszProductString[32];
 
+char* strcpy_safe(char* dst, char* src) {
+	int len = strlen(src);
+	memmove(dst, src, len + 1);
+	return dst;
+}
+
 
 /* <e875> ../engine/common.c:80 */
 char *Info_Serverinfo(void)

rehlds/engine/common.h

@@ -170,6 +170,9 @@ NOBODY uint64 Q_strtoull(char *str);
 
 #endif // Q_functions
 
+//strcpy that works correctly with overlapping src and dst buffers
+char* strcpy_safe(char* dst, char* src);
+
 int build_number(void);
 char *Info_Serverinfo(void);
 

rehlds/engine/info.cpp

@@ -30,7 +30,6 @@
 
 // NOTE: This file contains a lot of fixes that are not covered by REHLDS_FIXES define.
 // TODO: Most of the Info_ functions can be speedup via removing unneded copy of key and values.
-// TODO: We have a problem with Q_strcpy, because it maps to strcpy which have undefined behavior when strings overlaps (possibly we need to use memmove solution here)
 
 /*
 ===============
@@ -179,7 +178,7 @@ void Info_RemoveKey(char *s, const char *key)
 		// Compare keys
 		if (!Q_strncmp(key, pkey, cmpsize))
 		{
-			Q_strcpy(start, s);	// remove this part
+			strcpy_safe(start, s);	// remove this part
 			s = start;	// continue searching
 		}
 	}
@@ -245,7 +244,7 @@ void Info_RemovePrefixedKeys(char *s, const char prefix)
 
 		if (pkey[0] == prefix)
 		{
-			Q_strcpy(start, s);	// remove this part
+			strcpy_safe(start, s);	// remove this part
 			s = start;	// continue searching
 		}
 	}

rehlds/msvc/ReHLDS.vcxproj

@@ -248,6 +248,16 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release Play|Win32'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release Swds Play|Win32'">true</ExcludedFromBuild>
     </ClCompile>
+    <ClCompile Include="..\unittests\info_tests.cpp">
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug Swds|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug Play|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug Swds Play|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug Record|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release Play|Win32'">true</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release Swds Play|Win32'">true</ExcludedFromBuild>
+    </ClCompile>
     <ClCompile Include="..\unittests\mathlib_tests.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug Swds|Win32'">true</ExcludedFromBuild>

rehlds/msvc/ReHLDS.vcxproj.filters

@@ -337,6 +337,9 @@
     <ClCompile Include="..\unittests\unicode_tests.cpp">
       <Filter>unittests</Filter>
     </ClCompile>
+    <ClCompile Include="..\unittests\info_tests.cpp">
+      <Filter>unittests</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="..\hookers\memory.h">

rehlds/unittests/info_tests.cpp

@@ -0,0 +1,159 @@
+#include "precompiled.h"
+#include "cppunitlite/TestHarness.h"
+
+TEST(PrefixedKeysRemove, Info, 1000) {
+	struct testdata_t {
+		const char* inData;
+		const char* outData;
+	};
+
+	testdata_t testdata[] = {
+		{ "", "" },
+		{ "key\\value", "key\\value" },
+		{ "\\key\\value", "\\key\\value" },
+		{ "_key\\value", "" },
+		{ "\\_key\\value", "" },
+		{ "\\k\\v\\_u\\t\\y\\z", "\\k\\v\\y\\z" },
+		{ "\\_k\\v\\u\\t\\y\\z", "\\u\\t\\y\\z" },
+		{ "\\k\\v\\u\\t\\_y\\z", "\\k\\v\\u\\t" },
+		{ "\\k\\v\\_u\\t\\*yyyyyyy\\zzzzzzz", "\\k\\v\\*yyyyyyy\\zzzzzzz" },
+		{ "\\cl_updaterate\\60\\topcolor\\30\\_vgui_menus\\1\\_ah\\1\\_gm\\1ba5\\rate\\30000\\name\\aiaiaiaiaia\\*sid\\42893935\\model\\urban", "\\cl_updaterate\\60\\topcolor\\30\\rate\\30000\\name\\aiaiaiaiaia\\*sid\\42893935\\model\\urban" }
+	};
+
+	for (int i = 0; i < ARRAYSIZE(testdata); i++) {
+		testdata_t* d = &testdata[i];
+		char localInfo[256];
+		strcpy(localInfo, d->inData);
+		Info_RemovePrefixedKeys(localInfo, '_');
+		ZSTR_EQUAL("Invalid info string", d->outData, localInfo);
+	}
+
+}
+
+TEST(SetValueForStarKey, Info, 1000) {
+	struct testdata_t {
+		const char* initialInfo;
+		const char* key;
+		const char* value;
+		const char* finalInfo;
+	};
+
+	testdata_t testdata[] = {
+		// Behavior
+		{ "", "a", "b", "\\a\\b" },
+		{ "\\a\\b\\c\\d", "a", "b", "\\c\\d\\a\\b" },
+		{ "a\\b\\c\\d", "a", "b", "\\c\\d\\a\\b" },
+		{ "\\a\\b\\c\\d", "e", "f", "\\a\\b\\c\\d\\e\\f" },
+		{ "a\\b\\c\\d", "e", "f", "a\\b\\c\\d\\e\\f" },
+		{ "\\a\\b\\c\\d", "b", "c", "\\a\\b\\c\\d\\b\\c" },
+		{ "\\a\\b\\c\\d\\e\\f", "c", "q", "\\a\\b\\e\\f\\c\\q" },
+		
+
+		{ "\\a\\b\\c", "e", "f", "\\a\\b\\c\\e\\f" },
+		{ "\\a\\b\\c\\", "e", "f", "\\a\\b\\c\\\\e\\f" },
+		{ "\\a\\b\\\\c", "e", "f", "\\a\\b\\\\c\\e\\f" },
+
+		
+		//limits
+		{ //do nothing since 'team' is not important key
+			"\\cl_updaterate\\100\\topcolor\\60\\name\\abcdefghijklmnop\\*sid\\12332432525345\\_vgui_menus\\1\\model\\urban\\somelargeuselesskey\\12312321321323123123123213123123123123123123123123123123123123123dasdsad\\_cl_autowepswitch\\1",
+			"team",
+			"1234567890123456789012345678901234567890",
+			"\\cl_updaterate\\100\\topcolor\\60\\name\\abcdefghijklmnop\\*sid\\12332432525345\\_vgui_menus\\1\\model\\urban\\somelargeuselesskey\\12312321321323123123123213123123123123123123123123123123123123123dasdsad\\_cl_autowepswitch\\1",
+		},
+
+		{ 
+			"\\cl_updaterate\\100\\topcolor\\60\\name\\abcdefghijklmnop\\*sid\\12332432525345\\_vgui_menus\\1\\model\\urban\\somelargeuselesskey\\12312321321323123123123213123123123123123123123123123123123123123dasdsad\\_cl_autowepswitch\\1",
+			"*team",
+			"12345678901234567890123456789012345678",
+			"\\cl_updaterate\\100\\topcolor\\60\\name\\abcdefghijklmnop\\*sid\\12332432525345\\_vgui_menus\\1\\model\\urban\\_cl_autowepswitch\\1\\*team\\12345678901234567890123456789012345678",
+		},
+		
+		{
+			"\\cl_updaterate\\100\\topcolor\\60\\name\\abcdefghijklmnop\\*sid\\12332432525345\\_vgui_menus\\1\\model\\urban\\somelargeuselesskey\\12312321321323123123123213123123123123123123123123123123123123123dasdsad\\_cl_autowepswitch\\1",
+			"*team",
+			"1234567890123456789012345678901234567",
+			"\\cl_updaterate\\100\\topcolor\\60\\name\\abcdefghijklmnop\\*sid\\12332432525345\\_vgui_menus\\1\\model\\urban\\somelargeuselesskey\\12312321321323123123123213123123123123123123123123123123123123123dasdsad\\_cl_autowepswitch\\1\\*team\\1234567890123456789012345678901234567",
+		}
+	};
+
+	for (int i = 0; i < ARRAYSIZE(testdata); i++) {
+		testdata_t* d = &testdata[i];
+		char localInfo[257];
+		strcpy(localInfo, d->initialInfo);
+		localInfo[256] = 0;
+		localInfo[255] = 0;
+		Info_SetValueForStarKey(localInfo, d->key, d->value, 256);
+		int infoLen = strlen(localInfo);
+		CHECK("info string length", infoLen < 256);
+		ZSTR_EQUAL("Invalid info string", d->finalInfo, localInfo);
+	}
+}
+
+TEST(RemoveKeyValue, Info, 1000) {
+	struct testdata_t {
+		const char* initialInfo;
+		const char* key;
+		const char* finalInfo;
+	};
+
+	testdata_t testdata[] = {
+		{ "", "a", "" },
+		{ "\\a\\b", "a", "" },
+		{ "\\a\\", "a", "" },
+		{ "\\a\\\\", "a", "\\" },
+		{ "\\a", "a", "" },
+		{ "a", "a", "" },
+		{ "a\\", "a", "" },
+		{ "a\\b", "a", "" },
+		{ "a\\b\\", "a", "\\" },
+		{ "\\a\\b\\c\\d\\e\\f", "d", "\\a\\b\\c\\d\\e\\f" },
+		{ "\\a\\b\\c\\d\\e\\f", "c", "\\a\\b\\e\\f" },
+		{ "a\\b\\c\\d\\e\\f", "d", "a\\b\\c\\d\\e\\f" },
+		{ "a\\b\\c\\d\\e\\f", "c", "a\\b\\e\\f" },
+		{ "a\\b\\c\\d\\e\\f", "a", "\\c\\d\\e\\f" },
+	};
+
+	for (int i = 0; i < ARRAYSIZE(testdata); i++) {
+		testdata_t* d = &testdata[i];
+		char localInfo[256];
+		strcpy(localInfo, d->initialInfo);
+		Info_RemoveKey(localInfo, d->key);
+		ZSTR_EQUAL("Invalid info string", d->finalInfo, localInfo);
+	}
+}
+
+TEST(GetKeyValue, Info, 1000) {
+	struct testdata_t {
+		const char* info;
+		const char* key;
+		const char* result;
+	};
+
+	testdata_t testdata[] = {
+		{ "", "a", "" },
+		{ "\\a\\b", "a", "b" },
+		{ "\\a\\", "a", "" },
+		{ "\\a\\\\", "a", "" },
+		{ "\\a", "a", "" },
+		{ "a", "a", "" },
+		{ "a\\", "a", "" },
+		{ "a\\b", "a", "b" },
+		{ "a\\b\\", "a", "b" },
+		{ "\\a\\b\\c\\d\\e\\f", "d", "" },
+		{ "\\a\\b\\c\\d\\e\\f", "c", "d" },
+		{ "a\\b\\c\\d\\e\\f", "d", "" },
+		{ "a\\b\\c\\d\\e\\f", "c", "d" },
+
+		{ "a\\b\\c\\d\\e\\f", "e", "f" },
+		{ "\\a\\b\\c\\d\\e\\f", "e", "f" },
+
+	};
+
+	for (int i = 0; i < ARRAYSIZE(testdata); i++) {
+		testdata_t* d = &testdata[i];
+
+		const char* res = Info_ValueForKey(d->info, d->key);
+		ZSTR_EQUAL("Invalid info value", d->result, res);
+	}
+}