From 34686c381941226d0f19daf5ac2f4c2ea9cb865a Mon Sep 17 00:00:00 2001 From: dreamstalker Date: Sun, 14 Jun 2015 22:56:25 +0400 Subject: [PATCH 1/2] Tests for Info_ functions --- rehlds/msvc/ReHLDS.vcxproj | 10 ++ rehlds/msvc/ReHLDS.vcxproj.filters | 3 + rehlds/unittests/info_tests.cpp | 159 +++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 rehlds/unittests/info_tests.cpp diff --git a/rehlds/msvc/ReHLDS.vcxproj b/rehlds/msvc/ReHLDS.vcxproj index 1621a5d..3fa0431 100644 --- a/rehlds/msvc/ReHLDS.vcxproj +++ b/rehlds/msvc/ReHLDS.vcxproj @@ -248,6 +248,16 @@ true true + + true + true + true + true + true + true + true + true + true true diff --git a/rehlds/msvc/ReHLDS.vcxproj.filters b/rehlds/msvc/ReHLDS.vcxproj.filters index 18ed7c4..2f416c4 100644 --- a/rehlds/msvc/ReHLDS.vcxproj.filters +++ b/rehlds/msvc/ReHLDS.vcxproj.filters @@ -337,6 +337,9 @@ unittests + + unittests + diff --git a/rehlds/unittests/info_tests.cpp b/rehlds/unittests/info_tests.cpp new file mode 100644 index 0000000..56cc5ca --- /dev/null +++ b/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); + } +} From 3cd4a52567a6b406887d4b6ca9142114522b8877 Mon Sep 17 00:00:00 2001 From: dreamstalker Date: Sun, 14 Jun 2015 23:08:03 +0400 Subject: [PATCH 2/2] Implemented strcpy_safe which is safe to use with overlapping src and dst buffers Use strcpy_safe in Info_ functions --- rehlds/engine/common.cpp | 6 ++ rehlds/engine/common.h | 3 + rehlds/engine/info.cpp | 5 +- rehlds/engine/tmessage.cpp | 140 ++++++++++++++++++------------------- 4 files changed, 81 insertions(+), 73 deletions(-) diff --git a/rehlds/engine/common.cpp b/rehlds/engine/common.cpp index f652253..368cece 100644 --- a/rehlds/engine/common.cpp +++ b/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; +} + /* ../engine/common.c:80 */ char *Info_Serverinfo(void) diff --git a/rehlds/engine/common.h b/rehlds/engine/common.h index 10adfe8..5c2f2f2 100644 --- a/rehlds/engine/common.h +++ b/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); diff --git a/rehlds/engine/info.cpp b/rehlds/engine/info.cpp index dfec4f9..96ffd26 100644 --- a/rehlds/engine/info.cpp +++ b/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 } } diff --git a/rehlds/engine/tmessage.cpp b/rehlds/engine/tmessage.cpp index 9e0a898..ab82a58 100644 --- a/rehlds/engine/tmessage.cpp +++ b/rehlds/engine/tmessage.cpp @@ -248,76 +248,76 @@ NOXREF int IsToken(const char *pText, const char *pTokenName) /* ../engine/tmessage.c:245 */ NOXREF int ParseDirective(const char *pText) { - if (pText && pText[0] == '$') - { - float tempFloat[8]; - if (IsToken(pText, "position")) - { - if (ParseFloats(pText, tempFloat, 2)) - { - gMessageParms.x = tempFloat[0]; - gMessageParms.y = tempFloat[1]; - } - } - else if (IsToken(pText, "effect")) - { - if (ParseFloats(pText, tempFloat, 1)) - { - gMessageParms.effect = (int)tempFloat[0]; - } - } - else if (IsToken(pText, "fxtime")) - { - if (ParseFloats(pText, tempFloat, 1)) - { - gMessageParms.fxtime = tempFloat[0]; - } - } - else if (IsToken(pText, "color2")) - { - if (ParseFloats(pText, tempFloat, 3)) - { - gMessageParms.r2 = (int)tempFloat[0]; - gMessageParms.g2 = (int)tempFloat[1]; - gMessageParms.b2 = (int)tempFloat[2]; - } - } - else if (IsToken(pText, "color")) - { - if (ParseFloats(pText, tempFloat, 3)) - { - gMessageParms.r1 = (int)tempFloat[0]; - gMessageParms.g1 = (int)tempFloat[1]; - gMessageParms.b1 = (int)tempFloat[2]; - } - } - else if (IsToken(pText, "fadein")) - { - if (ParseFloats(pText, tempFloat, 1)) - { - gMessageParms.fadein = tempFloat[0]; - } - } - else if (IsToken(pText, "fadeout")) - { - if (ParseFloats(pText, tempFloat, 3)) - { - gMessageParms.fadeout = tempFloat[0]; - } - } - else if (IsToken(pText, "holdtime")) - { - if (ParseFloats(pText, tempFloat, 3)) - { - gMessageParms.holdtime = tempFloat[0]; - } - } - else - { - Con_DPrintf("Unknown token: %s\n", pText); - } - return 1; - } + if (pText && pText[0] == '$') + { + float tempFloat[8]; + if (IsToken(pText, "position")) + { + if (ParseFloats(pText, tempFloat, 2)) + { + gMessageParms.x = tempFloat[0]; + gMessageParms.y = tempFloat[1]; + } + } + else if (IsToken(pText, "effect")) + { + if (ParseFloats(pText, tempFloat, 1)) + { + gMessageParms.effect = (int)tempFloat[0]; + } + } + else if (IsToken(pText, "fxtime")) + { + if (ParseFloats(pText, tempFloat, 1)) + { + gMessageParms.fxtime = tempFloat[0]; + } + } + else if (IsToken(pText, "color2")) + { + if (ParseFloats(pText, tempFloat, 3)) + { + gMessageParms.r2 = (int)tempFloat[0]; + gMessageParms.g2 = (int)tempFloat[1]; + gMessageParms.b2 = (int)tempFloat[2]; + } + } + else if (IsToken(pText, "color")) + { + if (ParseFloats(pText, tempFloat, 3)) + { + gMessageParms.r1 = (int)tempFloat[0]; + gMessageParms.g1 = (int)tempFloat[1]; + gMessageParms.b1 = (int)tempFloat[2]; + } + } + else if (IsToken(pText, "fadein")) + { + if (ParseFloats(pText, tempFloat, 1)) + { + gMessageParms.fadein = tempFloat[0]; + } + } + else if (IsToken(pText, "fadeout")) + { + if (ParseFloats(pText, tempFloat, 3)) + { + gMessageParms.fadeout = tempFloat[0]; + } + } + else if (IsToken(pText, "holdtime")) + { + if (ParseFloats(pText, tempFloat, 3)) + { + gMessageParms.holdtime = tempFloat[0]; + } + } + else + { + Con_DPrintf("Unknown token: %s\n", pText); + } + return 1; + } return 0; }