Forum GLPI-Project

SamsonovAnton

Member

Registered: 2012-03-23

Posts: 2

SQL runFile function has a bug dut to oversimplification

When installing GLPI, database creation and population is handled by DBmysql :: runFile function in inc/dbmysql.class.php, which tries to run install/mysql/glpi-0.80.3-empty.sql script in a step-by-step mode. This function simply reads the script file line by line, and if a line ends with a semicolon, it is considered to be the end of current statement. Well, it works fine in most of cases, but is incompatible with INSERT statements with multiline string literals, specifically those containing HTML entities.

Most users don't notice those errors, because they are neither displayed nor logged by default, and the installation process looks like being successfully completed. But if debugging mode is enabled, and SQL errors are logged, you can see something like this:

*** MySQL query error : 
***
SQL: INSERT INTO `glpi_notificationtemplatetranslations` VALUES (\'2\',\'2\',\'\',\'##reservation.action##\',\'======================================================================
##lang.reservation.user##: ##reservation.user##
##lang.reservation.item.name##: ##reservation.itemtype## - ##reservation.item.name##
##IFreservation.tech## ##lang.reservation.tech## ##reservation.tech## ##ENDIFreservation.tech##
##lang.reservation.begin##: ##reservation.begin##
##lang.reservation.end##: ##reservation.end##
##lang.reservation.comment##: ##reservation.comment##
======================================================================
\',\'<!-- description{ color: inherit; background: #ebebeb;border-style: solid;border-color: #8d8d8d; border-width: 0px 1px 1px 0px; } -->

Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''<!-- description{ color: inherit; background: #ebebeb;border-style: solid;bo' at line 1
Backtrace :
/opt/glpi/inc/dbmysql.class.php :482               DBmysql->query()
/opt/glpi/install/install.php :347         DBmysql->runFile()
/opt/glpi/install/install.php :406         fill_db()
/opt/glpi/install/install.php :586         step4()
/opt/glpi/install/install.php

...{130 Kbytes more}...

I don't really understand why is double-escaping needed for HTML code, as well as commented-out CSS styles. Without it, those errors could possibly not surfaced at all.

At a glance, the process of creating and populating the tables is very dumb: instead of stopping on the very first error and reporting it, the installer retries its queries over and over and over and over again, then just ignores all failures, no matter how severe they were (for example, most table were not created), and finally printing a single joyful message: "Database created". Debugging this was a hell.

SamsonovAnton

Member

Registered: 2012-03-23

Posts: 2

Re: SQL runFile function has a bug dut to oversimplification

Proposed solution — replace this block of runFile() function:

      $formattedQuery = "";
      $lastresult     = false;
      while (!feof($DBf_handle)) {
         // specify read length to be able to read long lines
         $buffer = fgets($DBf_handle,102400);

         // do not strip comments due to problems when # in begin of a data line
         $formattedQuery .= $buffer;
         if (substr(rtrim($formattedQuery),-1) == ";") {

            if (get_magic_quotes_runtime()) {
               $formattedQuerytorun = stripslashes($formattedQuery);
            } else {
               $formattedQuerytorun = $formattedQuery;
            }

            // Do not use the $DB->query
            if ($this->query($formattedQuerytorun)) { //if no success continue to concatenate
               $formattedQuery = "";
               $lastresult     = true;
            } else {
               $lastresult = false;
            }
         }
      }

with this one:

      $formattedQuery = "";
      $isInsideStringLiteral = false;
      $lastresult = false;
      while (!feof($DBf_handle)) {
         // specify read length to be able to read long lines
         $buffer = rtrim(fgets($DBf_handle,102400));
         if (get_magic_quotes_runtime()) {
            $buffer = stripslashes($buffer);
         }

         // do not strip comments due to problems when # in begin of a data line
         $formattedQuery .= $buffer;
         $previousChar = "";
         foreach (str_split($buffer) as $char) {
            if ($isInsideStringLiteral) {
               if ($char == "'") {
                  if (($previousChar == "'") || ($previousChar == "\\")) {
                     $previousChar = "";
                  } else {
                     $isInsideStringLiteral = false;
                  }
               } else {
                  $previousChar = $char;
               }
            } elseif ($char == "'") {
               $isInsideStringLiteral = true;
               $previousChar = "";
            }
         }
         if ($isInsideStringLiteral && ($previousChar == "'")) {
            $isInsideStringLiteral = false;
         }

         if (!$isInsideStringLiteral && (substr($buffer,-1) == ";")) {
            $formattedQuerytorun = $formattedQuery;
            $formattedQuery = "";
            // Do not use the $DB->query
            if ($this->query($formattedQuerytorun)) {
               $lastresult = true;
            } else {
               $lastresult = false;
            }
         }
      }

This parser is quiet simple, it only supports single quotes as string delimiters — just enough for current glpi-0.80.3-empty.sql syntax.