5 files changed, 92 insertions, 99 deletions

diff --git a/src/WorldStorage/ScoreboardSerializer.cpp b/src/WorldStorage/ScoreboardSerializer.cpp
index da8236e0d..e30eecf67 100644
--- a/src/WorldStorage/ScoreboardSerializer.cpp
+++ b/src/WorldStorage/ScoreboardSerializer.cpp
@@ -283,37 +283,37 @@ bool cScoreboardSerializer::LoadScoreboardFromNBT(const cParsedNBT & a_NBT)
 		bool AllowsFriendlyFire = true, CanSeeFriendlyInvisible = false;
 
 		int CurrLine = a_NBT.FindChildByName(Child, "Name");
-		if (CurrLine >= 0)
+		if ((CurrLine >= 0) && (a_NBT.GetType(CurrLine) == TAG_String))
 		{
-			Name = a_NBT.GetInt(CurrLine);
+			Name = a_NBT.GetString(CurrLine);
 		}
 
 		CurrLine = a_NBT.FindChildByName(Child, "DisplayName");
-		if (CurrLine >= 0)
+		if ((CurrLine >= 0) && (a_NBT.GetType(CurrLine) == TAG_String))
 		{
-			DisplayName = a_NBT.GetInt(CurrLine);
+			DisplayName = a_NBT.GetString(CurrLine);
 		}
 
 		CurrLine = a_NBT.FindChildByName(Child, "Prefix");
-		if (CurrLine >= 0)
+		if ((CurrLine >= 0) && (a_NBT.GetType(CurrLine) == TAG_String))
 		{
-			Prefix = a_NBT.GetInt(CurrLine);
+			Prefix = a_NBT.GetString(CurrLine);
 		}
 
 		CurrLine = a_NBT.FindChildByName(Child, "Suffix");
-		if (CurrLine >= 0)
+		if ((CurrLine >= 0) && (a_NBT.GetType(CurrLine) == TAG_String))
 		{
-			Suffix = a_NBT.GetInt(CurrLine);
+			Suffix = a_NBT.GetString(CurrLine);
 		}
 
 		CurrLine = a_NBT.FindChildByName(Child, "AllowFriendlyFire");
-		if (CurrLine >= 0)
+		if ((CurrLine >= 0) && (a_NBT.GetType(CurrLine) == TAG_Int))
 		{
 			AllowsFriendlyFire = (a_NBT.GetInt(CurrLine) != 0);
 		}
 
 		CurrLine = a_NBT.FindChildByName(Child, "SeeFriendlyInvisibles");
-		if (CurrLine >= 0)
+		if ((CurrLine >= 0) && (a_NBT.GetType(CurrLine) == TAG_Int))
 		{
 			CanSeeFriendlyInvisible = (a_NBT.GetInt(CurrLine) != 0);
 		}
diff --git a/src/WorldStorage/WSSAnvil.cpp b/src/WorldStorage/WSSAnvil.cpp
index e79cc291d..fe309ce4e 100644
--- a/src/WorldStorage/WSSAnvil.cpp
+++ b/src/WorldStorage/WSSAnvil.cpp
@@ -66,8 +66,17 @@ Since only the header is actually in the memory, this number can be high, but st
 */
 #define MAX_MCA_FILES 32
 
-/// The maximum size of an inflated chunk; raw chunk data is 192 KiB, allow 64 KiB more of entities
-#define CHUNK_INFLATE_MAX 256 KiB
+#define LOAD_FAILED(CHX, CHZ) \
+	{ \
+		const int RegionX = FAST_FLOOR_DIV(CHX, 32); \
+		const int RegionZ = FAST_FLOOR_DIV(CHZ, 32); \
+		LOGERROR("%s (%d): Loading chunk [%d, %d] from file r.%d.%d.mca failed. " \
+			"The server will now abort in order to avoid further data loss. " \
+			"Please add the reported file and this message to the issue report.", \
+			__FUNCTION__, __LINE__, CHX, CHZ, RegionX, RegionZ \
+		); \
+		*((volatile int *)0) = 0;  /* Crash intentionally */ \
+	}
 
 
 
