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();
+}