Skip to content

Commit

Permalink
Throw exceptions on SQL errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@cedric-anne
cedric-anne committed Oct 29, 2024
1 parent 2e62db0 commit 1c82c4a
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 165 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ The present file will list all changes made to the project; according to the
- Usage of `DBmysql::query()`, `DBmysql::queryOrDie()` method are prohibited to ensure that legacy unsafe DB are no more executed.
Building and executing raw queries using `DBmysql::request()`, `DBmysqlIterator::buildQuery()` and `DBmysqlIterator::execute()` methods is also prohibited.
To execute DB queries, either `DBmysql::request()` can be used to craft query using the GLPI query builder,
either `DBmysql::doQuery()`/`DBmysql::doQueryOrDie()` can be used for safe queries to execute DB query using a self-crafted a SQL string.
either `DBmysql::doQuery()` can be used for safe queries to execute DB query using a self-crafted a SQL string.
- `js/fuzzysearch.js` replaced with `FuzzySearch/Modal` Vue component.
- `Html::fuzzySearch()` replaced with `Html::getMenuFuzzySearchList()` function.
- `NotificationEvent::raiseEvent()` signature cahnged. A new `$trigger` parameter has been added at 4th position, and `$label` is now the 5th parameter.
Expand Down Expand Up @@ -239,8 +239,11 @@ The present file will list all changes made to the project; according to the
- `ComputerVirtualMachine` has been deprecated and replaced by `ItemVirtualMachine`
- `Config::validatePassword()`
- `Contract::getExpiredCriteria()` renamed to `Contract::getNotExpiredCriteria()` to match the actual behavior.
- `DBmysql::deleteOrDie()`. Use `DBmysql::delete()` instead.
- `DBmysql::insertOrDie()`. Use `DBmysql::insert()` instead.
- `DBmysql::truncate()`
- `DBmysql::truncateOrDie()`
- `DBmysql::updateOrDie()`. Use `DBmysql::update()` instead.
- `Document::getImage()`
- `Glpi\Application\View\Extension\DataHelpersExtension::getVerbatimValue()`
- `Glpi\Application\View\Extension\PluginExtension::getPluginWebDir()`
Expand Down Expand Up @@ -368,6 +371,7 @@ The present file will list all changes made to the project; according to the
- `GLPI::getLogLevel()`
- `Glpi\Api\API::showDebug()`
- `Glpi\Api\API::returnSanitizedContent()`
- `Glpi\Application\ErrorHandler::handleSqlError()`
- `Glpi\Dashboard\Filter::dates()`
- `Glpi\Dashboard\Filter::dates_mod()`
- `Glpi\Dashboard\Filter::itilcategory()`
Expand Down
164 changes: 49 additions & 115 deletions src/DBmysql.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ public function escape($string)
*/
public function query($query)
{
trigger_error('Executing direct queries is not allowed!', E_USER_ERROR);
throw new \Exception('Executing direct queries is not allowed!');
}

/**
Expand All @@ -383,16 +383,14 @@ public function doQuery($query)

$res = $this->dbh->query($query);
if (!$res) {
// no translation for error logs
$error = " *** MySQL query error:\n SQL: " . $query . "\n Error: " .
$this->dbh->error . "\n";
$error .= Toolbox::backtrace(false, 'DBmysql->doQuery()', ['Toolbox::backtrace()']);

Toolbox::logSqlError($error);

ErrorHandler::getInstance()->handleSqlError($this->dbh->errno, $this->dbh->error, $query);

$debug_data['errors'] = $this->error();
throw new \RuntimeException(
sprintf(
'MySQL query error: %s (%d) in SQL query "%s".',
$this->dbh->error,
$this->dbh->errno,
$query
)
);
}

$duration = (microtime(true) - $start_time) * 1000;
Expand Down Expand Up @@ -458,66 +456,45 @@ static function ($warning) {
*/
public function queryOrDie($query, $message = '')
{
trigger_error('Executing direct queries is not allowed!', E_USER_ERROR);
throw new \Exception('Executing direct queries is not allowed!');
}

/**
* Execute a MySQL query and die
* (optionnaly with a message) if it fails
* Execute a MySQL query and throw an exception if it fails.
*
* @param string $query Query to execute
* @param string $message Explanation of query (default '')
*
* @return mysqli_result Query result handler
*
* @deprecated 11.0.0
*/
public function doQueryOrDie($query, $message = '')
{
$res = $this->doQuery($query);
if (!$res) {
//TRANS: %1$s is the description, %2$s is the query, %3$s is the error message
$message = sprintf(
__('%1$s - Error during the database query: %2$s - Error is %3$s'),
$message,
$query,
$this->error()
);
if (isCommandLine()) {
throw new \RuntimeException($message);
} else {
echo $message . "\n";
die(1);
}
}
return $res;
Toolbox::deprecated('Use `DBmysql::doQuery()`.');

return $this->doQuery($query);
}

/**
* Prepare a MySQL query
*
* @param string $query Query to prepare
*
* @return mysqli_stmt|boolean statement object or FALSE if an error occurred.
* @return mysqli_stmt
*/
public function prepare($query)
{
$res = $this->dbh->prepare($query);
if (!$res) {
// no translation for error logs
$error = " *** MySQL prepare error:\n SQL: " . $query . "\n Error: " .
$this->dbh->error . "\n";
$error .= Toolbox::backtrace(false, 'DBmysql->prepare()', ['Toolbox::backtrace()']);

Toolbox::logSqlError($error);

ErrorHandler::getInstance()->handleSqlError($this->dbh->errno, $this->dbh->error, $query);

if (isset($_SESSION['glpi_use_mode']) && $_SESSION['glpi_use_mode'] == Session::DEBUG_MODE) {
\Glpi\Debug\Profile::getCurrent()->addSQLQueryData(
query: $query,
time: 0,
errors: $this->error()
);
}
throw new \RuntimeException(
sprintf(
'MySQL prepare error: %s (%d) in SQL query "%s".',
$this->dbh->error,
$this->dbh->errno,
$query
)
);
}
$this->current_query = $query;
return $res;
Expand Down Expand Up @@ -1358,8 +1335,7 @@ public function insert($table, $params)
}

