Add functions for reading/writing strings from/to datafile

Simplify the usage of datafile reader and writer by adding utility functions to read and write zero-terminated strings.

Improve validation of string data read from datafiles. Truncated UTF-8 is now properly fixed when either reading truncated strings from the datafile or reading into a too small buffer.

Fix loading of external sounds in the editor. The wrong path variable was being used, so the sound files would not be loaded from correct folder.

Add tests for new datafile reader/writer functions.

src/engine/map.h

@@ -18,6 +18,7 @@ class IMap : public IInterface
  virtual int GetDataSize(int Index) const = 0;
  virtual void *GetData(int Index) = 0;
  virtual void *GetDataSwapped(int Index) = 0;
+ virtual bool GetDataString(int Index, char *pBuffer, size_t BufferSize) = 0;
  virtual void UnloadData(int Index) = 0;
  virtual int NumData() const = 0;
 

src/engine/shared/datafile.cpp

@@ -402,6 +402,21 @@ void *CDataFileReader::GetDataSwapped(int Index)
  return GetDataImpl(Index, true);
 }
 
+bool CDataFileReader::GetDataString(int Index, char *pBuffer, size_t BufferSize)
+{
+ const int DataSize = GetDataSize(Index);
+ const char *pData = static_cast<char *>(GetData(Index));
+ if(!DataSize || !pData)
+ {
+ pBuffer[0] = '\0';
+ UnloadData(Index);
+ return false;
+ }
+ str_copy(pBuffer, pData, minimum<size_t>(DataSize + 1, BufferSize));
+ UnloadData(Index);
+ return true;
+}
+
 void CDataFileReader::ReplaceData(int Index, char *pData, size_t Size)
 {
  dbg_assert(Index >= 0 && Index < m_pDataFile->m_Header.m_NumRawData, "Index invalid");
@@ -747,6 +762,13 @@ int CDataFileWriter::AddDataSwapped(int Size, const void *pData)
 #endif
 }
 
+int CDataFileWriter::AddDataString(const char *pStr)
+{
+ if(pStr[0] == '\0')
+ return -1;
+ return AddData(str_length(pStr) + 1, pStr);
+}
+
 void CDataFileWriter::Finish()
 {
  dbg_assert((bool)m_File, "file not open");

src/engine/shared/datafile.h

@@ -38,6 +38,7 @@ class CDataFileReader
  int GetDataSize(int Index) const;
  void *GetData(int Index);
  void *GetDataSwapped(int Index); // makes sure that the data is 32bit LE ints when saved
+ bool GetDataString(int Index, char *pBuffer, size_t BufferSize);
  void ReplaceData(int Index, char *pData, size_t Size); // memory for data must have been allocated with malloc
  void UnloadData(int Index);
  int NumData() const;
@@ -129,6 +130,7 @@ class CDataFileWriter
  bool Open(class IStorage *pStorage, const char *pFilename, int StorageType = IStorage::TYPE_SAVE);
  int AddData(int Size, const void *pData, int CompressionLevel = Z_DEFAULT_COMPRESSION);
  int AddDataSwapped(int Size, const void *pData);
+ int AddDataString(const char *pStr);
  int AddItem(int Type, int ID, int Size, const void *pData);
  void Finish();
 };

src/engine/shared/map.cpp

@@ -23,6 +23,11 @@ void *CMap::GetDataSwapped(int Index)
  return m_DataFile.GetDataSwapped(Index);
 }
 
+bool CMap::GetDataString(int Index, char *pBuffer, size_t BufferSize)
+{
+ return m_DataFile.GetDataString(Index, pBuffer, BufferSize);
+}
+
 void CMap::UnloadData(int Index)
 {
  m_DataFile.UnloadData(Index);

src/engine/shared/map.h

@@ -20,6 +20,7 @@ class CMap : public IEngineMap
  int GetDataSize(int Index) const override;
  void *GetData(int Index) override;
  void *GetDataSwapped(int Index) override;
+ bool GetDataString(int Index, char *pBuffer, size_t BufferSize) override;
  void UnloadData(int Index) override;
  int NumData() const override;
 

src/game/client/components/mapimages.cpp

@@ -99,35 +99,31 @@ void CMapImages::OnMapLoadImpl(class CLayers *pLayers, IMap *pMap)
  }
  }
 
