lib/nbt: Lists can no longer be constructed from invalid sets of data

The constructor `NBT::Tag::List::List(tiny_utf8::string, std::vector<NBT::Tag::Generic*>)`
is now private and has a wrapper function `NBT::Tag::List::constructWithData(tiny_utf8::string, std::vector<NBT::Tag::Generic*>)`
that performs a sanity check on the data it is given.

src/lib/error.hpp

@@ -95,6 +95,8 @@ namespace ErrorCodes {
     // never be performed (like deleting an end tag from an NBT compound)
     const uint8_t NOT_ALLOWED = 11;
 
+    const uint8_t MIXED_TYPES = 12;
+
     const uint8_t UNIMPLEMENTED = 254;
 
     const uint8_t UNKNOWN = 255;

src/lib/nbt.cpp

@@ -861,6 +861,22 @@ namespace NBT {
             }
         }
 
+        ErrorOr<List*> List::constructWithData(tiny_utf8::string name, std::vector<Generic*> data) {
+            if (data.size() > 0xFFFFFFFF) {
+                return ErrorOr<List*>(true, ErrorCodes::OVERRUN, nullptr);
+            }
+            if (data.size() > 0) {
+                uint8_t dataType = data[0]->getTagType();
+                for (uint32_t i=1; i<data.size(); i++) {
+                    if (data[i]->getTagType() != dataType) {
+                        return ErrorOr<List*>(true, ErrorCodes::MIXED_TYPES, nullptr);
+                    }
+                }
+            }
+
+            return ErrorOr<List*>(new List(name, data));
+        }
+
         List::~List() {
             for (uint64_t i=0; i<this->tags.size(); i++) {
                 delete this->tags.at(i);
@@ -1310,7 +1326,7 @@ namespace NBT {
                         returnValue = ErrorOr<std::vector<Tag::Generic*>>(true, nextListContents.errorCode);
                         goto returnError;
                     }
-                    contents.push_back(new Tag::List("", nextListContents.value));
+                    contents.push_back(Tag::List::constructWithData("", nextListContents.value).value);
                     *processedDataSize += *containedDataSize;
                 }
                 delete containedDataSize;
@@ -1427,7 +1443,7 @@ namespace NBT {
                             returnValue = ErrorOr<std::vector<Tag::Generic*>>(true, listData.errorCode);
                             goto returnNow;
                         }
-                        tags.push_back(new Tag::List(tagName.value, listData.value));
+                        tags.push_back(Tag::List::constructWithData(tagName.value, listData.value).value);
                         *processedTagSize += (uint64_t) nameSize + 3;
                     }
                     if (data[currentPosition]==TagType::COMPOUND) {

src/lib/nbt.hpp

@@ -225,10 +225,10 @@ namespace NBT {
             private:
                 std::vector<Generic*> tags;
                 uint8_t containedType;
+                List(tiny_utf8::string name, std::vector<Generic*> data);
             public:
                 List();
                 List(tiny_utf8::string name, uint8_t type);
-                List(tiny_utf8::string name, std::vector<Generic*> data);
 
                 ~List() override;
 
@@ -240,6 +240,7 @@ namespace NBT {
                 ErrorOrVoid appendPointer(Generic* pointer);
                 ErrorOrVoid deleteElement(uint64_t position);
                 uint64_t length();
+                static ErrorOr<List*> constructWithData(tiny_utf8::string name, std::vector<Generic*> data);
         };
 
         class Compound: public Generic {

src/test/nbt_tags.cpp

@@ -794,24 +794,26 @@ int main(){
     ASSERT(resultError.isError && resultError.errorCode == ErrorCodes::INVALID_TYPE);
     //TODO: Check that constructing with a vector of mixed tags
     // results in a clearly defined failure mode (issue #60)
-    NBT::Tag::List list_2 = NBT::Tag::List("list_2", listDataVector);
+    ErrorOr<NBT::Tag::List*> list_2_or_error = NBT::Tag::List::constructWithData("list_2", listDataVector);
-    ASSERT(list_2.getContainedType() == NBT::TagType::INT8);
+    ASSERT(!list_2_or_error.isError);
-    ASSERT(list_1.length() == 4 && list_2.length() == 4);
+    NBT::Tag::List* list_2 = list_2_or_error.value;
-    ASSERT(!list_2.deleteElement(1).isError);
+    ASSERT(list_2->getContainedType() == NBT::TagType::INT8);
-    ASSERT(list_2.deleteElement(3).isError && list_2.deleteElement(3).errorCode == ErrorCodes::OUT_OF_RANGE);
+    ASSERT(list_1.length() == 4 && list_2->length() == 4);
-    ASSERT(!list_2.getElementPointer(2).isError && dynamic_cast<NBT::Tag::Int8*>(list_2.getElementPointer(2).value)->getValue() == 78);
+    ASSERT(!list_2->deleteElement(1).isError);
-    ASSERT(list_2.getElementPointer(3).isError && list_2.getElementPointer(3).errorCode == ErrorCodes::OUT_OF_RANGE);
+    ASSERT(list_2->deleteElement(3).isError && list_2->deleteElement(3).errorCode == ErrorCodes::OUT_OF_RANGE);
-    ASSERT(!list_2.setElementPointerAt(0, new NBT::Tag::Int8("set_entry", 3)).isError);
+    ASSERT(!list_2->getElementPointer(2).isError && dynamic_cast<NBT::Tag::Int8*>(list_2->getElementPointer(2).value)->getValue() == 78);
-    ErrorOrVoid resultRange = list_2.setElementPointerAt(3, new NBT::Tag::Int8("out_of_range_entry", 2));
+    ASSERT(list_2->getElementPointer(3).isError && list_2->getElementPointer(3).errorCode == ErrorCodes::OUT_OF_RANGE);
+    ASSERT(!list_2->setElementPointerAt(0, new NBT::Tag::Int8("set_entry", 3)).isError);
+    ErrorOrVoid resultRange = list_2->setElementPointerAt(3, new NBT::Tag::Int8("out_of_range_entry", 2));
     ASSERT(resultRange.isError);
     ASSERT(resultRange.errorCode == ErrorCodes::OUT_OF_RANGE);
-    ErrorOrVoid resultType = list_2.setElementPointerAt(0, new NBT::Tag::Int16());
+    ErrorOrVoid resultType = list_2->setElementPointerAt(0, new NBT::Tag::Int16());
     ASSERT(resultType.isError);
     ASSERT(resultType.errorCode == ErrorCodes::INVALID_TYPE);
-    ASSERT(list_2.length() == 3);
+    ASSERT(list_2->length() == 3);
 
     ASSERT(!list_1.serialize(&vector).isError);
-    ASSERT(!list_2.serialize(&vector).isError);
+    ASSERT(!list_2->serialize(&vector).isError);
     ASSERT(vector.size() == 35);
     ASSERT(
         vector.at( 0) == 9 &&
@@ -852,7 +854,7 @@ int main(){
     );
     vector.clear();
     ASSERT(!list_1.serializeWithoutHeader(&vector).isError);
-    ASSERT(!list_2.serializeWithoutHeader(&vector).isError);
+    ASSERT(!list_2->serializeWithoutHeader(&vector).isError);
     ASSERT(vector.size() == 17);
     ASSERT(
         vector.at( 0) == 1 &&