- Fixed: script cacher ignored newlines, causing wrong script line co…

…ntext data in server logs.

- Fixed: custom triggers not firing.
- Fixed: CUOClientVersion not parsing correctly new string format.
- Fixed: TOOLTIP Dialog keyword sending the cliloc number in hex notation if ini DecimalVariables == 0 (unsupported by client, making some of them crash).
- Fixed: some integer data type size inconsistencies.
- Fixed: initialization order fiasco for CAccounts at server shutdown.
- Changed: Str_To* functions to return a std::optional value, in order to get the best error handling (before, it depended on errno and there were ambiguous return values for invalid or "0" zero strings.
- Changed: Refactored CUOMobTypes.
- Changed: replaced localtime function with thread-safe alternatives, avoiding data race conditions.
- Added: stypecast.h. It has utilities to cast numbers with strong guarantees and error checking at compile time, to avoid undefined behaviors, involuntary wrong sign/size cast or overflowing.

cmake/toolchains/Windows-MSVC.cmake

@@ -116,9 +116,9 @@ function (toolchain_exe_stuff)
 
 	target_compile_options(spheresvr PRIVATE
 		${cxx_compiler_flags_common}
-		$<$<CONFIG:Release>: $<IF:$<BOOL:${RUNTIME_STATIC_LINK}>,/MT,/MD>	/EHsc /Oy /GL /GA /Gw /Gy /GF /GR-  $<IF:$<BOOL:${ENABLED_SANITIZER}>,/O1 /Zi,/O2>>
-		$<$<CONFIG:Nightly>: $<IF:$<BOOL:${RUNTIME_STATIC_LINK}>,/MT,/MD>	/EHa  /Oy /GL /GA /Gw /Gy /GF		$<IF:$<BOOL:${ENABLED_SANITIZER}>,/O1 /Zi,/O2>>
-		$<$<CONFIG:Debug>:	 $<IF:$<BOOL:${RUNTIME_STATIC_LINK}>,/MTd,/MDd> /EHsc /Oy- /MDd /ob1 /Od 		$<IF:$<BOOL:${ENABLED_SANITIZER}>,/Zi,/ZI>> #/Gs
+		$<$<CONFIG:Release>: $<IF:$<BOOL:${RUNTIME_STATIC_LINK}>,/MT,/MD>	  /EHa  /Oy /GL /GA /Gw /Gy /GF $<IF:$<BOOL:${ENABLED_SANITIZER}>,/O1 /Zi,/O2>>
+		$<$<CONFIG:Nightly>: $<IF:$<BOOL:${RUNTIME_STATIC_LINK}>,/MT,/MD>	  /EHa  /Oy /GL /GA /Gw /Gy /GF $<IF:$<BOOL:${ENABLED_SANITIZER}>,/O1 /Zi,/O2>>
+		$<$<CONFIG:Debug>:	 $<IF:$<BOOL:${RUNTIME_STATIC_LINK}>,/MTd,/MDd> /EHsc /Oy- /MDd /ob1 /Od /Gs	$<IF:$<BOOL:${ENABLED_SANITIZER}>,/Zi,/ZI>>
 		# ASan (and compilation for ARM arch) doesn't support edit and continue option (ZI)
 	)
 

src/CMakeSources.cmake

@@ -270,6 +270,7 @@ src/common/sphere_library/sstring.cpp
 src/common/sphere_library/sstring.h
 src/common/sphere_library/sstringobjs.cpp
 src/common/sphere_library/sstringobjs.h
+src/common/sphere_library/stypecast.h
 )
 SOURCE_GROUP (common\\sphere_library FILES ${spherelibrary_SRCS})
 

src/common/CCacheableScriptFile.cpp

@@ -99,21 +99,51 @@ bool CCacheableScriptFile::_Open(lpctstr ptcFilename, uint uiModeFlags)
             _fileContent = new std::vector<std::string>;
             _fileContent->reserve(iFileLength / 25);
 
+            auto _SkipOneEndline = [](lpctstr &str_end) -> size_t // return how much i have moved past
+            {
+                bool fDoneN = false, fDoneR = false;
+                lpctstr before = str_end;
+                while (true)
+                {
+                    const char ch = *str_end;
+                    if (ch == '\n' && !fDoneN)
+                    {
+                        fDoneN = true;
+                        ++str_end;
+                    }
+                    else if (ch == '\r' && !fDoneR)
+                    {
+                        fDoneR = true;
+                        ++str_end;
+                    }
+                    else
+                    {
+                        break;
+                    }
+                }
+                return str_end - before;
+            };
+
             const char *fileCursor = fileContentCopy.get();
             size_t uiFileCursorRemainingLegth = iFileLength;
