Crash on space in score part id

• Mar 16, 2020 - 16:33

Hi

Musescore crashes (exits without message) on this musicXML file.

It seems that the space in this line is killing it.

OS: Windows 10 (10.0), Arch.: x86_64, MuseScore version (64-bit): 3.4.2.9788, revision: 148e43f

Attachment Size
output.musicxml 487 bytes

Comments

Not much in it:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE score-partwise PUBLIC "-//Recordare//DTD MusicXML 3.1 Partwise//EN" "http://www.musicxml.org/dtds/partwise.dtd">
<score-partwise version="3.1">
 <part-list>
  <score-part id="P18 ">
   <part-name>Bass</part-name>
   <score-instrument id="P18-I1">
    <instrument-name>Bass</instrument-name>
   </score-instrument>
  </score-part>
 </part-list>
 <part id="P18">
  <measure number="1">
  </measure>
 </part>
</score-partwise>

MuseScore 1.3 indeed imports it, but bogus.
MuseScore 2.3.2 and 3.4.2 both crash on it.
Latest development build crashes on it too.

And FWIW Capella Reader 8 doesn't read it but gives an error message

In a Debug build not really a crash, but a failed assertion (so more a suicide):

Debug: Error at line 12 col 16: cannot find part 'P18' (...\mscore\importmxmllogger.cpp:49, void Ms::log(Ms::MxmlLogger::Level, const QString&, const QXmlStreamReader*))
Fatal: ASSERT: "part" in file ...\mscore\importmxmlpass1.cpp, line 1869 (...\mscore\importmxmlpass1.cpp:1869, )

But in a non-debug build indeed a real crash, dereferencing a NULL pointer in the next line of code

Simple fix, leading to that score to get imported rather than just emmiting an error message:

diff --git a/mscore/importmxmlpass1.cpp b/mscore/importmxmlpass1.cpp
index 580c93d9b..961512536 100644
--- a/mscore/importmxmlpass1.cpp
+++ b/mscore/importmxmlpass1.cpp
@@ -1656,7 +1656,7 @@ void MusicXMLParserPass1::scorePart()
       {
       Q_ASSERT(_e.isStartElement() && _e.name() == "score-part");
       _logger->logDebugTrace("MusicXMLParserPass1::scorePart", &_e);
-      QString id = _e.attributes().value("id").toString();
+      QString id = _e.attributes().value("id").toString().trimmed();
 
       if (_parts.contains(id)) {
             _logger->logError(QString("duplicate part id '%1'").arg(id), &_e);

I.e. trim leading and trailing whitespace.
Might well be over-simplistic and probably part id should get trimmed() too:

@@ -1885,7 +1885,7 @@ void MusicXMLParserPass1::part()
       {
       Q_ASSERT(_e.isStartElement() && _e.name() == "part");
       _logger->logDebugTrace("MusicXMLParserPass1::part", &_e);
-      const QString id = _e.attributes().value("id").toString();
+      const QString id = _e.attributes().value("id").toString().trimmed();
 
       if (!_parts.contains(id)) {
             _logger->logError(QString("cannot find part '%1'").arg(id), &_e);

Spaces in the score-part's id (such as after P18) are not allowed, but it is not completely clear to me if this implies file output.musicxml is invalid or that the parser should remove the spaces.

It is clear that error handling in the MusicXML importer is insufficient for the (corner) case where part-list and parts do not match. This should result in an error message instead of a crash. As these errors do not happen frequently, I consider fixing it a low priority.

Do you still have an unanswered question? Please log in first to post your question.