4464: Skip the UTF-8 BOM on all read text files r=def- a=heinrich5991

Fixes #4462.

## Checklist

- [ ] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [x] Written a unit test if it works standalone, system.c especially
- [x] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: heinrich5991 <heinrich5991@gmail.com>
This commit is contained in:
bors[bot] 2021-12-17 21:18:58 +00:00 committed by GitHub
commit b4374b28f1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 105 additions and 25 deletions

View file

@ -2263,6 +2263,7 @@ if(GTEST_FOUND OR DOWNLOAD_GTEST)
fs.cpp
git_revision.cpp
hash.cpp
io.cpp
jobs.cpp
json.cpp
mapbugs.cpp

View file

@ -311,13 +311,13 @@ void mem_zero(void *block, unsigned size)
memset(block, 0, size);
}
IOHANDLE io_open(const char *filename, int flags)
IOHANDLE io_open_impl(const char *filename, int flags)
{
dbg_assert(flags == IOFLAG_READ || flags == IOFLAG_WRITE || flags == IOFLAG_APPEND, "flags must be read, write or append");
dbg_assert(flags == (IOFLAG_READ | IOFLAG_SKIP_BOM) || flags == IOFLAG_READ || flags == IOFLAG_WRITE || flags == IOFLAG_APPEND, "flags must be read, read+skipbom, write or append");
#if defined(CONF_FAMILY_WINDOWS)
WCHAR wBuffer[IO_MAX_PATH_LENGTH];
MultiByteToWideChar(CP_UTF8, 0, filename, -1, wBuffer, sizeof(wBuffer) / sizeof(WCHAR));
if(flags == IOFLAG_READ)
if((flags & IOFLAG_READ) != 0)
return (IOHANDLE)_wfsopen(wBuffer, L"rb", _SH_DENYNO);
if(flags == IOFLAG_WRITE)
return (IOHANDLE)_wfsopen(wBuffer, L"wb", _SH_DENYNO);
@ -325,7 +325,7 @@ IOHANDLE io_open(const char *filename, int flags)
return (IOHANDLE)_wfsopen(wBuffer, L"ab", _SH_DENYNO);
return 0x0;
#else
if(flags == IOFLAG_READ)
if((flags & IOFLAG_READ) != 0)
return (IOHANDLE)fopen(filename, "rb");
if(flags == IOFLAG_WRITE)
return (IOHANDLE)fopen(filename, "wb");
@ -335,6 +335,21 @@ IOHANDLE io_open(const char *filename, int flags)
#endif
}
IOHANDLE io_open(const char *filename, int flags)
{
IOHANDLE result = io_open_impl(filename, flags);
unsigned char buf[3];
if((flags & IOFLAG_SKIP_BOM) == 0 || !result)
{
return result;
}
if(io_read(result, buf, sizeof(buf)) != 3 || buf[0] != 0xef || buf[1] != 0xbb || buf[2] != 0xbf)
{
io_seek(result, 0, IOSEEK_START);
}
return result;
}
unsigned io_read(IOHANDLE io, void *buffer, unsigned size)
{
return fread(buffer, 1, size, (FILE *)io);

View file

@ -171,6 +171,7 @@ enum
IOFLAG_READ = 1,
IOFLAG_WRITE = 2,
IOFLAG_APPEND = 4,
IOFLAG_SKIP_BOM = 8,
IOSEEK_START = 0,
IOSEEK_CUR = 1,
@ -187,7 +188,7 @@ typedef struct IOINTERNAL *IOHANDLE;
Parameters:
filename - File to open.
flags - A set of flags. IOFLAG_READ, IOFLAG_WRITE, IOFLAG_APPEND.
flags - A set of flags. IOFLAG_READ, IOFLAG_WRITE, IOFLAG_APPEND, IOFLAG_SKIP_BOM.
Returns:
Returns a handle to the file on success and 0 on failure.

View file

@ -24,7 +24,7 @@ bool CGLSL::LoadShader(CGLSLCompiler *pCompiler, IStorage *pStorage, const char
{
if(m_IsLoaded)
return true;
IOHANDLE f = pStorage->OpenFile(pFile, IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE f = pStorage->OpenFile(pFile, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
std::vector<std::string> Lines;
if(f)

View file

@ -2403,12 +2403,12 @@ void CClient::ResetDDNetInfo()
bool CClient::IsDDNetInfoChanged()
{
IOHANDLE OldFile = m_pStorage->OpenFile(DDNET_INFO, IOFLAG_READ, IStorage::TYPE_SAVE);
IOHANDLE OldFile = m_pStorage->OpenFile(DDNET_INFO, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_SAVE);
if(!OldFile)
return true;
IOHANDLE NewFile = m_pStorage->OpenFile(m_aDDNetInfoTmp, IOFLAG_READ, IStorage::TYPE_SAVE);
IOHANDLE NewFile = m_pStorage->OpenFile(m_aDDNetInfoTmp, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_SAVE);
if(NewFile)
{
@ -3385,7 +3385,7 @@ void CClient::Run()
s_SavedConfig = true;
}
IOHANDLE File = m_pStorage->OpenFile(m_aDDNetInfoTmp, IOFLAG_READ, IStorage::TYPE_SAVE);
IOHANDLE File = m_pStorage->OpenFile(m_aDDNetInfoTmp, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_SAVE);
if(File)
{
io_close(File);

View file

@ -1487,7 +1487,7 @@ int CServerBrowser::HasRank(const char *pMap)
void CServerBrowser::LoadDDNetInfoJson()
{
IOHANDLE File = m_pStorage->OpenFile(DDNET_INFO, IOFLAG_READ, IStorage::TYPE_SAVE);
IOHANDLE File = m_pStorage->OpenFile(DDNET_INFO, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_SAVE);
if(!File)
return;

View file

@ -447,7 +447,7 @@ IServerBrowserHttp *CreateServerBrowserHttp(IEngine *pEngine, IConsole *pConsole
const char *apUrls[CChooseMaster::MAX_URLS] = {0};
const char **ppUrls = apUrls;
int NumUrls = 0;
IOHANDLE File = pStorage->OpenFile("ddnet-serverlist-urls.cfg", IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE File = pStorage->OpenFile("ddnet-serverlist-urls.cfg", IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
if(File)
{
CLineReader Lines;

View file

@ -247,7 +247,7 @@ bool CUpdater::ReplaceServer()
void CUpdater::ParseUpdate()
{
char aPath[IO_MAX_PATH_LENGTH];
IOHANDLE File = m_pStorage->OpenFile(m_pStorage->GetBinaryPath("update/update.json", aPath, sizeof aPath), IOFLAG_READ, IStorage::TYPE_ABSOLUTE);
IOHANDLE File = m_pStorage->OpenFile(m_pStorage->GetBinaryPath("update/update.json", aPath, sizeof aPath), IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ABSOLUTE);
if(!File)
return;

View file

@ -3695,7 +3695,7 @@ void CServer::GetClientAddr(int ClientID, NETADDR *pAddr) const
const char *CServer::GetAnnouncementLine(char const *pFileName)
{
IOHANDLE File = m_pStorage->OpenFile(pFileName, IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE File = m_pStorage->OpenFile(pFileName, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
if(!File)
return 0;

View file

@ -591,7 +591,7 @@ void CConsole::ExecuteFile(const char *pFilename, int ClientID, bool LogFailure,
m_pFirstExec = &ThisFile;
// exec the file
IOHANDLE File = m_pStorage->OpenFile(pFilename, IOFLAG_READ, StorageType);
IOHANDLE File = m_pStorage->OpenFile(pFilename, IOFLAG_READ | IOFLAG_SKIP_BOM, StorageType);
char aBuf[128];
if(File)

View file

@ -149,7 +149,7 @@ public:
return -1;
// try to open file
IOHANDLE File = m_pStorage->OpenFile("masters.cfg", IOFLAG_READ, IStorage::TYPE_SAVE);
IOHANDLE File = m_pStorage->OpenFile("masters.cfg", IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_SAVE);
if(!File)
return -1;

View file

@ -92,7 +92,7 @@ public:
void LoadPaths(const char *pArgv0)
{
// check current directory
IOHANDLE File = io_open("storage.cfg", IOFLAG_READ);
IOHANDLE File = io_open("storage.cfg", IOFLAG_READ | IOFLAG_SKIP_BOM);
if(!File)
{
// check usable path in argv[0]
@ -105,7 +105,7 @@ public:
char aBuffer[IO_MAX_PATH_LENGTH];
str_copy(aBuffer, pArgv0, Pos + 1);
str_append(aBuffer, "/storage.cfg", sizeof(aBuffer));
File = io_open(aBuffer, IOFLAG_READ);
File = io_open(aBuffer, IOFLAG_READ | IOFLAG_SKIP_BOM);
}
if(Pos >= IO_MAX_PATH_LENGTH || !File)

View file

@ -16,7 +16,7 @@
void CCountryFlags::LoadCountryflagsIndexfile()
{
IOHANDLE File = Storage()->OpenFile("countryflags/index.txt", IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE File = Storage()->OpenFile("countryflags/index.txt", IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
if(!File)
{
Console()->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "countryflags", "couldn't open index file");

View file

@ -1441,7 +1441,7 @@ public:
void LoadLanguageIndexfile(IStorage *pStorage, IConsole *pConsole, sorted_array<CLanguage> *pLanguages)
{
IOHANDLE File = pStorage->OpenFile("languages/index.txt", IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE File = pStorage->OpenFile("languages/index.txt", IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
if(!File)
{
pConsole->Print(IConsole::OUTPUT_LEVEL_ADDINFO, "localization", "couldn't open index file");

View file

@ -47,7 +47,7 @@ void CAutoMapper::Load(const char *pTileName)
{
char aPath[256];
str_format(aPath, sizeof(aPath), "editor/%s.rules", pTileName);
IOHANDLE RulesFile = m_pEditor->Storage()->OpenFile(aPath, IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE RulesFile = m_pEditor->Storage()->OpenFile(aPath, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
if(!RulesFile)
return;

View file

@ -55,7 +55,7 @@ bool CLocalizationDatabase::Load(const char *pFilename, IStorage *pStorage, ICon
return true;
}
IOHANDLE IoHandle = pStorage->OpenFile(pFilename, IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE IoHandle = pStorage->OpenFile(pFilename, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
if(!IoHandle)
return false;

View file

@ -3209,7 +3209,7 @@ void CGameContext::OnInit(/*class IKernel *pKernel*/)
m_pController = new CGameControllerDDRace(this);
const char *pCensorFilename = "censorlist.txt";
IOHANDLE File = Storage()->OpenFile(pCensorFilename, IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE File = Storage()->OpenFile(pCensorFilename, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
if(!File)
{
dbg_msg("censorlist", "failed to open '%s'", pCensorFilename);
@ -3427,7 +3427,7 @@ void CGameContext::OnMapChange(char *pNewMapName, int MapNameSize)
str_format(aConfig, sizeof(aConfig), "maps/%s.cfg", g_Config.m_SvMap);
str_format(aTemp, sizeof(aTemp), "%s.%d.tmp", pNewMapName, pid());
IOHANDLE File = Storage()->OpenFile(aConfig, IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE File = Storage()->OpenFile(aConfig, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
if(!File)
{
// No map-specific config, just return.

View file

@ -170,7 +170,7 @@ CScore::CScore(CGameContext *pGameServer, CDbConnectionPool *pPool) :
secure_random_fill(aSeed, sizeof(aSeed));
m_Prng.Seed(aSeed);
IOHANDLE File = GameServer()->Storage()->OpenFile("wordlist.txt", IOFLAG_READ, IStorage::TYPE_ALL);
IOHANDLE File = GameServer()->Storage()->OpenFile("wordlist.txt", IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ALL);
if(File)
{
CLineReader LineReader;

63
src/test/io.cpp Normal file
View file

@ -0,0 +1,63 @@
#include "test.h"
#include <gtest/gtest.h>
#include <base/system.h>
void TestFileRead(const char *pWritten, bool SkipBom, const char *pRead)
{
CTestInfo Info;
char aBuf[512] = {0};
IOHANDLE File = io_open(Info.m_aFilename, IOFLAG_WRITE);
ASSERT_TRUE(File);
EXPECT_EQ(io_write(File, pWritten, str_length(pWritten)), str_length(pWritten));
EXPECT_FALSE(io_close(File));
File = io_open(Info.m_aFilename, IOFLAG_READ | (SkipBom ? IOFLAG_SKIP_BOM : 0));
ASSERT_TRUE(File);
EXPECT_EQ(io_read(File, aBuf, sizeof(aBuf)), str_length(pRead));
EXPECT_TRUE(mem_comp(aBuf, pRead, str_length(pRead)) == 0);
EXPECT_FALSE(io_close(File));
fs_remove(Info.m_aFilename);
}
TEST(Io, Read1)
{
TestFileRead("", false, "");
}
TEST(Io, Read2)
{
TestFileRead("abc", false, "abc");
}
TEST(Io, Read3)
{
TestFileRead("\xef\xbb\xbf", false, "\xef\xbb\xbf");
}
TEST(Io, Read4)
{
TestFileRead("\xef\xbb\xbfxyz", false, "\xef\xbb\xbfxyz");
}
TEST(Io, ReadBom1)
{
TestFileRead("", true, "");
}
TEST(Io, ReadBom2)
{
TestFileRead("abc", true, "abc");
}
TEST(Io, ReadBom3)
{
TestFileRead("\xef\xbb\xbf", true, "");
}
TEST(Io, ReadBom4)
{
TestFileRead("\xef\xbb\xbfxyz", true, "xyz");
}
TEST(Io, ReadBom5)
{
TestFileRead("\xef\xbb\xbf\xef\xbb\xbf", true, "\xef\xbb\xbf");
}
TEST(Io, ReadBom6)
{
TestFileRead("\xef\xbb\xbfxyz\xef\xbb\xbf", true, "xyz\xef\xbb\xbf");
}

View file

@ -7,7 +7,7 @@
void Process(IStorage *pStorage, const char *pMapName, const char *pConfigName)
{
IOHANDLE File = pStorage->OpenFile(pConfigName, IOFLAG_READ, IStorage::TYPE_ABSOLUTE);
IOHANDLE File = pStorage->OpenFile(pConfigName, IOFLAG_READ | IOFLAG_SKIP_BOM, IStorage::TYPE_ABSOLUTE);
array<char *> aLines;
char *pSettings = NULL;
if(!File)