Change AMFArray getters to use Templates and fix CI halting when one matrix fails (#796)

* Change AMFArray getters to use Templates

Move Template definition to header

* Add more tests

Add tests for casting to wrong template type
Add tests for going out of bounds in the array.

* Try continue-on-error

* Update build-and-test.yml

* Try continue-on-error

Update build-and-test.yml

* change version

* Update CMakeMariaDBLists.txt

Update CMakeMariaDBLists.txt

.github/workflows/build-and-test.yml

@@ -10,6 +10,7 @@ jobs:
   build-and-test:
     name: Build & Test (${{ matrix.os }})
     runs-on: ${{ matrix.os }}
+    continue-on-error: true
     strategy:
       matrix:
         os: [ windows-2022, ubuntu-20.04, macos-11 ]

dCommon/AMFFormat.cpp

@@ -58,16 +58,6 @@ void AMFArrayValue::RemoveValue(const std::string& key) {
 	}
 }
 
-// AMFArray Find Value
-AMFValue* AMFArrayValue::FindValue(const std::string& key) {
-	_AMFArrayMap_::iterator it = this->associative.find(key);
-	if (it != this->associative.end()) {
-		return it->second;
-	}
-
-	return nullptr;
-}
-
 // AMFArray Get Associative Iterator Begin
 _AMFArrayMap_::iterator AMFArrayValue::GetAssociativeIteratorValueBegin() {
 	return this->associative.begin();
@@ -93,11 +83,6 @@ uint32_t AMFArrayValue::GetDenseValueSize() {
 	return (uint32_t)this->dense.size();
 }
 
-// AMFArray Get value at index in Dense List
-AMFValue* AMFArrayValue::GetValueAt(uint32_t index) {
-	return this->dense.at(index);
-}
-
 // AMFArray Get Dense Iterator Begin
 _AMFArrayList_::iterator AMFArrayValue::GetDenseIteratorBegin() {
 	return this->dense.begin();

dCommon/AMFFormat.h

@@ -75,7 +75,9 @@ private:
 	/*!
 	  \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFUndefined; }
+	AMFValueType GetValueType() { return ValueType; }
+public:
+	static const AMFValueType ValueType = AMFUndefined;
 };
 
 //! The null value AMF type
@@ -85,7 +87,9 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFNull; }
+	AMFValueType GetValueType() { return ValueType; }
+public:
+	static const AMFValueType ValueType = AMFNull;
 };
 
 //! The false value AMF type
@@ -95,7 +99,9 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFFalse; }
+	AMFValueType GetValueType() { return ValueType; }
+public:
+	static const AMFValueType ValueType = AMFFalse;
 };
 
 //! The true value AMF type
@@ -105,7 +111,9 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFTrue; }
+	AMFValueType GetValueType() { return ValueType; }
+public:
+	static const AMFValueType ValueType = AMFTrue;
 };
 
 //! The integer value AMF type
@@ -117,9 +125,10 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFInteger; }
+	AMFValueType GetValueType() { return ValueType; }
 
 public:
+	static const AMFValueType ValueType = AMFInteger;
 	//! Sets the integer value
 	/*!
 	  \param value The value to set
@@ -142,9 +151,10 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFDouble; }
+	AMFValueType GetValueType() { return ValueType; }
 
 public:
+	static const AMFValueType ValueType = AMFDouble;
 	//! Sets the double value
 	/*!
 	  \param value The value to set to
@@ -167,9 +177,10 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFString; }
+	AMFValueType GetValueType() { return ValueType; }
 
 public:
+	static const AMFValueType ValueType = AMFString;
 	//! Sets the string value
 	/*!
 	  \param value The string value to set to
@@ -192,9 +203,10 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFXMLDoc; }
+	AMFValueType GetValueType() { return ValueType; }
 
 public:
+	static const AMFValueType ValueType = AMFXMLDoc;
 	//! Sets the XML Doc value
 	/*!
 	  \param value The value to set to
@@ -217,9 +229,10 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFDate; }
+	AMFValueType GetValueType() { return ValueType; }
 
 public:
+	static const AMFValueType ValueType = AMFDate;
 	//! Sets the date time
 	/*!
 	  \param value The value to set to
@@ -244,9 +257,11 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFArray; }
+	AMFValueType GetValueType() { return ValueType; }
 
 public:
+	static const AMFValueType ValueType = AMFArray;
+
 	~AMFArrayValue() override;
 	//! Inserts an item into the array map for a specific key
 	/*!
@@ -265,7 +280,15 @@ public:
 	/*!
 	  \return The AMF value if found, nullptr otherwise
 	 */
-	AMFValue* FindValue(const std::string& key);
+	template <typename T>
+	T* FindValue(const std::string& key) const {
+		_AMFArrayMap_::const_iterator it = this->associative.find(key);
+		if (it != this->associative.end() && T::ValueType == it->second->GetValueType()) {
+			return dynamic_cast<T*>(it->second);
+		}
+
+		return nullptr;
+	};
 
 	//! Returns where the associative iterator begins
 	/*!
@@ -298,7 +321,12 @@ public:
 	/*!
 	  \param index The index to get
 	 */
-	AMFValue* GetValueAt(uint32_t index);
+	template <typename T>
+	T* GetValueAt(uint32_t index) {
+		if (index >= this->dense.size()) return nullptr;
+		AMFValue* foundValue = this->dense.at(index);
+		return T::ValueType == foundValue->GetValueType() ? dynamic_cast<T*>(foundValue) : nullptr;
+	};
 
 	//! Returns where the dense iterator begins
 	/*!
@@ -334,10 +362,11 @@ private:
 	/*!
 	 \return The AMF value type
 	 */
-	AMFValueType GetValueType() { return AMFObject; }
+	AMFValueType GetValueType() { return ValueType; }
 	~AMFObjectValue() override;
 
 public:
+	static const AMFValueType ValueType = AMFObject;
 	//! Constructor
 	/*!
 	  \param traits The traits to set

tests/AMFDeserializeTests.cpp

@@ -146,8 +146,8 @@ int ReadAMFArrayFromBitStream() {
 		ASSERT_EQ(res->GetValueType(), AMFValueType::AMFArray);
 		ASSERT_EQ(static_cast<AMFArrayValue*>(res.get())->GetAssociativeMap().size(), 1);
 		ASSERT_EQ(static_cast<AMFArrayValue*>(res.get())->GetDenseArray().size(), 1);
-		ASSERT_EQ(static_cast<AMFStringValue*>(static_cast<AMFArrayValue*>(res.get())->FindValue("BehaviorID"))->GetStringValue(), "10447");
+		ASSERT_EQ(static_cast<AMFArrayValue*>(res.get())->FindValue<AMFStringValue>("BehaviorID")->GetStringValue(), "10447");
-		ASSERT_EQ(static_cast<AMFStringValue*>(static_cast<AMFArrayValue*>(res.get())->GetDenseArray()[0])->GetStringValue(), "10447");
+		ASSERT_EQ(static_cast<AMFArrayValue*>(res.get())->GetValueAt<AMFStringValue>(0)->GetStringValue(), "10447");
 	}
 	// Test a dense array
 	return 0;
@@ -219,102 +219,103 @@ int TestLiveCapture() {
 	auto result = static_cast<AMFArrayValue*>(resultFromFn.get());
 	// Test the outermost array
 
-	ASSERT_EQ(dynamic_cast<AMFStringValue*>(result->FindValue("BehaviorID"))->GetStringValue(), "10447");
+	ASSERT_EQ(result->FindValue<AMFStringValue>("BehaviorID")->GetStringValue(), "10447");
-	ASSERT_EQ(dynamic_cast<AMFStringValue*>(result->FindValue("objectID"))->GetStringValue(), "288300744895913279")
+	ASSERT_EQ(result->FindValue<AMFStringValue>("objectID")->GetStringValue(), "288300744895913279");
 
 	// Test the execution state array
-		auto executionState = dynamic_cast<AMFArrayValue*>(result->FindValue("executionState"));
+	auto executionState = result->FindValue<AMFArrayValue>("executionState");
+
 	ASSERT_NE(executionState, nullptr);
 
-	auto strips = dynamic_cast<AMFArrayValue*>(executionState->FindValue("strips"))->GetDenseArray();
+	auto strips = executionState->FindValue<AMFArrayValue>("strips")->GetDenseArray();
 
 	ASSERT_EQ(strips.size(), 1);
 
 	auto stripsPosition0 = dynamic_cast<AMFArrayValue*>(strips[0]);
 
-	auto actionIndex = dynamic_cast<AMFDoubleValue*>(stripsPosition0->FindValue("actionIndex"));
+	auto actionIndex = stripsPosition0->FindValue<AMFDoubleValue>("actionIndex");
 
 	ASSERT_EQ(actionIndex->GetDoubleValue(), 0.0f);
 
-	auto stripIDExecution = dynamic_cast<AMFDoubleValue*>(stripsPosition0->FindValue("id"));
+	auto stripIDExecution = stripsPosition0->FindValue<AMFDoubleValue>("id");
 
 	ASSERT_EQ(stripIDExecution->GetDoubleValue(), 0.0f);
 
-	auto stateIDExecution = dynamic_cast<AMFDoubleValue*>(executionState->FindValue("stateID"));
+	auto stateIDExecution = executionState->FindValue<AMFDoubleValue>("stateID");
 
 	ASSERT_EQ(stateIDExecution->GetDoubleValue(), 0.0f);
 
-	auto states = dynamic_cast<AMFArrayValue*>(result->FindValue("states"))->GetDenseArray();
+	auto states = result->FindValue<AMFArrayValue>("states")->GetDenseArray();
 
 	ASSERT_EQ(states.size(), 1);
 
 	auto firstState = dynamic_cast<AMFArrayValue*>(states[0]);
 
-	auto stateID = dynamic_cast<AMFDoubleValue*>(firstState->FindValue("id"));
+	auto stateID = firstState->FindValue<AMFDoubleValue>("id");
 
 	ASSERT_EQ(stateID->GetDoubleValue(), 0.0f);
 
-	auto stripsInState = dynamic_cast<AMFArrayValue*>(firstState->FindValue("strips"))->GetDenseArray();
+	auto stripsInState = firstState->FindValue<AMFArrayValue>("strips")->GetDenseArray();
 
 	ASSERT_EQ(stripsInState.size(), 1);
 
 	auto firstStrip = dynamic_cast<AMFArrayValue*>(stripsInState[0]);
 
-	auto actionsInFirstStrip = dynamic_cast<AMFArrayValue*>(firstStrip->FindValue("actions"))->GetDenseArray();
+	auto actionsInFirstStrip = firstStrip->FindValue<AMFArrayValue>("actions")->GetDenseArray();
 
 	ASSERT_EQ(actionsInFirstStrip.size(), 3);
 
-	auto actionID = dynamic_cast<AMFDoubleValue*>(firstStrip->FindValue("id"));
+	auto actionID = firstStrip->FindValue<AMFDoubleValue>("id");
 
 	ASSERT_EQ(actionID->GetDoubleValue(), 0.0f)
 
-		auto uiArray = dynamic_cast<AMFArrayValue*>(firstStrip->FindValue("ui"));
+		auto uiArray = firstStrip->FindValue<AMFArrayValue>("ui");
 
-	auto xPos = dynamic_cast<AMFDoubleValue*>(uiArray->FindValue("x"));
+	auto xPos = uiArray->FindValue<AMFDoubleValue>("x");
-	auto yPos = dynamic_cast<AMFDoubleValue*>(uiArray->FindValue("y"));
+	auto yPos = uiArray->FindValue<AMFDoubleValue>("y");
 
 	ASSERT_EQ(xPos->GetDoubleValue(), 103.0f);
 	ASSERT_EQ(yPos->GetDoubleValue(), 82.0f);
 
-	auto stripID = dynamic_cast<AMFDoubleValue*>(firstStrip->FindValue("id"));
+	auto stripID = firstStrip->FindValue<AMFDoubleValue>("id");
 
 	ASSERT_EQ(stripID->GetDoubleValue(), 0.0f)
 
 		auto firstAction = dynamic_cast<AMFArrayValue*>(actionsInFirstStrip[0]);
 
-	auto firstType = dynamic_cast<AMFStringValue*>(firstAction->FindValue("Type"));
+	auto firstType = firstAction->FindValue<AMFStringValue>("Type");
 
 	ASSERT_EQ(firstType->GetStringValue(), "OnInteract");
 
-	auto firstCallback = dynamic_cast<AMFStringValue*>(firstAction->FindValue("__callbackID__"));
+	auto firstCallback = firstAction->FindValue<AMFStringValue>("__callbackID__");
 
 	ASSERT_EQ(firstCallback->GetStringValue(), "");
 
 	auto secondAction = dynamic_cast<AMFArrayValue*>(actionsInFirstStrip[1]);
 
-	auto secondType = dynamic_cast<AMFStringValue*>(secondAction->FindValue("Type"));
+	auto secondType = secondAction->FindValue<AMFStringValue>("Type");
 
 	ASSERT_EQ(secondType->GetStringValue(), "FlyUp");
 
-	auto secondCallback = dynamic_cast<AMFStringValue*>(secondAction->FindValue("__callbackID__"));
+	auto secondCallback = secondAction->FindValue<AMFStringValue>("__callbackID__");
 
 	ASSERT_EQ(secondCallback->GetStringValue(), "");
 
-	auto secondDistance = dynamic_cast<AMFDoubleValue*>(secondAction->FindValue("Distance"));
+	auto secondDistance = secondAction->FindValue<AMFDoubleValue>("Distance");
 
 	ASSERT_EQ(secondDistance->GetDoubleValue(), 25.0f);
 
 	auto thirdAction = dynamic_cast<AMFArrayValue*>(actionsInFirstStrip[2]);
 
-	auto thirdType = dynamic_cast<AMFStringValue*>(thirdAction->FindValue("Type"));
+	auto thirdType = thirdAction->FindValue<AMFStringValue>("Type");
 
 	ASSERT_EQ(thirdType->GetStringValue(), "FlyDown");
 
-	auto thirdCallback = dynamic_cast<AMFStringValue*>(thirdAction->FindValue("__callbackID__"));
+	auto thirdCallback = thirdAction->FindValue<AMFStringValue>("__callbackID__");
 
 	ASSERT_EQ(thirdCallback->GetStringValue(), "");
 
-	auto thirdDistance = dynamic_cast<AMFDoubleValue*>(thirdAction->FindValue("Distance"));
+	auto thirdDistance = thirdAction->FindValue<AMFDoubleValue>("Distance");
 
 	ASSERT_EQ(thirdDistance->GetDoubleValue(), 25.0f);
 
@@ -327,6 +328,42 @@ int TestNullStream() {
 	return 0;
 }
 
+int TestBadConversion() {
+	std::ifstream testFileStream;
+	testFileStream.open("AMFBitStreamTest.bin", std::ios::binary);
+
+	// Read a test BitStream from a file
+	RakNet::BitStream testBitStream;
+	char byte = 0;
+	while (testFileStream.get(byte)) {
+		testBitStream.Write<char>(byte);
+	}
+
+	testFileStream.close();
+
+	auto resultFromFn = ReadFromBitStream(&testBitStream);
+	auto result = static_cast<AMFArrayValue*>(resultFromFn.get());
+
+	// Actually a string value.
+	ASSERT_EQ(result->FindValue<AMFDoubleValue>("BehaviorID"), nullptr);
+
+	// Does not exist in the associative portion
+	ASSERT_EQ(result->FindValue<AMFNullValue>("DOES_NOT_EXIST"), nullptr);
+
+	result->PushBackValue(new AMFTrueValue());
+
+	// Exists and is correct type
+	ASSERT_NE(result->GetValueAt<AMFTrueValue>(0), nullptr);
+
+	// Value exists but is wrong typing
+	ASSERT_EQ(result->GetValueAt<AMFFalseValue>(0), nullptr);
+
+	// Value is out of bounds
+	ASSERT_EQ(result->GetValueAt<AMFTrueValue>(1), nullptr);
+
+	return 0;
+}
+
 int AMFDeserializeTests(int argc, char** const argv) {
 	std::cout << "Checking that using a null bitstream doesnt cause exception" << std::endl;
 	if (TestNullStream()) return 1;
@@ -343,6 +380,8 @@ int AMFDeserializeTests(int argc, char** const argv) {
 	if (TestLiveCapture() != 0) return 1;
 	std::cout << "Passed live capture, checking unimplemented amf values" << std::endl;
 	if (TestUnimplementedAMFValues() != 0) return 1;
+	std::cout << "Passed unimplemented values, checking poor casting" << std::endl;
+	if (TestBadConversion() != 0) return 1;
 	std::cout << "Passed all tests." << std::endl;
 	return 0;
 }