Moodle
  1. Moodle
  2. MDL-23431

XMLDB editor fails to parse data properly in certain cases + fix

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.9
    • Fix Version/s: 1.9.10
    • Component/s: Database SQL/XMLDB
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      27213

      Description

      I've just found that some data we've generated failed on the install of our block with this error:

      Errors found in XMLDB file: Problem loading statement insert block_qualmgr_criterion, Incorrect number of fields (learn_outcome_id, name, description) VALUES ('10618', '2.5', 'd) or values ('10618', '2.5', 'd) Values and beliefs')

      The problem is that the 'fields' bit has overrun to the next bracket along and hasn't stopped where it should, so it thinks that the comma separated list of DB fields looks like this:

      learn_outcome_id, name, description) VALUES ('10618', '2.5', 'd

      It comes from this line in our install.xml file:

      <SENTENCE TEXT="(learn_outcome_id, name, description) VALUES ('10618', '2.5', 'd) Values and beliefs')" />

      Which gets interpreted by this regexp in /lib/xmldb/classes/XMLDBStatement.class.php on line 339:

      preg_match('/^\((.*)\)\s+VALUES/is', $sentence, $matches);
      

      The problem being that any data containing ') value' is going to mess this up because the regexp is greedy. Changing it so that the regexp is not case sensitive would be an improvement, but even better would be to prevent this from being a greedy match too, like this:

      preg_match('/^\((.*)\)\s+VALUES/Us', $sentence, $matches);
      

        Activity

        Hide
        Petr Škoda added a comment -

        fixed, we are going to remove support fro STATEMENTS from install.xml, please use install.php in 2.0
        Thanks for the report and patch!!

        Petr Skoda

        Show
        Petr Škoda added a comment - fixed, we are going to remove support fro STATEMENTS from install.xml, please use install.php in 2.0 Thanks for the report and patch!! Petr Skoda
        Hide
        Petr Škoda added a comment -

        grrrr, sorry I forgot to give you credit in the commit message

        Show
        Petr Škoda added a comment - grrrr, sorry I forgot to give you credit in the commit message
        Hide
        Petr Škoda added a comment -

        btw we are going to migrate to git around Christmas, that should help a lot in the future...

        Show
        Petr Škoda added a comment - btw we are going to migrate to git around Christmas, that should help a lot in the future...
        Hide
        Matt Gibson added a comment -

        No problem! Thanks for commiting the change.

        Show
        Matt Gibson added a comment - No problem! Thanks for commiting the change.

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: