diff --git a/Source/Core/PropertySpecification.cpp b/Source/Core/PropertySpecification.cpp index 8ed590c2f..9de39893c 100644 --- a/Source/Core/PropertySpecification.cpp +++ b/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; diff --git a/Tests/Source/UnitTests/PropertySpecification.cpp b/Tests/Source/UnitTests/PropertySpecification.cpp index 83dce50d5..00f5d85d3 100644 --- a/Tests/Source/UnitTests/PropertySpecification.cpp +++ b/Tests/Source/UnitTests/PropertySpecification.cpp @@ -28,9 +28,12 @@ #include "../Common/TestsInterface.h" #include +#include +#include #include #include #include +#include #include #include @@ -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 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(); +}