From 60a8ac9788d6486d64769cc14b152b0f03813bed Mon Sep 17 00:00:00 2001
From: BodgeMaster <>
Date: Fri, 25 Nov 2022 15:45:20 +0100
Subject: [PATCH] lib/nbt: Make constructor NBT::Tag::Compound::Compound(name,
 data) private
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The constructor has been made private and replaced with a static wrapper
function to make constructing from invalid data impossible.

If there is more than one end tag or an end tag isn’t at the end,
an error will be returned.
---
 src/lib/nbt.cpp       | 18 ++++++++++++++++--
 src/lib/nbt.hpp       |  3 ++-
 src/test/nbt_tags.cpp | 14 +++++++++-----
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/src/lib/nbt.cpp b/src/lib/nbt.cpp
index f60a3ca..76aecbf 100644
--- a/src/lib/nbt.cpp
+++ b/src/lib/nbt.cpp
@@ -976,6 +976,20 @@ namespace NBT {
             this->endPointer = new End();
         }
 
+        ErrorOr<Compound*> Compound::constructWithData(tiny_utf8::string name, std::vector<Generic*> data) {
+            if (data.size() > 0) {
+                for (uint64_t i=0; i<data.size(); i++) {
+                    if (data[i]->getTagType() == TagType::END && i != data.size()-1) {
+                        return ErrorOr<Compound*>(true, ErrorCodes::NOT_ALLOWED, nullptr);
+                    }
+                }
+                if (data[data.size()-1]->getTagType() == TagType::END) {
+                    return ErrorOr<Compound*>(new Compound(name, std::vector(data.begin(), data.end()-1)));
+                }
+            }
+            return ErrorOr<Compound*>(new Compound(name, data));
+        }
+
         Compound::~Compound() {
             for (uint64_t i=0; i<this->tags.size(); i++) {
                 delete this->tags.at(i);
@@ -1342,7 +1356,7 @@ namespace NBT {
                         returnValue = ErrorOr<std::vector<Tag::Generic*>>(true, nextCompoundData.errorCode);
                         goto returnError;
                     }
-                    contents.push_back(new Tag::Compound("", nextCompoundData.value));
+                    contents.push_back(reinterpret_cast<Tag::Generic*>(Tag::Compound::constructWithData("", nextCompoundData.value).value));
                     *processedDataSize += *containedDataSize;
                 }
                 delete containedDataSize;
@@ -1454,7 +1468,7 @@ namespace NBT {
                             returnValue = ErrorOr<std::vector<Tag::Generic*>>(true, compoundData.errorCode);
                             goto returnNow;
                         }
-                        tags.push_back(new Tag::Compound(tagName.value, compoundData.value));
+                        tags.push_back(reinterpret_cast<Tag::Generic*>(Tag::Compound::constructWithData(tagName.value, compoundData.value).value));
                         *processedTagSize += (uint64_t) nameSize + 3;
                     }
                     currentPosition += *processedTagSize;
diff --git a/src/lib/nbt.hpp b/src/lib/nbt.hpp
index c0940e0..5a4e3a8 100644
--- a/src/lib/nbt.hpp
+++ b/src/lib/nbt.hpp
@@ -248,10 +248,10 @@ namespace NBT {
                 std::vector<Generic*> tags;
                 // built-in end tag
                 End* endPointer;
+                Compound(tiny_utf8::string name, std::vector<Generic*> data);
             public:
                 Compound();
                 Compound(tiny_utf8::string name);
-                Compound(tiny_utf8::string name, std::vector<Generic*> data);
 
                 ~Compound() override;
 
@@ -262,6 +262,7 @@ namespace NBT {
                 ErrorOrVoid appendPointer(Generic* pointer);
                 ErrorOrVoid deleteElement(uint64_t position);
                 uint64_t length();
+                static ErrorOr<Compound*> constructWithData(tiny_utf8::string name, std::vector<Generic*> data);
         };
 
         class Int32Array: public Generic {
diff --git a/src/test/nbt_tags.cpp b/src/test/nbt_tags.cpp
index 1682c46..79e135c 100644
--- a/src/test/nbt_tags.cpp
+++ b/src/test/nbt_tags.cpp
@@ -890,7 +890,11 @@ int main(){
     NBT::Tag::Compound compound_0 = NBT::Tag::Compound();
     compound_0.name = "compound_0";
     NBT::Tag::Compound compound_1 = NBT::Tag::Compound("compound_1");
-    NBT::Tag::Compound compound_2 = NBT::Tag::Compound("compound_2", compoundDataVector);
+    //TODO: Check that constructing with a vector containing an end tag that
+    // isn’t at the end results in a clearly defined failure mode (issue #60)
+    ErrorOr<NBT::Tag::Compound*> compound_2_or_error = NBT::Tag::Compound::constructWithData("compound_2", compoundDataVector);
+    ASSERT(!compound_2_or_error.isError);
+    NBT::Tag::Compound* compound_2 = compound_2_or_error.value;
 
     ASSERT(!compound_1.appendPointer(new NBT::Tag::Int32("0", 69420)).isError);
     ASSERT(!compound_1.appendPointer(new NBT::Tag::Int8("1", 1)).isError);
@@ -906,15 +910,15 @@ int main(){
     resultNotAllowed = compound_1.setElementPointerAt(0, new NBT::Tag::End());
     ASSERT(resultNotAllowed.isError && resultNotAllowed.errorCode==ErrorCodes::NOT_ALLOWED);
     ASSERT(compound_0.setElementPointerAt(1, new NBT::Tag::Int8()).isError && compound_0.setElementPointerAt(1, new NBT::Tag::Int8()).errorCode == ErrorCodes::OUT_OF_RANGE);
-    ASSERT(!compound_2.deleteElement(1).isError);
+    ASSERT(!compound_2->deleteElement(1).isError);
     ASSERT(compound_0.deleteElement(0).isError && compound_0.deleteElement(0).errorCode == ErrorCodes::NOT_ALLOWED);
     ASSERT(compound_0.deleteElement(1).isError && compound_0.deleteElement(1).errorCode == ErrorCodes::OUT_OF_RANGE);
     ASSERT(compound_0.length() == 1);
     ASSERT(compound_1.length() == 3);
-    ASSERT(compound_2.length() == 4);
+    ASSERT(compound_2->length() == 4);
     compound_0.serialize(&vector);
     compound_1.serialize(&vector);
-    compound_2.serialize(&vector);
+    compound_2->serialize(&vector);
     ASSERT(vector.size() == 95);
     ASSERT(
         vector.at( 0) == 10 &&
@@ -1016,7 +1020,7 @@ int main(){
     vector.clear();
     compound_0.serializeWithoutHeader(&vector);
     compound_1.serializeWithoutHeader(&vector);
-    compound_2.serializeWithoutHeader(&vector);
+    compound_2->serializeWithoutHeader(&vector);
     ASSERT(vector.size() == 56);
     ASSERT(
         vector.at( 0) == 0 &&