daePlugin convert istringstream occasionally fails

[unelsson]

In project OpenMW, we are using virtual file system for storing collada files. When loading from cache, istringstream method convert fails, giving error messages:
Error: Extra content at the end of the document
Error: Failed to load XML document from memory

My guess is that collada-dom is expecting the end of file or end of line character or something like that. I'm not sure if this bug is related to using vfs, or if it should be corrected in OSG repository.

A very "classic" fix works here through: Adding "+1" to this line fixed random loading from cache errors:

OpenSceneGraph/src/osgPlugins/dae/daeReader.cpp

Line 297 in 9dad0b4

std::streampos length = fin.tellg();

std::streampos length = static_cast<int>(fin.tellg()) + 1;

[openscenegraph]

THis looks more like a "classic hack" rather than a "classic fix". It would be worth looking to see if it's an issue with the daeString not being null terminated. If so the fix should be to create a buffer one longer and then explicitly set the last character 0.

…

On Sat, 7 Nov 2020 at 10:05, unelsson ***@***.***> wrote: In project OpenMW, we are using virtual file system for storing collada files. When loading from cache, istringstream method convert fails, giving error messages: Error: Extra content at the end of the document Error: Failed to load XML document from memory My guess is that collada-dom is expecting the end of file or end of line character or something like that. I'm not sure if this bug is related to using vfs, or if it should be corrected in OSG repository. A very "classic" fix works here through: Adding "+1" to this line fixed random loading from cache errors: https://github.com/openscenegraph/OpenSceneGraph/blob/9dad0b4148249ca1379530365611abc183131326/src/osgPlugins/dae/daeReader.cpp#L297 std::streampos length = static_cast<int>(fin.tellg()) + 1; — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#994>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABUA23JWNGUPXTP5IF72ADSOULVTANCNFSM4TNRX5WA> .

[unelsson]

The issue is indeed related to lack of null-termination. A decent fix is adding buffer.emplace_back('\0'); here which does exactly what you suggest. Should I make a PR?

I am not experienced in handling std::istream, but to my understanding, it is terminated by \0, and the function bool daeReader::convert( std::istream& fin ) uses C string handling methods that always create std::vector<char> buffer without the null in the end. That sounds like it's guaranteed to fail?

[unelsson]

For convenience, I made a PR that adds buffer.emplace_back('\0');.
#1012