Skip to content

Commit

Permalink
Revert PR multitheftauto#3620 (Read file from path) due to bugs persi…
Browse files Browse the repository at this point in the history
…sting

After testing, it turns out not to be stable yet, it could be reproduced on FFS server before they changed something (on which the exact details they still need to collaborate with Tracer. We can't accept that a PR may break certain servers due to their specific use of texture/PNG implementations). Investigation into this took place in a gated channel within MTA discord, but will ideally move to dev discord should collaboration to get to the bottom of why the issues still aren't resolved, resume.
  • Loading branch information
Dutchman101 committed Aug 14, 2024
1 parent fb189bb commit 1532130
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 294 deletions.
16 changes: 2 additions & 14 deletions Client/mods/deathmatch/logic/CScriptFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ long CScriptFile::Read(unsigned long ulSize, SString& outBuffer)
DoResourceFileCheck();

// If read size is large, limit it to how many bytes can be read (avoid memory problems with over allocation)
// large : >10KB
if (ulSize > 10240)
if (ulSize > 10000)
{
long lCurrentPos = m_pFile->FTell();
m_pFile->FSeek(0, SEEK_END);
Expand All @@ -178,18 +177,7 @@ long CScriptFile::Read(unsigned long ulSize, SString& outBuffer)
return -2;
}

auto bytesRead = m_pFile->FRead(outBuffer.data(), ulSize);

// EOF reached?
// Cant check for error as binary interface
// is pure virtual class with no definitions
// available (CNetServer)
if (m_pFile->FEof())
{
// if so, truncate the data to the amount of bytes read
outBuffer.resize(bytesRead + 1);
}
return bytesRead;
return m_pFile->FRead(outBuffer.data(), ulSize);
}

long CScriptFile::Write(unsigned long ulSize, const char* pData)
Expand Down
15 changes: 0 additions & 15 deletions Client/mods/deathmatch/logic/lua/CLuaFunctionParseHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1268,11 +1268,6 @@ void CheckCanModifyOtherResource(CScriptArgReader& argStream, CResource* pThisRe
// No operation on the client
}

std::pair<bool, std::string> CheckCanModifyOtherResource(CResource* pThisResource, CResource* pOtherResource) noexcept
{
return {true, ""};
}

//
// Set error if pThisResource does not have permission to modify every resource in resourceList
//
Expand All @@ -1281,11 +1276,6 @@ void CheckCanModifyOtherResources(CScriptArgReader& argStream, CResource* pThisR
// No operation on the client
}

std::pair<bool, std::string> CheckCanModifyOtherResources(CResource* pThisResource, std::initializer_list<CResource*> resourceList) noexcept
{
return {true, ""};
}

//
// Set error if resource file access is blocked due to reasons
//
Expand All @@ -1294,8 +1284,3 @@ void CheckCanAccessOtherResourceFile(CScriptArgReader& argStream, CResource* pTh
{
// No operation on the client
}

std::pair<bool, std::string> CheckCanAccessOtherResourceFile(CResource* pThisResource, CResource* pOtherResource, const SString& strAbsPath, bool* pbReadOnly) noexcept
{
return {true, ""};
}
6 changes: 1 addition & 5 deletions Client/mods/deathmatch/logic/lua/CLuaFunctionParseHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -590,13 +590,9 @@ void ReadPregFlags(CScriptArgReader& argStream, pcrecpp::RE_Options& pOptions);
// Resource access helpers
//
void CheckCanModifyOtherResource(CScriptArgReader& argStream, CResource* pThisResource, CResource* pOtherResource);
std::pair<bool, std::string> CheckCanModifyOtherResource(CResource* pThisResource, CResource* pOtherResource) noexcept;
void CheckCanModifyOtherResources(CScriptArgReader& argStream, CResource* pThisResource, std::initializer_list<CResource*> resourceList);
std::pair<bool, std::string> CheckCanModifyOtherResources(CResource* pThisResource, std::initializer_list<CResource*> resourceList) noexcept;
void CheckCanAccessOtherResourceFile(CScriptArgReader& argStream, CResource* pThisResource, CResource* pOtherResource, const SString& strAbsPath,
void CheckCanAccessOtherResourceFile(CScriptArgReader& argStream, CResource* pThisResource, CResource* pOtherResource, const SString& strAbsPath,
bool* pbReadOnly = nullptr);
std::pair<bool, std::string> CheckCanAccessOtherResourceFile(CResource* pThisResource, CResource* pOtherResource, const SString& strAbsPath,
bool* pbReadOnly = nullptr) noexcept;

//
// Other misc helpers
Expand Down
13 changes: 2 additions & 11 deletions Server/mods/deathmatch/logic/CScriptFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,7 @@ long CScriptFile::Read(unsigned long ulSize, SString& outBuffer)
return -1;

// If read size is large, limit it to how many bytes can be read (avoid memory problems with over allocation)
// large : >10KB
if (ulSize > 10240)
if (ulSize > 10000)
{
long lCurrentPos = ftell(m_pFile);
fseek(m_pFile, 0, SEEK_END);
Expand All @@ -198,15 +197,7 @@ long CScriptFile::Read(unsigned long ulSize, SString& outBuffer)
return -2;
}

auto bytesRead = fread(outBuffer.data(), 1, ulSize, m_pFile);

// EOF reached or error was thrown?
if (feof(m_pFile) || ferror(m_pFile))
{
// if so, truncate the data to the amount of bytes read
outBuffer.resize(bytesRead + 1);
}
return bytesRead;
return fread(outBuffer.data(), 1, ulSize, m_pFile);
}

long CScriptFile::Write(unsigned long ulSize, const char* pData)
Expand Down
76 changes: 0 additions & 76 deletions Server/mods/deathmatch/logic/lua/CLuaFunctionParseHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -746,17 +746,6 @@ void CheckCanModifyOtherResource(CScriptArgReader& argStream, CResource* pThisRe
"Access denied");
}

