Trade: expect MaterialData attrs to not be larger than end layer offset.

Such an unnecessary footgun -- I was already checking the other case,
having attribute data too short, but this I thought is fine because it's
not leading to any crash. Well it's leading to needless pain and
suffering, that's what it is doing!

And of course already found FIVE such bugs in just Magnum tests alone.

src/Magnum/MaterialTools/Test/PhongToPbrMetallicRoughnessTest.cpp

@@ -152,7 +152,7 @@ const struct {
             {Trade::MaterialAttribute::AlphaMask, 0.7f},
             {Trade::MaterialLayer::ClearCoat},
             {Trade::MaterialAttribute::LayerFactor, 0.35f},
-        }, {2, 4}},
+        }, {3, 5}},
         Trade::MaterialData{Trade::MaterialType::PbrMetallicRoughness, {
             {Trade::MaterialAttribute::BaseColor, 0xff3366ff_rgbaf},
             {Trade::MaterialAttribute::BaseColorTexture, 0u},
@@ -161,7 +161,7 @@ const struct {
             {Trade::MaterialAttribute::AlphaMask, 0.7f},
             {Trade::MaterialLayer::ClearCoat},
             {Trade::MaterialAttribute::LayerFactor, 0.35f},
-        }, {4, 6}}},
+        }, {5, 7}}},
     {"diffuse texture properties without texture", {},
         Trade::MaterialData{{}, {
             {Trade::MaterialAttribute::DiffuseColor, 0xff3366cc_rgbaf},

src/Magnum/Trade/MaterialData.cpp

@@ -321,6 +321,9 @@ MaterialData::MaterialData(const MaterialTypes types, Containers::Array<Material
 
         begin = end;
     }
+
+    CORRADE_ASSERT(layerOffsets.back() == _data.size(),
+        "Trade::MaterialData: last layer offset" << layerOffsets.back() << "too short for" << _data.size() << "attributes in total", );
 }
 
 MaterialData::MaterialData(const MaterialTypes types, const std::initializer_list<MaterialAttributeData> attributeData, const std::initializer_list<UnsignedInt> layerData, const void* const importerState): MaterialData{types, Implementation::initializerListToArrayWithDefaultDeleter(attributeData), Implementation::initializerListToArrayWithDefaultDeleter(layerData), importerState} {}
@@ -358,6 +361,9 @@ MaterialData::MaterialData(const MaterialTypes types, const DataFlags attributeD
 
         begin = end;
     }
+
+    CORRADE_ASSERT(layerOffsets.back() == _data.size(),
+        "Trade::MaterialData: last layer offset" << layerOffsets.back() << "too short for" << _data.size() << "attributes in total", );
     #endif
 }
 

src/Magnum/Trade/MaterialData.h

@@ -2095,7 +2095,7 @@ class MAGNUM_TRADE_EXPORT MaterialData {
          * The @p attributeData gets sorted by name internally, expecting no
          * duplicates inside each layer. The @p layerData is expected to be
          * either empty or a monotonically non-decreasing sequence of offsets
-         * not larger than @p attributeData size, with *i*-th item specifying
+         * counting up to @p attributeData size, with *i*-th item specifying
          * end offset of *i*-th layer.
          *
          * The @ref attributeDataFlags() / @ref layerDataFlags() are implicitly
@@ -2123,7 +2123,7 @@ class MAGNUM_TRADE_EXPORT MaterialData {
          * The @p data is expected to be already sorted by name, without
          * duplicates inside each layer. The @p layerData is expected to be
          * either empty or a monotonically non-decreasing sequence of offsets
-         * not larger than @p attributeData size, with *i*-th item specifying
+         * counting up to @p attributeData size, with *i*-th item specifying
          * end offset of *i*-th layer.
          *
          * The @p attributeDataFlags / @p layerDataFlags parameters can contain

src/Magnum/Trade/Test/AbstractImporterTest.cpp

@@ -6515,7 +6515,7 @@ void AbstractImporterTest::materialNonOwningDeleters() {
             return MaterialData{{}, {}, attributeData, {}, layerData};
         }
 
-        UnsignedInt layerData[1]{};
+        UnsignedInt layerData[1]{1};
         MaterialAttributeData attributeData[1]{
             {MaterialAttribute::DiffuseColor, Color4{}}
         };

src/Magnum/Trade/Test/MaterialDataTest.cpp

@@ -100,6 +100,7 @@ struct MaterialDataTest: TestSuite::Tester {
     void constructLayers();
     void constructLayersNotMonotonic();
     void constructLayersOffsetOutOfBounds();
+    void constructLayersLastOffsetTooShort();
 
     void constructNonOwned();
     void constructNonOwnedLayers();
@@ -108,6 +109,7 @@ struct MaterialDataTest: TestSuite::Tester {
     void constructNonOwnedDuplicateAttribute();
     void constructNonOwnedLayersNotMonotonic();
     void constructNonOwnedLayersOffsetOutOfBounds();
+    void constructNonOwnedLayersLastOffsetTooShort();
     void constructNonOwnedAttributeFlagOwned();
     void constructNonOwnedLayerFlagOwned();
 
@@ -267,6 +269,7 @@ MaterialDataTest::MaterialDataTest() {
               &MaterialDataTest::constructLayers,
               &MaterialDataTest::constructLayersNotMonotonic,
               &MaterialDataTest::constructLayersOffsetOutOfBounds,
+              &MaterialDataTest::constructLayersLastOffsetTooShort,
 
               &MaterialDataTest::constructNonOwned,
               &MaterialDataTest::constructNonOwnedLayers,
@@ -275,6 +278,7 @@ MaterialDataTest::MaterialDataTest() {
               &MaterialDataTest::constructNonOwnedDuplicateAttribute,
               &MaterialDataTest::constructNonOwnedLayersNotMonotonic,
               &MaterialDataTest::constructNonOwnedLayersOffsetOutOfBounds,
+              &MaterialDataTest::constructNonOwnedLayersLastOffsetTooShort,
               &MaterialDataTest::constructNonOwnedAttributeFlagOwned,
               &MaterialDataTest::constructNonOwnedLayerFlagOwned,
 
@@ -1511,6 +1515,20 @@ void MaterialDataTest::constructLayersOffsetOutOfBounds() {
     CORRADE_COMPARE(out.str(), "Trade::MaterialData: invalid range (2, 6) for layer 1 with 5 attributes in total\n");
 }
 
+void MaterialDataTest::constructLayersLastOffsetTooShort() {
+    CORRADE_SKIP_IF_NO_ASSERT();
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    MaterialData data{MaterialType::Phong, {
+        {MaterialAttribute::DoubleSided, true},
+        {MaterialLayer::ClearCoat},
+        {MaterialAttribute::LayerFactor, 0.5f},
+        {MaterialAttribute::NormalTexture, 3u}
+    }, {1, 3}};
+    CORRADE_COMPARE(out.str(), "Trade::MaterialData: last layer offset 3 too short for 4 attributes in total\n");
+}
+
 void MaterialDataTest::constructNonOwned() {
     constexpr MaterialAttributeData attributes[]{
         {"AmbientTextureMatrix"_s, Matrix3{{0.5f, 0.0f, 0.0f},
@@ -1700,6 +1718,28 @@ void MaterialDataTest::constructNonOwnedLayersOffsetOutOfBounds() {
     CORRADE_COMPARE(out.str(), "Trade::MaterialData: invalid range (2, 6) for layer 1 with 5 attributes in total\n");
 }
 
+void MaterialDataTest::constructNonOwnedLayersLastOffsetTooShort() {
+    CORRADE_SKIP_IF_NO_ASSERT();
+
+    MaterialAttributeData attributes[]{
+        {MaterialAttribute::DoubleSided, true},
+        {MaterialLayer::ClearCoat},
+        {MaterialAttribute::LayerFactor, 0.5f},
+        {MaterialAttribute::NormalTexture, 3u}
+    };
+
+    UnsignedInt layers[]{
+        1, 3
+    };
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    MaterialData data{MaterialType::Phong,
+        {}, attributes,
+        {}, layers};
+    CORRADE_COMPARE(out.str(), "Trade::MaterialData: last layer offset 3 too short for 4 attributes in total\n");
+}
+
 void MaterialDataTest::constructNonOwnedAttributeFlagOwned() {
     CORRADE_SKIP_IF_NO_ASSERT();
 
@@ -2953,7 +2993,7 @@ void MaterialDataTest::accessNotFoundInLayerString() {
     MaterialData data{{}, {
         {MaterialAttribute::LayerName, "ClearCoat"},
         {"DiffuseColor", 0xff3366aa_rgbaf}
-    }, {0, 1}};
+    }, {0, 2}};
 
     /* These are fine */
     CORRADE_VERIFY(!data.hasAttribute("ClearCoat", "DiffuseColour"));

src/Magnum/Trade/Test/PbrClearCoatMaterialDataTest.cpp

@@ -464,7 +464,7 @@ void PbrClearCoatMaterialDataTest::commonCoordinatesLayerImplicit() {
         {textureName, 5u},
         {textureName + "Coordinates", 0u},
         {textureName + "Layer", 0u},
-    }, {0, 3}};
+    }, {0, 4}};
 
     /* Zero is treated same as if there would be no attribute at all */
     CORRADE_VERIFY(!data.hasTextureCoordinates());