- int TextureLoadFlag = Graphics()->HasTextureArrays() ? IGraphics::TEXLOAD_TO_2D_ARRAY_TEXTURE : IGraphics::TEXLOAD_TO_3D_TEXTURE;
+ const int TextureLoadFlag = Graphics()->HasTextureArrays() ? IGraphics::TEXLOAD_TO_2D_ARRAY_TEXTURE : IGraphics::TEXLOAD_TO_3D_TEXTURE;
 
  // load new textures
  for(int i = 0; i < m_Count; i++)
  {
- int LoadFlag = (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 1) != 0) ? TextureLoadFlag : 0) | (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 2) != 0) ? 0 : (Graphics()->IsTileBufferingEnabled() ? IGraphics::TEXLOAD_NO_2D_TEXTURE : 0));
+ const int LoadFlag = (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 1) != 0) ? TextureLoadFlag : 0) | (((m_aTextureUsedByTileOrQuadLayerFlag[i] & 2) != 0) ? 0 : (Graphics()->IsTileBufferingEnabled() ? IGraphics::TEXLOAD_NO_2D_TEXTURE : 0));
  const CMapItemImage_v2 *pImg = (CMapItemImage_v2 *)pMap->GetItem(Start + i);
  const int Format = pImg->m_Version < CMapItemImage_v2::CURRENT_VERSION ? CImageInfo::FORMAT_RGBA : pImg->m_Format;
+
+ char aName[IO_MAX_PATH_LENGTH];
+ if(!pMap->GetDataString(pImg->m_ImageName, aName, sizeof(aName)))
+ continue;
+
  if(pImg->m_External)
  {
  char aPath[IO_MAX_PATH_LENGTH];
- char *pName = (char *)pMap->GetData(pImg->m_ImageName);
- str_format(aPath, sizeof(aPath), "mapres/%s.png", pName);
+ str_format(aPath, sizeof(aPath), "mapres/%s.png", aName);
  m_aTextures[i] = Graphics()->LoadTexture(aPath, IStorage::TYPE_ALL, CImageInfo::FORMAT_AUTO, LoadFlag);
- pMap->UnloadData(pImg->m_ImageName);
  }
- else if(Format != CImageInfo::FORMAT_RGBA)
- {
- m_aTextures[i] = Graphics()->InvalidTexture();
- pMap->UnloadData(pImg->m_ImageName);
- }
- else
+ else if(Format == CImageInfo::FORMAT_RGBA)
  {
  void *pData = pMap->GetData(pImg->m_ImageData);
- char *pName = (char *)pMap->GetData(pImg->m_ImageName);
- char aTexName[128];
- str_format(aTexName, sizeof(aTexName), "%s %s", "embedded:", pName);
+ char aTexName[IO_MAX_PATH_LENGTH];
+ str_format(aTexName, sizeof(aTexName), "embedded: %s", aName);
  m_aTextures[i] = Graphics()->LoadTextureRaw(pImg->m_Width, pImg->m_Height, Format, pData, CImageInfo::FORMAT_RGBA, LoadFlag, aTexName);
- pMap->UnloadData(pImg->m_ImageName);
  pMap->UnloadData(pImg->m_ImageData);
  }
  }

src/game/client/components/mapsounds.cpp

@@ -30,14 +30,14 @@ void CMapSounds::OnMapLoad()
  // load new samples
  for(int i = 0; i < m_Count; i++)
  {
- m_aSounds[i] = 0;
-
  CMapItemSound *pSound = (CMapItemSound *)pMap->GetItem(Start + i);
  if(pSound->m_External)
  {
+ char aName[IO_MAX_PATH_LENGTH];
+ if(!pMap->GetDataString(pSound->m_SoundName, aName, sizeof(aName)))
+ continue;
  char aBuf[IO_MAX_PATH_LENGTH];
- char *pName = (char *)pMap->GetData(pSound->m_SoundName);
- str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pName);
+ str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", aName);
  m_aSounds[i] = Sound()->LoadOpus(aBuf);
  }
  else

src/game/editor/io.cpp