@@ -248,29 +257,22 @@ cWSSAnvil::cMCAFile * cWSSAnvil::LoadMCAFile(const cChunkCoords & a_Chunk)
 
 bool cWSSAnvil::LoadChunkFromData(const cChunkCoords & a_Chunk, const AString & a_Data)
 {
-	// Decompress the data:
-	char Uncompressed[CHUNK_INFLATE_MAX];
-	z_stream strm;
-	strm.zalloc = (alloc_func)NULL;
-	strm.zfree = (free_func)NULL;
-	strm.opaque = NULL;
-	inflateInit(&strm);
-	strm.next_out  = (Bytef *)Uncompressed;
-	strm.avail_out = sizeof(Uncompressed);
-	strm.next_in   = (Bytef *)a_Data.data();
-	strm.avail_in  = (uInt)a_Data.size();
-	int res = inflate(&strm, Z_FINISH);
-	inflateEnd(&strm);
-	if (res != Z_STREAM_END)
+	// Uncompress the data:
+	AString Uncompressed;
+	int res = InflateString(a_Data.data(), a_Data.size(), Uncompressed);
+	if (res != Z_OK)
 	{
+		LOGWARNING("Uncompressing chunk [%d, %d] failed: %d", a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ, res);
+		LOAD_FAILED(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 	
 	// Parse the NBT data:
-	cParsedNBT NBT(Uncompressed, strm.total_out);
+	cParsedNBT NBT(Uncompressed.data(), Uncompressed.size());
 	if (!NBT.IsValid())
 	{
 		// NBT Parsing failed
+		LOAD_FAILED(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 
@@ -317,11 +319,19 @@ bool cWSSAnvil::LoadChunkFromNBT(const cChunkCoords & a_Chunk, const cParsedNBT
 	int Level = a_NBT.FindChildByName(0, "Level");
 	if (Level < 0)
 	{
+		LOAD_FAILED(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 	int Sections = a_NBT.FindChildByName(Level, "Sections");
-	if ((Sections < 0) || (a_NBT.GetType(Sections) != TAG_List) || (a_NBT.GetChildrenType(Sections) != TAG_Compound))
+	if ((Sections < 0) || (a_NBT.GetType(Sections) != TAG_List))
+	{
+		LOAD_FAILED(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
+		return false;
+	}
+	eTagType SectionsType = a_NBT.GetChildrenType(Sections);
+	if ((SectionsType != TAG_Compound) && (SectionsType != TAG_End))
 	{
+		LOAD_FAILED(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 	for (int Child = a_NBT.GetFirstChild(Sections); Child >= 0; Child = a_NBT.GetNextSibling(Child))
@@ -585,7 +595,7 @@ void cWSSAnvil::LoadBlockEntitiesFromNBT(cBlockEntityList & a_BlockEntities, con
 
 		// Get the BlockEntity's position
 		int x, y, z;
-		if (!GetBlockEntityNBTPos(a_NBT, Child, x, y, z))
+		if (!GetBlockEntityNBTPos(a_NBT, Child, x, y, z) || (y < 0) || (y >= cChunkDef::Height))
 		{
 			LOGWARNING("Bad block entity, missing the coords. Will be ignored.");
 			continue;
@@ -613,6 +623,8 @@ void cWSSAnvil::LoadBlockEntitiesFromNBT(cBlockEntityList & a_BlockEntities, con
 
 cBlockEntity * cWSSAnvil::LoadBlockEntityFromNBT(const cParsedNBT & a_NBT, int a_Tag, int a_BlockX, int a_BlockY, int a_BlockZ, BLOCKTYPE a_BlockType, NIBBLETYPE a_BlockMeta)
 {
+	ASSERT((a_BlockY >= 0) && (a_BlockY < cChunkDef::Height));
+
 	// Load the specific BlockEntity type:
 	switch (a_BlockType)
 	{
@@ -2811,30 +2823,42 @@ bool cWSSAnvil::cMCAFile::GetChunkData(const cChunkCoords & a_Chunk, AString & a
 	}
 	unsigned ChunkLocation = ntohl(m_Header[LocalX + 32 * LocalZ]);
 	unsigned ChunkOffset = ChunkLocation >> 8;
+	if (ChunkOffset < 2)
+	{
+		return false;
+	}
 	
 	m_File.Seek((int)ChunkOffset * 4096);
 	
 	int ChunkSize = 0;
 	if (m_File.Read(&ChunkSize, 4) != 4)
 	{
+		LOAD_FAILED(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 	ChunkSize = ntohl((u_long)ChunkSize);
 	char CompressionType = 0;
 	if (m_File.Read(&CompressionType, 1) != 1)
 	{
+		LOAD_FAILED(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 	if (CompressionType != 2)
 	{
 		// Chunk is in an unknown compression
+		LOAD_FAILED(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 	ChunkSize--;
 	
 	// HACK: This depends on the internal knowledge that AString's data() function returns the internal buffer directly
 	a_Data.assign(ChunkSize, '\0');
-	return (m_File.Read((void *)a_Data.data(), ChunkSize) == ChunkSize);
+	if (m_File.Read((void *)a_Data.data(), ChunkSize) == ChunkSize)
+	{
+		return true;
+	}
+	LOAD_FAILED(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
+	return false;
 }
 
 
@@ -2889,7 +2913,13 @@ bool cWSSAnvil::cMCAFile::SetChunkData(const cChunkCoords & a_Chunk, const AStri
 	
 	// Store the header:
 	ChunkSize = ((u_long)a_Data.size() + MCA_CHUNK_HEADER_LENGTH + 4095) / 4096;  // Round data size *up* to nearest 4KB sector, make it a sector number
-	ASSERT(ChunkSize < 256);
+	if (ChunkSize > 255)
+	{
+		LOGWARNING("Cannot save chunk [%d, %d], the data is too large (%u KiB, maximum is 1024 KiB). Remove some entities and retry.",
+			a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ, (unsigned)(ChunkSize * 4)
+		);
+		return false;
+	}
 	m_Header[LocalX + 32 * LocalZ] = htonl((ChunkSector << 8) | ChunkSize);
 	if (m_File.Seek(0) < 0)
 	{
diff --git a/src/WorldStorage/WSSCompact.cpp b/src/WorldStorage/WSSCompact.cpp
index 58f9e3cab..6760186b2 100644
--- a/src/WorldStorage/WSSCompact.cpp
+++ b/src/WorldStorage/WSSCompact.cpp
@@ -980,7 +980,7 @@ bool cWSSCompact::cPAKFile::SaveChunkToData(const cChunkCoords & a_Chunk, cWorld
 	if (!a_World->GetChunkData(a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ, Serializer))
 	{
 		// Chunk not valid
-		LOG("cWSSCompact: Trying to save chunk [%d, %d, %d] that has no data, ignoring request.", a_Chunk.m_ChunkX, a_Chunk.m_ChunkY, a_Chunk.m_ChunkZ);
+		LOG("cWSSCompact: Trying to save chunk [%d, %d] that has no data, ignoring request.", a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 
@@ -999,7 +999,7 @@ bool cWSSCompact::cPAKFile::SaveChunkToData(const cChunkCoords & a_Chunk, cWorld
 	int errorcode = CompressString(Data.data(), Data.size(), CompressedData, m_CompressionFactor);
 	if (errorcode != Z_OK)
 	{
-		LOGERROR("Error %i compressing data for chunk [%d, %d, %d]", errorcode, a_Chunk.m_ChunkX, a_Chunk.m_ChunkY, a_Chunk.m_ChunkZ);
+		LOGERROR("Error %i compressing data for chunk [%d, %d]", errorcode, a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 	
@@ -1010,7 +1010,7 @@ bool cWSSCompact::cPAKFile::SaveChunkToData(const cChunkCoords & a_Chunk, cWorld
 	sChunkHeader * Header = new sChunkHeader;
 	if (Header == NULL)
 	{
-		LOGWARNING("Cannot create a new chunk header to save chunk [%d, %d, %d]", a_Chunk.m_ChunkX, a_Chunk.m_ChunkY, a_Chunk.m_ChunkZ);
+		LOGWARNING("Cannot create a new chunk header to save chunk [%d, %d]", a_Chunk.m_ChunkX, a_Chunk.m_ChunkZ);
 		return false;
 	}
 	Header->m_CompressedSize = (int)CompressedData.size();
diff --git a/src/WorldStorage/WorldStorage.cpp b/src/WorldStorage/WorldStorage.cpp
index 707e8f929..179cf9393 100644
--- a/src/WorldStorage/WorldStorage.cpp
+++ b/src/WorldStorage/WorldStorage.cpp
@@ -141,9 +141,11 @@ size_t cWorldStorage::GetSaveQueueLength(void)
 
 
 
-void cWorldStorage::QueueLoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ, bool a_Generate)
+void cWorldStorage::QueueLoadChunk(int a_ChunkX, int a_ChunkZ)
 {
-	m_LoadQueue.EnqueueItem(sChunkLoad(a_ChunkX, a_ChunkY, a_ChunkZ, a_Generate));
+	ASSERT(m_World->IsChunkQueued(a_ChunkX, a_ChunkZ));
+
+	m_LoadQueue.EnqueueItem(cChunkCoords(a_ChunkX, a_ChunkZ));
 	m_Event.Set();
 }
 
@@ -151,9 +153,11 @@ void cWorldStorage::QueueLoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ, boo
 
 
 
-void cWorldStorage::QueueSaveChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ)
+void cWorldStorage::QueueSaveChunk(int a_ChunkX, int a_ChunkZ)
 {
-	m_SaveQueue.EnqueueItemIfNotPresent(cChunkCoords(a_ChunkX, a_ChunkY, a_ChunkZ));
+	ASSERT(m_World->IsChunkValid(a_ChunkX, a_ChunkZ));
+
+	m_SaveQueue.EnqueueItemIfNotPresent(cChunkCoords(a_ChunkX, a_ChunkZ));
 	m_Event.Set();
 }
 
@@ -161,9 +165,9 @@ void cWorldStorage::QueueSaveChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ)
 
 
 
-void cWorldStorage::UnqueueLoad(int a_ChunkX, int a_ChunkY, int a_ChunkZ)
+void cWorldStorage::UnqueueLoad(int a_ChunkX, int a_ChunkZ)
 {
-	m_LoadQueue.Remove(sChunkLoad(a_ChunkX, a_ChunkY, a_ChunkZ, true));
+	m_LoadQueue.Remove(cChunkCoords(a_ChunkX, a_ChunkZ));
 }
 
 
@@ -242,22 +246,14 @@ void cWorldStorage::Execute(void)
 
 bool cWorldStorage::LoadOneChunk(void)
 {
-	sChunkLoad ToLoad(0, 0, 0, false);
+	cChunkCoords ToLoad(0, 0);
 	bool ShouldLoad = m_LoadQueue.TryDequeueItem(ToLoad);
-	if (ShouldLoad && !LoadChunk(ToLoad.m_ChunkX, ToLoad.m_ChunkY, ToLoad.m_ChunkZ))
+
+	if (ShouldLoad)
 	{
-		if (ToLoad.m_Generate)
-		{
-			// The chunk couldn't be loaded, generate it:
-			m_World->GetGenerator().QueueGenerateChunk(ToLoad.m_ChunkX, ToLoad.m_ChunkY, ToLoad.m_ChunkZ);
-		}
-		else
-		{
-			// TODO: Notify the world that the load has failed:
-			// m_World->ChunkLoadFailed(ToLoad.m_ChunkX, ToLoad.m_ChunkY, ToLoad.m_ChunkZ);
-		}
+		return LoadChunk(ToLoad.m_ChunkX, ToLoad.m_ChunkZ);
 	}
-	return ShouldLoad;
+	return false;
 }
 
 
@@ -266,7 +262,7 @@ bool cWorldStorage::LoadOneChunk(void)
 
 bool cWorldStorage::SaveOneChunk(void)
 {
-	cChunkCoords ToSave(0, 0, 0);
+	cChunkCoords ToSave(0, 0);
 	bool ShouldSave = m_SaveQueue.TryDequeueItem(ToSave);
 	if (ShouldSave && m_World->IsChunkValid(ToSave.m_ChunkX, ToSave.m_ChunkZ))
 	{
@@ -283,15 +279,11 @@ bool cWorldStorage::SaveOneChunk(void)
 
 
 
-bool cWorldStorage::LoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ)
+bool cWorldStorage::LoadChunk(int a_ChunkX, int a_ChunkZ)
 {
-	if (m_World->IsChunkValid(a_ChunkX, a_ChunkZ))
-	{
-		// Already loaded (can happen, since the queue is async)
-		return true;
-	}
+	ASSERT(m_World->IsChunkQueued(a_ChunkX, a_ChunkZ));
 	
-	cChunkCoords Coords(a_ChunkX, a_ChunkY, a_ChunkZ);
+	cChunkCoords Coords(a_ChunkX, a_ChunkZ);
 
 	// First try the schema that is used for saving
 	if (m_SaveSchema->LoadChunk(Coords))
@@ -309,7 +301,7 @@ bool cWorldStorage::LoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ)
 	}
 	
 	// Notify the chunk owner that the chunk failed to load (sets cChunk::m_HasLoadFailed to true):
-	m_World->ChunkLoadFailed(a_ChunkX, a_ChunkY, a_ChunkZ);
+	m_World->ChunkLoadFailed(a_ChunkX, a_ChunkZ);
 	
 	return false;
 }
diff --git a/src/WorldStorage/WorldStorage.h b/src/WorldStorage/WorldStorage.h
index dd07ecb64..fc7e9f84c 100644
--- a/src/WorldStorage/WorldStorage.h
+++ b/src/WorldStorage/WorldStorage.h
@@ -64,13 +64,10 @@ public:
 	cWorldStorage(void);
 	~cWorldStorage();
 	
-	void QueueLoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ, bool a_Generate);  // Queues the chunk for loading; if not loaded, the chunk will be generated if a_Generate is true
-	void QueueSaveChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ);
+	void QueueLoadChunk(int a_ChunkX, int a_ChunkZ);
+	void QueueSaveChunk(int a_ChunkX, int a_ChunkZ);
 	
-	/// Loads the chunk specified; returns true on success, false on failure
-	bool LoadChunk(int a_ChunkX, int a_ChunkY, int a_ChunkZ);
-
-	void UnqueueLoad(int a_ChunkX, int a_ChunkY, int a_ChunkZ);
+	void UnqueueLoad(int a_ChunkX, int a_ChunkZ);
 	void UnqueueSave(const cChunkCoords & a_Chunk);
 	
 	bool Start(cWorld * a_World, const AString & a_StorageSchemaName, int a_StorageCompressionFactor);  // Hide the cIsThread's Start() method, we need to provide args
@@ -84,40 +81,10 @@ public:
 	
 protected:
 
-	struct sChunkLoad
-	{
-		int m_ChunkX;
-		int m_ChunkY;
-		int m_ChunkZ;
-		bool m_Generate;  // If true, the chunk will be generated if it cannot be loaded
-		
-		sChunkLoad(int a_ChunkX, int a_ChunkY, int a_ChunkZ, bool a_Generate) : m_ChunkX(a_ChunkX), m_ChunkY(a_ChunkY), m_ChunkZ(a_ChunkZ), m_Generate(a_Generate) {}
-
-		bool operator ==(const sChunkLoad other) const
-		{
-			return (
-				(this->m_ChunkX == other.m_ChunkX) &&
-				(this->m_ChunkY == other.m_ChunkY) &&
-				(this->m_ChunkZ == other.m_ChunkZ)
-			);
-		}
-	} ;
-
-	struct FuncTable
-	{
-		static void Delete(sChunkLoad) {}
-		static void Combine(sChunkLoad & a_orig, const sChunkLoad a_new)
-		{
-			a_orig.m_Generate |= a_new.m_Generate;
-		}
-	};
-
-	typedef cQueue<sChunkLoad, FuncTable> sChunkLoadQueue;
-	
 	cWorld * m_World;
 	AString  m_StorageSchemaName;
 
-	sChunkLoadQueue  m_LoadQueue;
+	cChunkCoordsQueue  m_LoadQueue;
 	cChunkCoordsQueue m_SaveQueue;
 	
 	/// All the storage schemas (all used for loading)
@@ -125,7 +92,11 @@ protected:
 	
 	/// The one storage schema used for saving
 	cWSSchema *   m_SaveSchema;
+
 	
+	/// Loads the chunk specified; returns true on success, false on failure
+	bool LoadChunk(int a_ChunkX, int a_ChunkZ);
+
 	void InitSchemas(int a_StorageCompressionFactor);
 	
 	virtual void Execute(void) override;