Infinite loop with an XML

• Aug 2, 2016 - 14:44
Reported version
2.1
Type
Functional
Severity
S3 - Major
Status
closed
Project
Tags

The attached XML is slightly malformed, but in a way that produces an infinite loop rather than an error message.

The fix is actually quite simple in this case:
importxmlpass2.cpp line 4502 (in fn note()) needs a check for EndDocument too.

while (!(_e.tokenType() == QXmlStreamReader::StartElement)
&& !(_e.tokenType() == QXmlStreamReader::EndElement && _e.name() == "note")
&& !(_e.tokenType() == QXmlStreamReader::EndDocument ));

However, the same issue might occur in other slightly differently malformed cases as well so I'd prefer someone more knowledgeable of the relevant code do the PR.

Attachment Size
infloop.xml 57.62 KB

Comments

The DOM-tree does not seem to be malformed on closer inspection, so this is a somewhat more critical bug (as infinite loops are usually considered worse than segfaults).

@lasconic, could you attach the changed file that does work ? That would possibly make understanding the issue easier.

Note that on my (slightly older) version, I get a failed assertion:

ASSERT: "_e.isEndElement() && _e.name() == "notations"" in file /Users/lvidev/dev/MuseScore/mscore/importmxmlpass2.cpp, line 5846

Here is the file. I just opened the one attach in the top post with a text editor and reindent it automatically. I does open with a nightly once reindented.

Attachment Size
infloop.xml 104.72 KB

For me, using the current trunk (37e531e), running on OS X, I still do not get an infinite loop but a failed assertion. This seems to be caused by (lack of) white space in the last notations element. Two files are attached (differing only in white space), with one imposing correctly and one failing the same assertion as infloop.xml.

@margus.niitsoo, what behaviour do you get for files notations_OK.xml and notations_fail.xml ? Which museScore version are you running on which platform ?

PS: file infloop.xml is actually invalid MusicXML (due to incorrect order of the chromatic and diatonic elements), but this does not seem to affect the failed assertion.

Attachment Size
notations_OK.xml 1.88 KB
notations_fail.xml 1.83 KB
Reported version 3.0 2.1

Seems notation_fail.xml causes 2.0.3 to go into the endless loop (probably because in RELEASE mode the Q_ASSERT() is disabled), so it is affected too, and the fix should probably go into the 2.0.4 branch too