Crash when material contains unknown tags

There are COLLADA exporters that insert additional tags into the material/technique definitions. And right now, COLLADA2GLTF crashes when it encounters a technique definition like this:

...
128.0

Particularly, this appears in a COLLADA file that was exported from http://www.makehuman.org/ (I opened a related issue there: http://bugtracker.makehumancommunity.org/issues/1084 ). Although, strictly speaking, these additional tags are *not* compliant to the COLLADA spec, I think that such tags should be handled more graciously.

---

I'm not very familiar with COLLADA, XML and COLLADA2GLTF right now, and thus made a very "pragmatic" fix that prevents the crash. Here is it as a unified diff, in case someone wants to have a look at it:

5f6d7ccf38fc5cefcda3eb19f518c28fde4d2ec0
GLTF/GLTFExtraDataHandler.cpp | 45 +++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/GLTF/GLTFExtraDataHandler.cpp b/GLTF/GLTFExtraDataHandler.cpp
index d4dcd88..bf82c00 100644
--- a/GLTF/GLTFExtraDataHandler.cpp
+++ b/GLTF/GLTFExtraDataHandler.cpp
@@ -37,9 +37,13 @@ namespace GLTF
*/

//------------------------------
- ExtraDataHandler::ExtraDataHandler() : mExtraTagType(EXTRA_TAG_TYPE_UNKNOWN)
+ ExtraDataHandler::ExtraDataHandler() :
+ mTextBuffer(),
+ mExtraTagType(EXTRA_TAG_TYPE_UNKNOWN),
+ mCurrentElementUniqueId(),
+ mCurrentObject(),
+ _allExtras(new JSONObject())
{
- _allExtras = shared_ptr (new JSONObject());
}

//------------------------------
@@ -67,23 +71,26 @@ namespace GLTF
if ((bump == nullptr) || (textureAttributes == nullptr))
return;

- size_t index = 0;
-
- const GeneratedSaxParser::xmlChar* attributeKey = attributes[index++];
- const GeneratedSaxParser::xmlChar* attributeValue = 0;
- while( attributeKey != 0 ) {
- attributeValue = attributes[index++];
- if( attributeValue != 0 ) {
- bump->setString(attributeKey, attributeValue);
- }
-
- if (strcmp(attributeKey, "texture") == 0) {
- textureAttributes->textureSampler = attributeValue;
- } else if (strcmp(attributeKey, "texcoord")) {
- textureAttributes->texCoord = attributeValue;
- }
- attributeKey = attributes[index++];
- }
+ if (attributes) {
+ size_t index = 0;
+
+ const GeneratedSaxParser::xmlChar* attributeKey = attributes[index++];
+ const GeneratedSaxParser::xmlChar* attributeValue = 0;
+ while (attributeKey != 0) {
+ attributeValue = attributes[index++];
+ if (attributeValue != 0) {
+ bump->setString(attributeKey, attributeValue);
+ }
+
+ if (strcmp(attributeKey, "texture") == 0) {
+ textureAttributes->textureSampler = attributeValue;
+ }
+ else if (strcmp(attributeKey, "texcoord")) {
+ textureAttributes->texCoord = attributeValue;
+ }
+ attributeKey = attributes[index++];
+ }
+ }
}

//------------------------------

(As far as I know, it is a common (and **important**) "best practice" to initialize **all** members of a class in the constructor, *exactly* in the order in which they are declared in the header - see https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-complete and C.45, C.47 and C.49).

I'm not sure whether or not the pragmatic ` if (attributes) {` is acceptable here, that's why I hesitate to make this a pull request. If this fix is OK and this should be a PR, just drop me a note.

It prevents the crash, and afterwards, COLLADA2GLTF generates a glTF file, but this glTF still has many other issues that I have not yet analyzed in detail.

Start by reading GLTF/GLTFExtraDataHandler.cpp and reproduce the crash with the COLLADA technique example containing extra tags such as normal, bump, displacement, and extra. Review the proposed handling of missing attributes and unknown elements, then verify that COLLADA2GLTF no longer crashes and still generates a glTF file.

Written by the indexing model from the issue text.