Make property shorthand parsing fail when passing too many arguments, or arguments in the wrong order

Previously such cases would be silently accepted, which would normally not give what the user intended.

Source/Core/PropertySpecification.cpp

@@ -290,8 +290,7 @@ bool PropertySpecification::ParseShorthandDeclaration(PropertyDictionary& dictio
 		}
 	}
 
-	// If this definition is a 'box'-style shorthand (x-top, x-right, x-bottom, x-left, etc) and there are fewer
-	// than four values
+	// If this definition is a 'box'-style shorthand (x-top x-right x-bottom x-left) that needs replication.
 	if (shorthand_definition->type == ShorthandType::Box && property_values.size() < 4)
 	{
 		// This array tells which property index each side is parsed from
@@ -382,6 +381,10 @@ bool PropertySpecification::ParseShorthandDeclaration(PropertyDictionary& dictio
 		RMLUI_ASSERT(shorthand_definition->type == ShorthandType::Box || shorthand_definition->type == ShorthandType::FallThrough ||
 			shorthand_definition->type == ShorthandType::Replicate || shorthand_definition->type == ShorthandType::Flex);
 
+		// Abort over-specified shorthand values.
+		if (property_values.size() > shorthand_definition->items.size())
+			return false;
+
 		size_t value_index = 0;
 		size_t property_index = 0;
 
@@ -407,6 +410,11 @@ bool PropertySpecification::ParseShorthandDeclaration(PropertyDictionary& dictio
 			if (shorthand_definition->type != ShorthandType::Replicate || value_index < property_values.size() - 1)
 				value_index++;
 		}
+
+		// Abort if we still have values left to parse but no more properties to pass them to.
+		if (shorthand_definition->type != ShorthandType::Replicate && value_index < property_values.size() &&
+			property_index >= shorthand_definition->items.size())
+			return false;
 	}
 
 	return true;

Tests/Source/UnitTests/PropertySpecification.cpp

@@ -28,9 +28,12 @@
 
 #include "../Common/TestsInterface.h"
 #include <RmlUi/Core/Core.h>
+#include <RmlUi/Core/Element.h>
+#include <RmlUi/Core/Factory.h>
 #include <RmlUi/Core/PropertyDefinition.h>
 #include <RmlUi/Core/PropertyDictionary.h>
 #include <RmlUi/Core/PropertySpecification.h>
+#include <RmlUi/Core/StyleSheetSpecification.h>
 #include <doctest.h>
 #include <limits.h>
 
@@ -156,3 +159,67 @@ TEST_CASE("PropertyParser.Keyword")
 
 	Rml::Shutdown();
 }
+
+TEST_CASE("PropertyParser.InvalidShorthands")
+{
+	TestsSystemInterface system_interface;
+	TestsRenderInterface render_interface;
+	SetRenderInterface(&render_interface);
+	SetSystemInterface(&system_interface);
+	Rml::Initialise();
+
+	ElementPtr element = Factory::InstanceElement(nullptr, "*", "div", {});
+
+	struct TestCase {
+		bool expected_result;
+		String name;
+		String value;
+	};
+	Vector<TestCase> tests = {
+		{true, "margin", "10px "},                     //
+		{true, "margin", "10px 20px "},                //
+		{true, "margin", "10px 20px 30px "},           //
+		{true, "margin", "10px 20px 30px 40px"},       //
+		{false, "margin", ""},                         // Too few values
+		{false, "margin", "10px 20px 30px 40px 50px"}, // Too many values
+
+		{true, "flex", "1 2 3px"},    //
+		{false, "flex", "1 2 3px 4"}, // Too many values
+		{false, "flex", "1px 2 3 4"}, // Wrong order
+
+		{true, "perspective-origin", "center center"},         //
+		{true, "perspective-origin", "left top"},              //
+		{false, "perspective-origin", "center center center"}, // Too many values
+		{false, "perspective-origin", "left top 50px"},        // Too many values
+		{false, "perspective-origin", "50px 50px left"},       // Too many values
+		{false, "perspective-origin", "top left"},             // Wrong order
+		{false, "perspective-origin", "50px left"},            // Wrong order
+
+		{true, "font", "20px arial"},  //
+		{false, "font", "arial 20px"}, // Wrong order
+
+		{true, "decorator", "gradient(vertical blue red)"},        //
+		{false, "decorator", "gradient(blue red vertical)"},       // Wrong order
+		{false, "decorator", "gradient(blue vertical red)"},       // Wrong order
+		{false, "decorator", "gradient(vertical blue red green)"}, // Too many values
+
+		{true, "overflow", "hidden"},                //
+		{true, "overflow", "scroll hidden"},         //
+		{false, "overflow", ""},                     // Too few values
+		{false, "overflow", "scroll hidden scroll"}, // Too many values
+	};
+
+	for (const TestCase& test : tests)
+	{
+		PropertyDictionary properties;
+		INFO(test.name, ": ", test.value);
+		CHECK(StyleSheetSpecification::ParsePropertyDeclaration(properties, test.name, test.value) == test.expected_result);
+
+		// Ensure we get a warning when trying to set the invalid property on an element.
+		system_interface.SetNumExpectedWarnings(test.expected_result ? 0 : 1);
+		CHECK(element->SetProperty(test.name, test.value) == test.expected_result);
+	}
+
+	element.reset();
+	Rml::Shutdown();
+}