@@ -66,23 +66,10 @@ bool CEditorMap::Save(const char *pFileName)
  {
  CMapItemInfoSettings Item;
  Item.m_Version = 1;
-
- if(m_MapInfo.m_aAuthor[0])
- Item.m_Author = Writer.AddData(str_length(m_MapInfo.m_aAuthor) + 1, m_MapInfo.m_aAuthor);
- else
- Item.m_Author = -1;
- if(m_MapInfo.m_aVersion[0])
- Item.m_MapVersion = Writer.AddData(str_length(m_MapInfo.m_aVersion) + 1, m_MapInfo.m_aVersion);
- else
- Item.m_MapVersion = -1;
- if(m_MapInfo.m_aCredits[0])
- Item.m_Credits = Writer.AddData(str_length(m_MapInfo.m_aCredits) + 1, m_MapInfo.m_aCredits);
- else
- Item.m_Credits = -1;
- if(m_MapInfo.m_aLicense[0])
- Item.m_License = Writer.AddData(str_length(m_MapInfo.m_aLicense) + 1, m_MapInfo.m_aLicense);
- else
- Item.m_License = -1;
+ Item.m_Author = Writer.AddDataString(m_MapInfo.m_aAuthor);
+ Item.m_MapVersion = Writer.AddDataString(m_MapInfo.m_aVersion);
+ Item.m_Credits = Writer.AddDataString(m_MapInfo.m_aCredits);
+ Item.m_License = Writer.AddDataString(m_MapInfo.m_aLicense);
 
  Item.m_Settings = -1;
  if(!m_vSettings.empty())
@@ -123,7 +110,7 @@ bool CEditorMap::Save(const char *pFileName)
  Item.m_Width = pImg->m_Width;
  Item.m_Height = pImg->m_Height;
  Item.m_External = pImg->m_External;
- Item.m_ImageName = Writer.AddData(str_length(pImg->m_aName) + 1, pImg->m_aName);
+ Item.m_ImageName = Writer.AddDataString(pImg->m_aName);
  if(pImg->m_External)
  {
  Item.m_ImageData = -1;
@@ -162,7 +149,7 @@ bool CEditorMap::Save(const char *pFileName)
  Item.m_Version = 1;
 
  Item.m_External = 0;
- Item.m_SoundName = Writer.AddData(str_length(pSound->m_aName) + 1, pSound->m_aName);
+ Item.m_SoundName = Writer.AddDataString(pSound->m_aName);
  Item.m_SoundData = Writer.AddData(pSound->m_DataSize, pSound->m_pData);
  Item.m_SoundDataSize = pSound->m_DataSize;
 
@@ -478,14 +465,10 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  if(!pItem || ItemID != 0)
  continue;
 
- if(pItem->m_Author > -1)
- str_copy(m_MapInfo.m_aAuthor, (char *)DataFile.GetData(pItem->m_Author));
- if(pItem->m_MapVersion > -1)
- str_copy(m_MapInfo.m_aVersion, (char *)DataFile.GetData(pItem->m_MapVersion));
- if(pItem->m_Credits > -1)
- str_copy(m_MapInfo.m_aCredits, (char *)DataFile.GetData(pItem->m_Credits));
- if(pItem->m_License > -1)
- str_copy(m_MapInfo.m_aLicense, (char *)DataFile.GetData(pItem->m_License));
+ DataFile.GetDataString(pItem->m_Author, m_MapInfo.m_aAuthor, sizeof(m_MapInfo.m_aAuthor));
+ DataFile.GetDataString(pItem->m_MapVersion, m_MapInfo.m_aVersion, sizeof(m_MapInfo.m_aVersion));
+ DataFile.GetDataString(pItem->m_Credits, m_MapInfo.m_aCredits, sizeof(m_MapInfo.m_aCredits));
+ DataFile.GetDataString(pItem->m_License, m_MapInfo.m_aLicense, sizeof(m_MapInfo.m_aLicense));
 
  if(pItem->m_Version != 1 || ItemSize < (int)sizeof(CMapItemInfoSettings))
  break;
@@ -512,17 +495,18 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  for(int i = 0; i < Num; i++)
  {
  CMapItemImage_v2 *pItem = (CMapItemImage_v2 *)DataFile.GetItem(Start + i);
- char *pName = (char *)DataFile.GetData(pItem->m_ImageName);
 
  // copy base info
  std::shared_ptr<CEditorImage> pImg = std::make_shared<CEditorImage>(m_pEditor);
  pImg->m_External = pItem->m_External;
+ if(!DataFile.GetDataString(pItem->m_ImageName, pImg->m_aName, sizeof(pImg->m_aName)))
+ continue;
 
  const int Format = pItem->m_Version < CMapItemImage_v2::CURRENT_VERSION ? CImageInfo::FORMAT_RGBA : pItem->m_Format;
  if(pImg->m_External || (Format != CImageInfo::FORMAT_RGB && Format != CImageInfo::FORMAT_RGBA))
  {
  char aBuf[IO_MAX_PATH_LENGTH];
- str_format(aBuf, sizeof(aBuf), "mapres/%s.png", pName);
+ str_format(aBuf, sizeof(aBuf), "mapres/%s.png", pImg->m_aName);
 
  // load external
  CEditorImage ImgInfo(m_pEditor);
@@ -554,10 +538,6 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  pImg->m_Texture = m_pEditor->Graphics()->LoadTextureRaw(pImg->m_Width, pImg->m_Height, pImg->m_Format, pImg->m_pData, CImageInfo::FORMAT_AUTO, TextureLoadFlag);
  }
 
- // copy image name
- if(pName)
- str_copy(pImg->m_aName, pName);
-
  // load auto mapper file
  pImg->m_AutoMapper.Load(pImg->m_aName);
 
@@ -576,18 +556,19 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  for(int i = 0; i < Num; i++)
  {
  CMapItemSound *pItem = (CMapItemSound *)DataFile.GetItem(Start + i);
- char *pName = (char *)DataFile.GetData(pItem->m_SoundName);
 
  // copy base info
  std::shared_ptr<CEditorSound> pSound = std::make_shared<CEditorSound>(m_pEditor);
+ if(!DataFile.GetDataString(pItem->m_SoundName, pSound->m_aName, sizeof(pSound->m_aName)))
+ continue;
 
  if(pItem->m_External)
  {
  char aBuf[IO_MAX_PATH_LENGTH];
- str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pName);
+ str_format(aBuf, sizeof(aBuf), "mapres/%s.opus", pSound->m_aName);
 
  // load external
- if(m_pEditor->Storage()->ReadFile(pName, IStorage::TYPE_ALL, &pSound->m_pData, &pSound->m_DataSize))
+ if(m_pEditor->Storage()->ReadFile(aBuf, IStorage::TYPE_ALL, &pSound->m_pData, &pSound->m_DataSize))
  {
  pSound->m_SoundID = m_pEditor->Sound()->LoadOpusFromMem(pSound->m_pData, pSound->m_DataSize, true);
  }
@@ -603,10 +584,6 @@ bool CEditorMap::Load(const char *pFileName, int StorageType, const std::functio
  pSound->m_SoundID = m_pEditor->Sound()->LoadOpusFromMem(pSound->m_pData, pSound->m_DataSize, true);
  }
 
- // copy image name
- if(pName)
- str_copy(pSound->m_aName, pName);
-
  m_vpSounds.push_back(pSound);
 
  // unload image

src/test/datafile.cpp

@@ -58,3 +58,51 @@ TEST(Datafile, ExtendedType)
  pStorage->RemoveFile(Info.m_aFilename, IStorage::TYPE_SAVE);
  }
 }
+
+TEST(Datafile, StringData)
+{
+ auto pStorage = std::unique_ptr<IStorage>(CreateLocalStorage());
+ CTestInfo Info;
+
+ {
+ CDataFileWriter Writer;
+ Writer.Open(pStorage.get(), Info.m_aFilename);
+
+ EXPECT_EQ(Writer.AddDataString(""), -1); // Empty string is not added
+ EXPECT_EQ(Writer.AddDataString("Abc"), 0);
+ EXPECT_EQ(Writer.AddDataString("DDNet最好了"), 1);
+ EXPECT_EQ(Writer.AddDataString("aβい🐘"), 2);
+ EXPECT_EQ(Writer.AddData(3, "Abc"), 3); // Not zero-terminated
+ EXPECT_EQ(Writer.AddData(7, "foo\0bar"), 4); // Early zero-terminator
+ EXPECT_EQ(Writer.AddData(5, "xyz\xff\0"), 5); // Truncated UTF-8
+ EXPECT_EQ(Writer.AddData(4, "XYZ\xff"), 6); // Truncated UTF-8 and not zero-terminated
+
+ Writer.Finish();
+ }
+
+ {
+ CDataFileReader Reader;
+ Reader.Open(pStorage.get(), Info.m_aFilename, IStorage::TYPE_ALL);
+
+ char aBuf[256];
+ EXPECT_TRUE(Reader.GetDataString(0, aBuf, sizeof(aBuf)));
+ EXPECT_STREQ(aBuf, "Abc");
+ EXPECT_TRUE(Reader.GetDataString(1, aBuf, sizeof(aBuf)));
+ EXPECT_STREQ(aBuf, "DDNet最好了");
+ EXPECT_TRUE(Reader.GetDataString(2, aBuf, sizeof(aBuf)));
+ EXPECT_STREQ(aBuf, "aβい🐘");
+ EXPECT_TRUE(Reader.GetDataString(3, aBuf, sizeof(aBuf)));
+ EXPECT_STREQ(aBuf, "Abc");
+ EXPECT_TRUE(Reader.GetDataString(4, aBuf, sizeof(aBuf)));
+ EXPECT_STREQ(aBuf, "foo");
+ EXPECT_TRUE(Reader.GetDataString(5, aBuf, sizeof(aBuf)));
+ EXPECT_STREQ(aBuf, "xyz");
+ EXPECT_TRUE(Reader.GetDataString(6, aBuf, sizeof(aBuf)));
+ EXPECT_STREQ(aBuf, "XYZ");
+ }
+
+ if(!HasFailure())
+ {
+ pStorage->RemoveFile(Info.m_aFilename, IStorage::TYPE_SAVE);
+ }
+}

src/tools/map_convert_07.cpp

@@ -91,8 +91,9 @@ bool CheckImageDimensions(void *pLayerItem, int LayerType, const char *pFilename
 
  char aTileLayerName[12];
  IntsToStr(pTMap->m_aName, sizeof(pTMap->m_aName) / sizeof(int), aTileLayerName);
- char *pName = (char *)g_DataReader.GetData(pImgItem->m_ImageName);
- dbg_msg("map_convert_07", "%s: Tile layer \"%s\" uses image \"%s\" with width %d, height %d, which is not divisible by 16. This is not supported in Teeworlds 0.7. Please scale the image and replace it manually.", pFilename, aTileLayerName, pName, pImgItem->m_Width, pImgItem->m_Height);
+ char aName[IO_MAX_PATH_LENGTH];
+ g_DataReader.GetDataString(pImgItem->m_ImageName, aName, sizeof(aName));
+ dbg_msg("map_convert_07", "%s: Tile layer \"%s\" uses image \"%s\" with width %d, height %d, which is not divisible by 16. This is not supported in Teeworlds 0.7. Please scale the image and replace it manually.", pFilename, aTileLayerName, aName, pImgItem->m_Width, pImgItem->m_Height);
  return false;
 }
 
@@ -101,12 +102,13 @@ void *ReplaceImageItem(CMapItemImage *pImgItem, CMapItemImage *pNewImgItem)
  if(!pImgItem->m_External)
  return pImgItem;
 
- char *pName = (char *)g_DataReader.GetData(pImgItem->m_ImageName);
- dbg_msg("map_convert_07", "embedding image '%s'", pName);
+ char aName[IO_MAX_PATH_LENGTH];
+ g_DataReader.GetDataString(pImgItem->m_ImageName, aName, sizeof(aName));
+ dbg_msg("map_convert_07", "embedding image '%s'", aName);
 
  CImageInfo ImgInfo;
  char aStr[IO_MAX_PATH_LENGTH];
- str_format(aStr, sizeof(aStr), "data/mapres/%s.png", pName);
+ str_format(aStr, sizeof(aStr), "data/mapres/%s.png", aName);
  if(!LoadPNG(&ImgInfo, aStr))
  return pImgItem; // keep as external if we don't have a mapres to replace