/**
* Insert a row in the database and die
* (optionnaly with a message) if it fails
* Insert a row in the database and throw an exception if it fails.
*
* @since 9.3
*
Expand All @@ -1368,26 +1344,14 @@ public function insert($table, $params)
* @param string $message Explanation of query (default '')
*
* @return mysqli_result|boolean Query result handler
*
* @deprecated 11.0.0
*/
public function insertOrDie($table, $params, $message = '')
{
$res = $this->insert($table, $params);
if (!$res) {
//TRANS: %1$s is the description, %2$s is the query, %3$s is the error message
$message = sprintf(
__('%1$s - Error during the database query: %2$s - Error is %3$s'),
$message,
$this->current_query,
$this->error()
);
if (isCommandLine()) {
throw new \RuntimeException($message);
} else {
echo $message . "\n";
die(1);
}
}
return $res;
Toolbox::deprecated('Use `DBmysql::insert()`.');

return $this->insert($table, $params);
}

/**
Expand Down Expand Up @@ -1488,8 +1452,7 @@ public function update($table, $params, $where, array $joins = [])
}

/**
* Update a row in the database or die
* (optionnaly with a message) if it fails
* Update a row in the database and throw an exception if it fails.
*
* @since 9.3
*
Expand All @@ -1501,26 +1464,14 @@ public function update($table, $params, $where, array $joins = [])
*
* @since 9.4.0 $joins parameter added
* @return mysqli_result|boolean Query result handler
*
* @deprecated 11.0.0
*/
public function updateOrDie($table, $params, $where, $message = '', array $joins = [])
{
$res = $this->update($table, $params, $where, $joins);
if (!$res) {
//TRANS: %1$s is the description, %2$s is the query, %3$s is the error message
$message = sprintf(
__('%1$s - Error during the database query: %2$s - Error is %3$s'),
$message,
$this->current_query,
$this->error()
);
if (isCommandLine()) {
throw new \RuntimeException($message);
} else {
echo $message . "\n";
die(1);
}
}
return $res;
Toolbox::deprecated('Use `DBmysql::update()`.');

return $this->update($table, $params, $where, $joins);
}

/**
Expand All @@ -1537,13 +1488,8 @@ public function updateOrDie($table, $params, $where, $message = '', array $joins
*/
public function updateOrInsert($table, $params, $where, $onlyone = true)
{
try {
$query = $this->buildUpdateOrInsert($table, $params, $where, $onlyone);
return $this->doQueryOrDie($query, 'Unable to create new element or update existing one');
} catch (\RuntimeException $e) {
trigger_error($e->getMessage(), E_USER_WARNING);
return false;
}
$query = $this->buildUpdateOrInsert($table, $params, $where, $onlyone);
return $this->doQuery($query);
}

public function buildUpdateOrInsert($table, $params, $where, $onlyone = true): string
Expand Down Expand Up @@ -1607,8 +1553,7 @@ public function delete($table, $where, array $joins = [])
}

/**
* Delete a row in the database and die
* (optionnaly with a message) if it fails
* Delete a row in the database and throw an exception if it fails.
*
* @since 9.3
*
Expand All @@ -1619,26 +1564,14 @@ public function delete($table, $where, array $joins = [])
*
* @since 9.4.0 $joins parameter added
* @return mysqli_result|boolean Query result handler
*
* @deprecated 11.0.0
*/
public function deleteOrDie($table, $where, $message = '', array $joins = [])
{
$res = $this->delete($table, $where, $joins);
if (!$res) {
//TRANS: %1$s is the description, %2$s is the query, %3$s is the error message
$message = sprintf(
__('%1$s - Error during the database query: %2$s - Error is %3$s'),
$message,
$this->current_query,
$this->error()
);
if (isCommandLine()) {
throw new \RuntimeException($message);
} else {
echo $message . "\n";
die(1);
}
}
return $res;
Toolbox::deprecated('Use `DBmysql::delete()`.');

return $this->delete($table, $where, $joins);
}


Expand Down Expand Up @@ -2087,9 +2020,10 @@ public function executeStatement(mysqli_stmt $stmt): void
if (!$stmt->execute()) {
throw new \RuntimeException(
sprintf(
'Error executing statement "%s": %s',
$this->current_query,
$stmt->error
'MySQL statement error: %s (%d) in SQL query "%s".',
$stmt->error,
$stmt->errno,
$this->current_query
)
);
}
Expand Down
20 changes: 0 additions & 20 deletions src/Glpi/Application/ErrorHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,26 +313,6 @@ public function handleTwigError(Error $error): void
$this->outputDebugMessage($error_type, $error_description, $log_level);
}

/**
* SQL error handler.
*
* This handler is manually called by application when a SQL error occurred.
*
* @param integer $error_code
* @param string $error_message
* @param string $query
*
* @return void
*/
public function handleSqlError(int $error_code, string $error_message, string $query)
{
$this->outputDebugMessage(
sprintf('SQL Error "%s"', $error_code),
sprintf('%s in query "%s"', $error_message, preg_replace('/\\n/', ' ', $query)),
self::ERROR_LEVEL_MAP[E_USER_ERROR]
);
}

/**
* SQL warnings handler.
*
Expand Down
Loading

0 comments on commit 1c82c4a

Please sign in to comment.