std::pair<bool, SString> CheckCanModifyOtherResource(CResource* pThisResource, CResource* pOtherResource) noexcept
{
if (GetResourceModifyScope(pThisResource, pOtherResource) != eResourceModifyScope::NONE)
return {true, ""};

SString str("ModifyOtherObjects in ACL denied resource %s to access %s",
pThisResource->GetName().c_str(), pOtherResource->GetName().c_str()
);
return {false, str};
}

//
// Set error if pThisResource does not have permission to modify every resource in resourceList
//
Expand Down Expand Up @@ -798,46 +787,6 @@ void CheckCanModifyOtherResources(CScriptArgReader& argStream, CResource* pThisR
SString("ModifyOtherObjects in ACL denied resource %s to access %s", pThisResource->GetName().c_str(), ssResourceNames.str().c_str()), "Access denied");
}

std::pair<bool, SString> CheckCanModifyOtherResources(CResource* pThisResource, std::initializer_list<CResource*> resourceList) noexcept
{
// std::unordered_set only allows unique values and resourceList can contain duplicates
std::unordered_set<CResource*> setNoPermissionResources;

for (const auto& pOtherResource : resourceList)
{
eResourceModifyScope modifyScope = GetResourceModifyScope(pThisResource, pOtherResource);

if (modifyScope == eResourceModifyScope::SINGLE_RESOURCE)
continue;

if (modifyScope == eResourceModifyScope::EVERY_RESOURCE)
return {true, ""};

setNoPermissionResources.emplace(pOtherResource);
}

if (setNoPermissionResources.empty())
return {true, ""};

std::stringstream ssResourceNames;
std::size_t remainingElements = setNoPermissionResources.size();

for (const auto& pResource : setNoPermissionResources)
{
ssResourceNames << pResource->GetName();

if (remainingElements > 1)
ssResourceNames << ", ";

--remainingElements;
}

SString str("ModifyOtherObjects in ACL denied resource %s to access %s",
pThisResource->GetName().c_str(), ssResourceNames.str().c_str()
);
return {false, str};
}

//
// Set error if resource file access is blocked due to reasons
//
Expand All @@ -864,28 +813,3 @@ void CheckCanAccessOtherResourceFile(CScriptArgReader& argStream, CResource* pTh
SString("Database credentials protection denied resource %s to access %s", *pThisResource->GetName(), *pOtherResource->GetName()), "Access denied");
}
}

std::pair<bool, SString> CheckCanAccessOtherResourceFile(CResource* pThisResource, CResource* pOtherResource, const SString& strAbsPath, bool* pbReadOnly) noexcept
{
if (!g_pGame->GetConfig()->IsDatabaseCredentialsProtectionEnabled())
return {true, ""};

// Is other resource different and requested access denied
if (pThisResource == pOtherResource)
return {true, ""};

if (!pOtherResource->IsFileDbConnectMysqlProtected(strAbsPath, pbReadOnly ? *pbReadOnly : false))
return {true, ""};

// No access - See if we can change to readonly
if (pbReadOnly && !(*pbReadOnly) && !pOtherResource->IsFileDbConnectMysqlProtected(strAbsPath, true)) {
// Yes readonly access
*pbReadOnly = true;
return {true, ""};
}

SString str("Database credentials protection denied resource %s to access %s",
*pThisResource->GetName(), *pOtherResource->GetName()
);
return {false, str};
}
5 changes: 1 addition & 4 deletions Server/mods/deathmatch/logic/lua/CLuaFunctionParseHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -403,12 +403,9 @@ enum class eResourceModifyScope

eResourceModifyScope GetResourceModifyScope(CResource* pThisResource, CResource* pOtherResource);
void CheckCanModifyOtherResource(CScriptArgReader& argStream, CResource* pThisResource, CResource* pOtherResource);
std::pair<bool, SString> CheckCanModifyOtherResource(CResource* pThisResource, CResource* pOtherResource) noexcept;
void CheckCanModifyOtherResources(CScriptArgReader& argStream, CResource* pThisResource, std::initializer_list<CResource*> resourceList);
std::pair<bool, SString> CheckCanModifyOtherResources(CResource* pThisResource, std::initializer_list<CResource*> resourceList) noexcept;
void CheckCanModifyOtherResources(CScriptArgReader& argStream, CResource* pThisResource, std::initializer_list<CResource*> resourceList);
void CheckCanAccessOtherResourceFile(CScriptArgReader& argStream, CResource* pThisResource, CResource* pOtherResource, const SString& strAbsPath,
bool* pbReadOnly = nullptr);
std::pair<bool, SString> CheckCanAccessOtherResourceFile(CResource* pThisResource, CResource* pOtherResource, const SString& strAbsPath, bool* pbReadOnly = nullptr) noexcept;

//
// Other misc helpers
Expand Down
Loading

0 comments on commit 1532130

Please sign in to comment.