-            ssize_t iStrLen;
+            ssize_t iStrLen = 0;
             for (;; fileCursor += (size_t)iStrLen, uiFileCursorRemainingLegth -= (size_t)iStrLen)
             {
                 if (uiFileCursorRemainingLegth == 0)
                     break;
 
                 iStrLen = sGetLine_StaticBuf(fileCursor, minimum(uiFileCursorRemainingLegth, SCRIPT_MAX_LINE_LEN));
                 if (iStrLen < 0)
-                break;
+                    break;
 
                 if (iStrLen < 1 /*|| (fileCursor[iStrLen] != '\n') It can also be a '\0' value, but it might not be necessary to check for either of the two...*/)
                 {
-                    ++ iStrLen; // Skip \n
+                    lpctstr str_end = fileCursor;
+                    size_t uiSkip = _SkipOneEndline(str_end);
+                    ASSERT(uiSkip > 0);
+                    //iStrLen = (ssize_t)std::max((size_t)1, uiSkip);
+                    iStrLen = (ssize_t)uiSkip;
+                    _fileContent->emplace_back();
                     continue;
                 }
 
@@ -128,24 +158,14 @@ bool CCacheableScriptFile::_Open(lpctstr ptcFilename, uint uiModeFlags)
 
                 const lpctstr str_start = (fUTF ? &fileCursor[3] : fileCursor);
                 size_t len_to_copy = (size_t)iStrLen - (fUTF ? 3 : 0);
-                while (len_to_copy > 0)
-                {
-                    const int ch = str_start[len_to_copy - 1];
-                    if (ch == '\n' || ch == '\r')
-                    {
-                        // Do not copy that, but skip it.
-                        len_to_copy -= 1;
-                        iStrLen += 1;
-                    }
-                    else {
-                        break;
-                    }
-                }
+                ASSERT(len_to_copy > 0);
+
+                // go past the end of this substring, there should be the delimiter/newline/etc, just skip it.
+                lpctstr str_end = &str_start[len_to_copy];
+                _SkipOneEndline(str_end);
+                _fileContent->emplace_back(str_start, len_to_copy);
+                iStrLen += (str_end - len_to_copy - str_start);
 
-                if (len_to_copy == 0)
-                    _fileContent->emplace_back();
-                else
-                    _fileContent->emplace_back(str_start, len_to_copy);
                 fFirstLine = false;
                 fUTF = false;
             }   // closes while

src/common/CPointBase.cpp

@@ -905,6 +905,7 @@ int CPointBase::Read( tchar * pszVal )
     CPointBase ptTest;
     ptTest.m_z = 0;
     ptTest.m_map = 0;
+    bool fError = false;
 
 	tchar * ppVal[4];
 	int iArgs = Str_ParseCmds( pszVal, ppVal, ARRAY_COUNT( ppVal ), " ,\t" );
