cParsedNBT: Improved error reporting (#3876)

* cParsedNBT: Improved error reporting

* Fix typos

src/Globals.h

@@ -46,10 +46,11 @@
 	#define SIZE_T_FMT_HEX "%Ix"
 
 	#define NORETURN __declspec(noreturn)
-	#if (_MSC_VER < 1910)
+	#if (_MSC_VER < 1900)  // noexcept support was added in VS 2015
-		// MSVC 2013 (and possibly 2015?) have no idea about "noexcept(false)"
+		#define NOEXCEPT  throw()
 		#define CAN_THROW throw(...)
 	#else
+		#define NOEXCEPT  noexcept
 		#define CAN_THROW noexcept(false)
 	#endif
 
@@ -108,6 +109,7 @@
 	#endif
 
 	#define NORETURN __attribute((__noreturn__))
+	#define NOEXCEPT  noexcept
 	#define CAN_THROW noexcept(false)
 
 #else

src/Protocol/Protocol_1_8.cpp

@@ -2851,7 +2851,9 @@ void cProtocol_1_8_0::ParseItemMetadata(cItem & a_Item, const AString & a_Metada
 	{
 		AString HexDump;
 		CreateHexDump(HexDump, a_Metadata.data(), std::max<size_t>(a_Metadata.size(), 1024), 16);
-		LOGWARNING("Cannot parse NBT item metadata: (" SIZE_T_FMT " bytes)\n%s", a_Metadata.size(), HexDump.c_str());
+		LOGWARNING("Cannot parse NBT item metadata: %s at (" SIZE_T_FMT " / " SIZE_T_FMT " bytes)\n%s",
+			NBT.GetErrorCode().message().c_str(), NBT.GetErrorPos(), a_Metadata.size(), HexDump.c_str()
+		);
 		return;
 	}
 

src/Protocol/Protocol_1_9.cpp

@@ -2949,7 +2949,9 @@ void cProtocol_1_9_0::ParseItemMetadata(cItem & a_Item, const AString & a_Metada
 	{
 		AString HexDump;
 		CreateHexDump(HexDump, a_Metadata.data(), std::max<size_t>(a_Metadata.size(), 1024), 16);
-		LOGWARNING("Cannot parse NBT item metadata: (" SIZE_T_FMT " bytes)\n%s", a_Metadata.size(), HexDump.c_str());
+		LOGWARNING("Cannot parse NBT item metadata: %s at (" SIZE_T_FMT " / " SIZE_T_FMT " bytes)\n%s",
+			NBT.GetErrorCode().message().c_str(), NBT.GetErrorPos(), a_Metadata.size(), HexDump.c_str()
+		);
 		return;
 	}
 

src/WorldStorage/FastNBT.cpp

@@ -25,20 +25,100 @@ static const int MAX_LIST_ITEMS = 10000;
 
 #ifdef _MSC_VER
 	// Dodge a C4127 (conditional expression is constant) for this specific macro usage
-	#define RETURN_FALSE_IF_FALSE(X) do { if (!X) return false; } while ((false, false))
+	#define PROPAGATE_ERROR(X) do { auto Err = (X); if (Err != eNBTParseError::npSuccess) return Err; } while ((false, false))
 #else
-	#define RETURN_FALSE_IF_FALSE(X) do { if (!X) return false; } while (false)
+	#define PROPAGATE_ERROR(X) do { auto Err = (X); if (Err != eNBTParseError::npSuccess) return Err; } while (false)
 #endif
 
 
 
+////////////////////////////////////////////////////////////////////////////////
+// cNBTParseErrorCategory:
+
+AString cNBTParseErrorCategory::message(int a_Condition) const
+{
+	switch (static_cast<eNBTParseError>(a_Condition))
+	{
+		case eNBTParseError::npSuccess:
+		{
+			return "Parsing succeded";
+		}
+		case eNBTParseError::npNeedBytes:
+		{
+			return "Expected more data";
+		}
+		case eNBTParseError::npNoTopLevelCompound:
+		{
+			return "No top level compound tag";
+		}
+		case eNBTParseError::npStringMissingLength:
+		{
+			return "Expected a string length but had insufficient data";
+		}
+		case eNBTParseError::npStringInvalidLength:
+		{
+			return "String length invalid";
+		}
+		case eNBTParseError::npCompoundImbalancedTag:
+		{
+			return "Compound tag was unmatched at end of file";
+		}
+		case eNBTParseError::npListMissingType:
+		{
+			return "Expected a list type but had insuffiecient data";
+		}
+		case eNBTParseError::npListMissingLength:
+		{
+			return "Expected a list length but had insufficient data";
+		}
+		case eNBTParseError::npListInvalidLength:
+		{
+			return "List length invalid";
+		}
+		case eNBTParseError::npSimpleMissing:
+		{
+			return "Expected a numeric type but had insufficient data";
+		}
+		case eNBTParseError::npArrayMissingLength:
+		{
+			return "Expected an array length but had insufficient data";
+		}
+		case eNBTParseError::npArrayInvalidLength:
+		{
+			return "Array length invalid";
+		}
+		case eNBTParseError::npUnknownTag:
+		{
+			return "Unknown tag";
+		}
+
+		#ifdef __clang__
+			#pragma clang diagnostic push
+			#pragma clang diagnostic ignored "-Wcovered-switch-default"
+		#endif
+
+		default:
+		{
+			return "<unrecognized error>";
+		}
+
+		#ifdef __clang__
+			#pragma clang diagnostic pop
+		#endif
+	}
+}
+
+
+
+
+
 ////////////////////////////////////////////////////////////////////////////////
 // cParsedNBT:
 
-#define NEEDBYTES(N) \
+#define NEEDBYTES(N, ERR) \
 	if (m_Length - m_Pos < static_cast<size_t>(N)) \
 	{ \
-		return false; \
+		return ERR; \
 	}
 
 
@@ -50,57 +130,55 @@ cParsedNBT::cParsedNBT(const char * a_Data, size_t a_Length) :
 	m_Length(a_Length),
 	m_Pos(0)
 {
-	m_IsValid = Parse();
+	m_Error = Parse();
 }
 
 
 
 
 
-bool cParsedNBT::Parse(void)
+eNBTParseError cParsedNBT::Parse(void)
 {
 	if (m_Length < 3)
 	{
 		// Data too short
-		return false;
+		return eNBTParseError::npNeedBytes;
 	}
 	if (m_Data[0] != TAG_Compound)
 	{
 		// The top-level tag must be a Compound
-		return false;
+		return eNBTParseError::npNoTopLevelCompound;
 	}
 
 	m_Tags.reserve(NBT_RESERVE_SIZE);
 
-	m_Tags.push_back(cFastNBTTag(TAG_Compound, -1));
+	m_Tags.emplace_back(TAG_Compound, -1);
 
 	m_Pos = 1;
 
-	RETURN_FALSE_IF_FALSE(ReadString(m_Tags.back().m_NameStart, m_Tags.back().m_NameLength));
+	PROPAGATE_ERROR(ReadString(m_Tags.back().m_NameStart, m_Tags.back().m_NameLength));
-	RETURN_FALSE_IF_FALSE(ReadCompound());
+	return ReadCompound();
-
-	return true;
 }
 
 
 
 
 
-bool cParsedNBT::ReadString(size_t & a_StringStart, size_t & a_StringLen)
+eNBTParseError cParsedNBT::ReadString(size_t & a_StringStart, size_t & a_StringLen)
 {
-	NEEDBYTES(2);
+	NEEDBYTES(2, eNBTParseError::npStringMissingLength);
 	a_StringStart = m_Pos + 2;
 	a_StringLen = static_cast<size_t>(GetBEShort(m_Data + m_Pos));
-	NEEDBYTES(2 + a_StringLen);
+	NEEDBYTES(2 + a_StringLen, eNBTParseError::npStringInvalidLength);
 	m_Pos += 2 + a_StringLen;
-	return true;
+	return eNBTParseError::npSuccess;
 }
 
 
 
 
 
-bool cParsedNBT::ReadCompound(void)
+eNBTParseError cParsedNBT::ReadCompound(void)
 {
 	ASSERT(m_Tags.size() > 0);
 
@@ -109,11 +187,11 @@ bool cParsedNBT::ReadCompound(void)
 	int PrevSibling = -1;
 	for (;;)
 	{
-		NEEDBYTES(1);
+		NEEDBYTES(1, eNBTParseError::npCompoundImbalancedTag);
 		const char TagTypeNum = m_Data[m_Pos];
 		if ((TagTypeNum < TAG_Min) || (TagTypeNum > TAG_Max))
 		{
-			return false;
+			return eNBTParseError::npUnknownTag;
 		}
 		eTagType TagType = static_cast<eTagType>(TagTypeNum);
 		m_Pos++;
@@ -121,7 +199,7 @@ bool cParsedNBT::ReadCompound(void)
 		{
 			break;
 		}
-		m_Tags.push_back(cFastNBTTag(TagType, static_cast<int>(ParentIdx), PrevSibling));
+		m_Tags.emplace_back(TagType, static_cast<int>(ParentIdx), PrevSibling);
 		if (PrevSibling >= 0)
 		{
 			m_Tags[static_cast<size_t>(PrevSibling)].m_NextSibling = static_cast<int>(m_Tags.size()) - 1;
@@ -131,28 +209,28 @@ bool cParsedNBT::ReadCompound(void)
 			m_Tags[ParentIdx].m_FirstChild = static_cast<int>(m_Tags.size()) - 1;
 		}
 		PrevSibling = static_cast<int>(m_Tags.size()) - 1;
-		RETURN_FALSE_IF_FALSE(ReadString(m_Tags.back().m_NameStart, m_Tags.back().m_NameLength));
+		PROPAGATE_ERROR(ReadString(m_Tags.back().m_NameStart, m_Tags.back().m_NameLength));
-		RETURN_FALSE_IF_FALSE(ReadTag());
+		PROPAGATE_ERROR(ReadTag());
 	}  // while (true)
 	m_Tags[ParentIdx].m_LastChild = PrevSibling;
-	return true;
+	return eNBTParseError::npSuccess;
 }
 
 
 
 
 
-bool cParsedNBT::ReadList(eTagType a_ChildrenType)
+eNBTParseError cParsedNBT::ReadList(eTagType a_ChildrenType)
 {
 	// Reads the latest tag as a list of items of type a_ChildrenType
 
 	// Read the count:
-	NEEDBYTES(4);
+	NEEDBYTES(4, eNBTParseError::npListMissingLength);
 	int Count = GetBEInt(m_Data + m_Pos);
 	m_Pos += 4;
 	if ((Count < 0) || (Count > MAX_LIST_ITEMS))
 	{
-		return false;
+		return eNBTParseError::npListInvalidLength;
 	}
 
 	// Read items:
@@ -161,7 +239,7 @@ bool cParsedNBT::ReadList(eTagType a_ChildrenType)
 	int PrevSibling = -1;
 	for (int i = 0; i < Count; i++)
 	{
-		m_Tags.push_back(cFastNBTTag(a_ChildrenType, static_cast<int>(ParentIdx), PrevSibling));
+		m_Tags.emplace_back(a_ChildrenType, static_cast<int>(ParentIdx), PrevSibling);
 		if (PrevSibling >= 0)
 		{
 			m_Tags[static_cast<size_t>(PrevSibling)].m_NextSibling = static_cast<int>(m_Tags.size()) - 1;
@@ -171,10 +249,10 @@ bool cParsedNBT::ReadList(eTagType a_ChildrenType)
 			m_Tags[ParentIdx].m_FirstChild = static_cast<int>(m_Tags.size()) - 1;
 		}
 		PrevSibling = static_cast<int>(m_Tags.size()) - 1;
-		RETURN_FALSE_IF_FALSE(ReadTag());
+		PROPAGATE_ERROR(ReadTag());
 	}  // for (i)
 	m_Tags[ParentIdx].m_LastChild = PrevSibling;
-	return true;
+	return eNBTParseError::npSuccess;
 }
 
 
@@ -184,14 +262,14 @@ bool cParsedNBT::ReadList(eTagType a_ChildrenType)
 #define CASE_SIMPLE_TAG(TAGTYPE, LEN) \
 	case TAG_##TAGTYPE: \
 	{ \
-		NEEDBYTES(LEN); \
+		NEEDBYTES(LEN, eNBTParseError::npSimpleMissing); \
 		Tag.m_DataStart = m_Pos; \
 		Tag.m_DataLength = LEN; \
 		m_Pos += LEN; \
-		return true; \
+		return eNBTParseError::npSuccess; \
 	}
 
-bool cParsedNBT::ReadTag(void)
+eNBTParseError cParsedNBT::ReadTag(void)
 {
 	cFastNBTTag & Tag = m_Tags.back();
 	switch (Tag.m_Type)
@@ -210,52 +288,52 @@ bool cParsedNBT::ReadTag(void)
 
 		case TAG_ByteArray:
 		{
-			NEEDBYTES(4);
+			NEEDBYTES(4, eNBTParseError::npArrayMissingLength);
 			int len = GetBEInt(m_Data + m_Pos);
 			m_Pos += 4;
 			if (len < 0)
 			{
 				// Invalid length
-				return false;
+				return eNBTParseError::npArrayInvalidLength;
 			}
-			NEEDBYTES(len);
+			NEEDBYTES(len, eNBTParseError::npArrayInvalidLength);
 			Tag.m_DataLength = static_cast<size_t>(len);
 			Tag.m_DataStart = m_Pos;
 			m_Pos += static_cast<size_t>(len);
-			return true;
+			return eNBTParseError::npSuccess;
 		}
 
 		case TAG_List:
 		{
-			NEEDBYTES(1);
+			NEEDBYTES(1, eNBTParseError::npListMissingType);
 			eTagType ItemType = static_cast<eTagType>(m_Data[m_Pos]);
 			m_Pos++;
-			RETURN_FALSE_IF_FALSE(ReadList(ItemType));
+			PROPAGATE_ERROR(ReadList(ItemType));
-			return true;
+			return eNBTParseError::npSuccess;
 		}
 
 		case TAG_Compound:
 		{
-			RETURN_FALSE_IF_FALSE(ReadCompound());
+			PROPAGATE_ERROR(ReadCompound());
-			return true;
+			return eNBTParseError::npSuccess;
 		}
 
 		case TAG_IntArray:
 		{
-			NEEDBYTES(4);
+			NEEDBYTES(4, eNBTParseError::npArrayMissingLength);
 			int len = GetBEInt(m_Data + m_Pos);
 			m_Pos += 4;
 			if (len < 0)
 			{
 				// Invalid length
-				return false;
+				return eNBTParseError::npArrayInvalidLength;
 			}
 			len *= 4;
-			NEEDBYTES(len);
+			NEEDBYTES(len, eNBTParseError::npArrayInvalidLength);
 			Tag.m_DataLength = static_cast<size_t>(len);
 			Tag.m_DataStart = m_Pos;
 			m_Pos += static_cast<size_t>(len);
-			return true;
+			return eNBTParseError::npSuccess;
 		}
 
 		#if !defined(__clang__)
@@ -263,7 +341,7 @@ bool cParsedNBT::ReadTag(void)
 		#endif
 		case TAG_Min:
 		{
-			return false;
+			return eNBTParseError::npUnknownTag;
 		}
 	}  // switch (iType)
 }

src/WorldStorage/FastNBT.h

@@ -19,6 +19,7 @@ It directly outputs a string containing the serialized NBT data.
 
 #pragma once
 
+#include <system_error>
 #include "../Endianness.h"
 
 
@@ -107,6 +108,72 @@ public:
 
 
 
+enum class eNBTParseError
+{
+	npSuccess = 0,
+	npNeedBytes,
+	npNoTopLevelCompound,
+	npCompoundImbalancedTag,
+	npStringMissingLength,
+	npStringInvalidLength,
+	npListMissingType,
+	npListMissingLength,
+	npListInvalidLength,
+	npSimpleMissing,
+	npArrayMissingLength,
+	npArrayInvalidLength,
+	npUnknownTag,
+};
+
+
+
+
+
+class cNBTParseErrorCategory final:
+	public std::error_category
+{
+	cNBTParseErrorCategory() = default;
+public:
+	/** Category name */
+	virtual const char * name() const NOEXCEPT override
+	{
+		return "NBT parse error";
+	}
+
+	/** Maps a parse error code to an error message */
+	virtual AString message(int a_Condition) const override;
+
+	/** Returns the canonical error category instance. */
+	static const cNBTParseErrorCategory & Get()
+	{
+		static cNBTParseErrorCategory Category;
+		return Category;
+	}
+};
+
+
+
+
+
+// The following is required to make an error_code constructible from an eNBTParseError
+inline std::error_code make_error_code(eNBTParseError a_Err) NOEXCEPT
+{
+	return { static_cast<int>(a_Err), cNBTParseErrorCategory::Get() };
+}
+
+namespace std
+{
+	template <>
+	struct is_error_code_enum<eNBTParseError>:
+		public std::true_type
+	{
+	};
+}
+
+
+
+
+
 /** Parses and contains the parsed data
 Also implements data accessor functions for tree traversal and value getters
 The data pointer passed in the constructor is assumed to be valid throughout the object's life. Care must be taken not to initialize from a temporary.
@@ -120,10 +187,16 @@ class cParsedNBT
 public:
 	cParsedNBT(const char * a_Data, size_t a_Length);
 
-	bool IsValid(void) const {return m_IsValid; }
+	bool IsValid(void) const { return (m_Error == eNBTParseError::npSuccess); }
+
+	/** Returns the error code for the parsing of the NBT data. */
+	std::error_code GetErrorCode() const { return m_Error; }
+
+	/** Returns the position where an error occurred while parsing. */
+	size_t GetErrorPos() const { return m_Pos; }
 
 	/** Returns the root tag of the hierarchy. */
-	int GetRoot(void) const {return 0; }
+	int GetRoot(void) const { return 0; }
 
 	/** Returns the first child of the specified tag, or -1 if none / not applicable. */
 	int GetFirstChild (int a_Tag) const { return m_Tags[static_cast<size_t>(a_Tag)].m_FirstChild; }
@@ -257,16 +330,16 @@ protected:
 	const char *             m_Data;
 	size_t                   m_Length;
 	std::vector<cFastNBTTag> m_Tags;
-	bool                     m_IsValid;  // True if parsing succeeded
+	eNBTParseError           m_Error;  // npSuccess if parsing succeeded
 
 	// Used while parsing:
 	size_t m_Pos;
 
-	bool Parse(void);
+	eNBTParseError Parse(void);
-	bool ReadString(size_t & a_StringStart, size_t & a_StringLen);  // Reads a simple string (2 bytes length + data), sets the string descriptors
+	eNBTParseError ReadString(size_t & a_StringStart, size_t & a_StringLen);  // Reads a simple string (2 bytes length + data), sets the string descriptors
-	bool ReadCompound(void);  // Reads the latest tag as a compound
+	eNBTParseError ReadCompound(void);  // Reads the latest tag as a compound
-	bool ReadList(eTagType a_ChildrenType);  // Reads the latest tag as a list of items of type a_ChildrenType
+	eNBTParseError ReadList(eTagType a_ChildrenType);  // Reads the latest tag as a list of items of type a_ChildrenType
-	bool ReadTag(void);       // Reads the latest tag, depending on its m_Type setting
+	eNBTParseError ReadTag(void);       // Reads the latest tag, depending on its m_Type setting
 } ;