Skip to content

Commit

Permalink
ENG-48: Crash caused by importing negative offset
Browse files Browse the repository at this point in the history
Some faulty MusicXML files include directions with an offset that
points to a tick < 0. This imports, but then crashes as soon as any
command is done on that direction. This commit adds a check upon import
that truncates any negative offset with a larger magnitude than the
associated tick, preventing this issue in MusicXML import.
  • Loading branch information
iveshenry18 authored and vpereverzev committed Jun 15, 2021
1 parent 3476c0f commit 2c2dcab
Show file tree
Hide file tree
Showing 4 changed files with 1,176 additions and 2 deletions.
23 changes: 21 additions & 2 deletions importexport/musicxml/importmxmlpass2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2390,6 +2390,21 @@ static Fraction calcTicks(const QString& text, int divs, MxmlLogger* logger, con
return dura;
}

//---------------------------------------------------------
// preventNegativeTick
//---------------------------------------------------------
/**
Prevent an offset that would result in a negative tick (set offset to -tick instead, resulting in a tick of 0)
*/

static void preventNegativeTick(const Fraction& tick, Fraction& offset, MxmlLogger* logger)
{
if (tick + offset < Fraction(0, 1)) {
logger->logError(QString("illegal offset %1 at tick %2").arg(offset.ticks()).arg(tick.ticks()));
offset = -tick;
}
}


void MusicXMLDelayedDirectionElement::addElem()
{
Expand Down Expand Up @@ -2432,8 +2447,10 @@ void MusicXMLParserDirection::direction(const QString& partId,
while (_e.readNextStartElement()) {
if (_e.name() == "direction-type")
directionType(starts, stops);
else if (_e.name() == "offset")
else if (_e.name() == "offset") {
_offset = calcTicks(_e.readElementText(), divisions, _logger, &_e);
preventNegativeTick(tick, _offset, _logger);
}
else if (_e.name() == "sound")
sound();
else if (_e.name() == "staff") {
Expand Down Expand Up @@ -5262,8 +5279,10 @@ void MusicXMLParserPass2::harmony(const QString& partId, Measure* measure, const
fd = frame(defaultY, relativeY, hasTotalY);
else if (_e.name() == "level")
skipLogCurrElem();
else if (_e.name() == "offset")
else if (_e.name() == "offset") {
offset = calcTicks(_e.readElementText(), _divs, _logger, &_e);
preventNegativeTick(sTime, offset, _logger);
}
else if (_e.name() == "staff") {
int nstaves = _pass1.getPart(partId)->nstaves();
QString strStaff = _e.readElementText();
Expand Down
Loading

0 comments on commit 2c2dcab

Please sign in to comment.