@@ -914,7 +915,14 @@ int CPointBase::Read( tchar * pszVal )
 		case 4:	// m_map
 			if ( IsDigit(ppVal[3][0]))
 			{
-                ptTest.m_map = (uchar)(Str_ToUI(ppVal[3]));
+                const std::optional<uchar> from = Str_ToU8(ppVal[3]);
+                if (!from.has_value())
+                {
+                    fError = true;
+                    break;
+                }
+
+                ptTest.m_map = from.value();
 				if ( !g_MapList.IsMapSupported(ptTest.m_map) )
 				{
 					g_Log.EventError("Unsupported map #%d specified. Auto-fixing that to 0.\n", ptTest.m_map);
@@ -925,26 +933,48 @@ int CPointBase::Read( tchar * pszVal )
 		case 3: // m_z
 			if (IsDigit(ppVal[2][0]) || ppVal[2][0] == '-')
 			{
-				ptTest.m_z = (char)(Str_ToI(ppVal[2]));
+                const std::optional<char> from = Str_ToI8(ppVal[2]);
+                if (!from.has_value())
+                {
+                    fError = true;
+                    break;
+                }
+
+				ptTest.m_z = from.value();
 			}
 			FALLTHROUGH;
 		case 2:
 			if (IsDigit(ppVal[1][0]))
 			{
-				ptTest.m_y = (short)(Str_ToI(ppVal[1]));
+                const std::optional<short> from = Str_ToI16(ppVal[1]);
+                if (!from.has_value())
+                {
+                    fError = true;
+                    break;
+                }
+
+				ptTest.m_y = from.value();
 			}
 			FALLTHROUGH;
 		case 1:
 			if (IsDigit(ppVal[0][0]))
 			{
-				ptTest.m_x = (short)(Str_ToI(ppVal[0]));
+                std::optional<short> from = Str_ToI16(ppVal[0]);
+                if (!from.has_value())
+                {
+                    fError = true;
+                    break;
+                }
+
+				ptTest.m_x = from.value();
 			}
 			FALLTHROUGH;
 		case 0:
 			break;
 	}
 
-    if (!ptTest.IsValidPoint())
+	fError |= !ptTest.IsValidPoint();
+    if (fError)
     {
         InitPoint();
         return 0;

src/common/CScriptObj.cpp

@@ -1103,9 +1103,20 @@ bool CScriptObj::r_WriteVal( lpctstr ptcKey, CSString &sVal, CTextConsole * pSrc
             int iQty = Str_ParseCmds(const_cast<tchar*>(ptcKey), ppCmd, ARRAY_COUNT(ppCmd), ", ");
             if ( iQty < 3 )
                 return false;
-            int iPrefixCode = Str_ToI(ppCmd[0]);
-            int iCost = Str_ToI(ppCmd[1]);
-            CSString sHash = CBCrypt::HashBCrypt(ppCmd[2], iPrefixCode, maximum(4,minimum(31,iCost)));
+
+            std::optional<int> iconv;
+
+            iconv = Str_ToI(ppCmd[0]);
+            if (!iconv.has_value())
+                return false;
+            int iPrefixCode = *iconv;
+
+            iconv = Str_ToI(ppCmd[1]);
+            if (!iconv.has_value())
+                return false;
+            int iCost = *iconv;
+
+            CSString sHash(CBCrypt::HashBCrypt(ppCmd[2], iPrefixCode, maximum(4,minimum(31,iCost))));
             sVal.Format("%s", sHash.GetBuffer());
         } return true;
 
@@ -1115,6 +1126,7 @@ bool CScriptObj::r_WriteVal( lpctstr ptcKey, CSString &sVal, CTextConsole * pSrc
             int iQty = Str_ParseCmds(const_cast<tchar*>(ptcKey), ppCmd, ARRAY_COUNT(ppCmd), ", ");
             if ( iQty < 2 )
                 return false;
+
             bool fValidated = CBCrypt::ValidateBCrypt(ppCmd[0], ppCmd[1]);
             sVal.FormatVal((int)fValidated);
         } return true;
@@ -1897,7 +1909,7 @@ int CScriptObj::ParseScriptText(tchar * ptcResponse, CTextConsole * pSrc, int iF
 					pContext->_fParseScriptText_Brackets = false;
 
 					tchar* ptcRecurseParse = ptcResponse + i;
-					const size_t iLen = ParseScriptText(ptcRecurseParse, pSrc, 4, pArgs);
+					const int iLen = ParseScriptText(ptcRecurseParse, pSrc, 4, pArgs);
 
 					pContext->_fParseScriptText_Brackets = true;
 					-- pContext->_iParseScriptText_Reentrant;
@@ -2136,7 +2148,11 @@ bool CScriptObj::Execute_FullTrigger(CScript& s, CTextConsole* pSrc, CScriptTrig
 	CScriptObj* pRef = this;
 	if (iArgQty == 2)
 	{
-		CChar* pCharFound = CUID::CharFindFromUID(Str_ToI(piCmd[1]));
+        std::optional<dword> iconv = Str_ToU(piCmd[1]);
+        if (!iconv.has_value())
+            return false;
+
+		CChar* pCharFound = CUID::CharFindFromUID(*iconv);
 		if (pCharFound)
 			pRef = pCharFound;
 	}

src/common/CUOClientVersion.cpp

@@ -1,5 +1,6 @@
 #include "CUOClientVersion.h"
 #include "CExpression.h"
+#include "CLog.h"
 #include "sphereproto.h"
 #include <algorithm>
 #include <string>
@@ -18,11 +19,13 @@ uint CUOClientVersion::GetLegacyVersionNumber() const noexcept
     uint factor_revision    = 100;
     uint factor_minor       = 100'00;
     uint factor_major       = 100'00'00;
-    if (m_revision > 100) {
+    if (m_revision > 100)
+    {
         factor_minor *= 10;
         factor_major *= 10;
     }
-    if (m_minor > 100) {
+    if (m_minor > 100)
+    {
         factor_major *= 10;
     }
 
@@ -75,7 +78,7 @@ CUOClientVersion::CUOClientVersion(dword uiClientVersionNumber) noexcept :
             m_major = uiClientVersionNumber / 100'000'00;
             m_minor = (uiClientVersionNumber / 1000'00) % 100;
             m_revision = (uiClientVersionNumber / 100 ) % 1000;
-            
+
         }
         else if (uiClientVersionNumber > 10'00'000'00)
         {
@@ -120,13 +123,16 @@ CUOClientVersion::CUOClientVersion(lpctstr ptcVersion) noexcept :
     // Ranges algorithms not yet supported by Apple Clang...
     // const ptrdiff_t count = std::ranges::count(std::string_view(ptcVersion), '.');
     const auto svVersion = std::string_view(ptcVersion);
-    const auto count = std::count(svVersion.cbegin(), svVersion.cend(), '_');
+    const auto count = std::count(svVersion.cbegin(), svVersion.cend(), '.');
     if (count == 2)
         ApplyVersionFromStringOldFormat(ptcVersion);
     else if (count == 3)
-        ApplyVersionFromStringOldFormat(ptcVersion);
+        ApplyVersionFromStringNewFormat(ptcVersion);
     else
-        ASSERT(false); // Malformed string?
+    {
+        // Malformed string?
+        g_Log.Event(LOGL_CRIT|LOGM_CLIENTS_LOG|LOGM_NOCONTEXT, "Received malformed client version string?.\n");
+    }
 }
 
 
@@ -167,7 +173,7 @@ void CUOClientVersion::ApplyVersionFromStringOldFormat(lpctstr ptcVersion) noexc
     // Get version of old clients, which report the client version as ASCII string (eg: '5.0.2b')
 
     byte uiLetter = 0;
-    size_t uiMax = strlen(ptcVersion);
+    const size_t uiMax = strnlen(ptcVersion, 20);
     for (size_t i = 0; i < uiMax; ++i)
     {
         if (IsAlpha(ptcVersion[i]))
@@ -195,7 +201,7 @@ void CUOClientVersion::ApplyVersionFromStringNewFormat(lpctstr ptcVersion) noexc
 {
     // Get version of newer clients, which use only 4 numbers separated by dots (example: 6.0.1.1)
 
-    std::string_view sw(ptcVersion);
+    const std::string_view sw(ptcVersion);
     const size_t dot1 = sw.find_first_of('.', 0);
     const size_t dot2 = sw.find_first_of('.', dot1 + 1);
     const size_t dot3 = sw.find_first_of('.', dot2 + 1);
@@ -210,10 +216,10 @@ void CUOClientVersion::ApplyVersionFromStringNewFormat(lpctstr ptcVersion) noexc
 
     uint val1, val2, val3, val4;
     bool ok = true;
-    ok = ok && svtonum(sw1, val1);
-    ok = ok && svtonum(sw2, val2);
-    ok = ok && svtonum(sw3, val3);
-    ok = ok && svtonum(sw4, val4);
+    ok = ok && sv_to_num(sw1, &val1);
+    ok = ok && sv_to_num(sw2, &val2);
+    ok = ok && sv_to_num(sw3, &val3);
+    ok = ok && sv_to_num(sw4, &val4);
     if (!ok)
         return;
 

src/common/basic_threading.h

@@ -50,15 +50,15 @@
 
 #if MT_ENGINES == 1  //_DEBUG
 
-#define MT_ENGINE_SHARED_LOCK_SET       MT_SHARED_LOCK_SET
-#define MT_ENGINE_SHARED_LOCK           MT_SHARED_LOCK
-#define MT_ENGINE_SHARED_UNLOCK	       MT_SHARED_UNLOCK
-#define MT_ENGINE_UNIQUE_LOCK_SET       MT_UNIQUE_LOCK_SET
-#define MT_ENGINE_UNIQUE_UNLOCK         MT_UNIQUE_UNLOCK
-
-#define MT_ENGINE_RETURN(x)             MT_RETURN(x)
-#define MT_ENGINE_SHARED_LOCK_RETURN(x) MT_SHARED_LOCK_RETURN(x)
-#define MT_ENGINE_UNIQUE_LOCK_RETURN(x) MT_UNIQUE_LOCK_RETURN(x)
+    #define MT_ENGINE_SHARED_LOCK_SET       MT_SHARED_LOCK_SET
+    #define MT_ENGINE_SHARED_LOCK           MT_SHARED_LOCK
+    #define MT_ENGINE_SHARED_UNLOCK	       MT_SHARED_UNLOCK
+    #define MT_ENGINE_UNIQUE_LOCK_SET       MT_UNIQUE_LOCK_SET
+    #define MT_ENGINE_UNIQUE_UNLOCK         MT_UNIQUE_UNLOCK
+
+    #define MT_ENGINE_RETURN(x)             MT_RETURN(x)
+    #define MT_ENGINE_SHARED_LOCK_RETURN(x) MT_SHARED_LOCK_RETURN(x)
+    #define MT_ENGINE_UNIQUE_LOCK_RETURN(x) MT_UNIQUE_LOCK_RETURN(x)
 
 #else
 	#define MT_ENGINE_SHARED_LOCK_SET	      (void)0