From c0bdbd9d81eb22ded95cd7ea9b26a0c36cfa1be0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= <jfd@butonic.de>
Date: Mon, 10 Jun 2013 12:56:45 +0200
Subject: [PATCH] introduce and use executeAudited in db.php
---
lib/db.php | 150 ++++++++++++++++++++++++++++++++---------------
lib/template.php | 21 +++++++
2 files changed, 124 insertions(+), 47 deletions(-)
diff --git a/lib/db.php b/lib/db.php
index 61836551833..ccbfcf3ab53 100644
--- a/lib/db.php
+++ b/lib/db.php
@@ -23,7 +23,8 @@
class DatabaseException extends Exception{
private $query;
- public function __construct($message, $query){
+ //FIXME getQuery seems to be unused, maybe use parent constructor with $message, $code and $previous
+ public function __construct($message, $query = null){
parent::__construct($message);
$this->query = $query;
}
@@ -391,10 +392,63 @@ class OC_DB {
return $result;
}
+ /**
+ * @brief execute a prepared statement, on error write log and throw exception
+ * @param mixed $stmt PDOStatementWrapper | MDB2_Statement_Common ,
+ * an array with 'sql' and optionally 'limit' and 'offset' keys
+ * .. or a simple sql query string
+ * @param array $parameters
+ * @return result
+ * @throws DatabaseException
+ */
+ static public function executeAudited( $stmt, array $parameters = null) {
+ if (is_string($stmt)) {
+ // convert to an array with 'sql'
+ if (stripos($stmt,'LIMIT') !== false) { //OFFSET requires LIMIT, se we only neet to check for LIMIT
+ // TODO try to convert LIMIT OFFSET notation to parameters, see fixLimitClauseForMSSQL
+ $message = 'LIMIT and OFFSET are forbidden for portability reasons,'
+ . ' pass an array with \'limit\' and \'offset\' instead';
+ \OCP\Util::writeLog('db', $message, \OCP\Util::FATAL);
+ throw new DatabaseException($message);
+ }
+ $stmt = array('sql' => $stmt, 'limit' => null, 'offset' => null);
+ }
+ if (is_array($stmt)){
+ // convert to prepared statement
+ if ( ! array_key_exists('sql', $stmt) ) {
+ $message = 'statement array must at least contain key \'sql\'';
+ \OCP\Util::writeLog('db', $message, \OCP\Util::FATAL);
+ throw new DatabaseException($message);
+ }
+ if ( ! array_key_exists('limit', $stmt) ) {
+ $stmt['limit'] = null;
+ }
+ if ( ! array_key_exists('limit', $stmt) ) {
+ $stmt['offset'] = null;
+ }
+ $stmt = self::prepare($stmt['sql'], $stmt['limit'], $stmt['offset']);
+ }
+ self::raiseExceptionOnError($stmt, 'Could not prepare statement');
+ if ($stmt instanceof PDOStatementWrapper || $stmt instanceof MDB2_Statement_Common) {
+ $result = $stmt->execute($parameters);
+ self::raiseExceptionOnError($result, 'Could not execute statement');
+ } else {
+ if (is_object($stmt)) {
+ $message = 'Expected a prepared statement or array got ' . get_class($stmt);
+ } else {
+ $message = 'Expected a prepared statement or array got ' . gettype($stmt);
+ }
+ \OCP\Util::writeLog('db', $message, \OCP\Util::FATAL);
+ throw new DatabaseException($message);
+ }
+ return $result;
+ }
+
/**
* @brief gets last value of autoincrement
* @param string $table The optional table name (will replace *PREFIX*) and add sequence suffix
* @return int id
+ * @throws DatabaseException
*
* MDB2 lastInsertID()
*
@@ -404,25 +458,27 @@ class OC_DB {
public static function insertid($table=null) {
self::connect();
$type = OC_Config::getValue( "dbtype", "sqlite" );
- if( $type == 'pgsql' ) {
- $query = self::prepare('SELECT lastval() AS id');
- $row = $query->execute()->fetchRow();
+ if( $type === 'pgsql' ) {
+ $result = self::executeAudited('SELECT lastval() AS id');
+ $row = $result->fetchRow();
+ self::raiseExceptionOnError($row, 'fetching row for insertid failed');
return $row['id'];
- }
- if( $type == 'mssql' ) {
+ } else if( $type === 'mssql') {
if($table !== null) {
$prefix = OC_Config::getValue( "dbtableprefix", "oc_" );
$table = str_replace( '*PREFIX*', $prefix, $table );
}
- return self::$connection->lastInsertId($table);
- }else{
+ $result = self::$connection->lastInsertId($table);
+ } else {
if($table !== null) {
$prefix = OC_Config::getValue( "dbtableprefix", "oc_" );
$suffix = OC_Config::getValue( "dbsequencesuffix", "_id_seq" );
$table = str_replace( '*PREFIX*', $prefix, $table ).$suffix;
}
- return self::$connection->lastInsertId($table);
+ $result = self::$connection->lastInsertId($table);
}
+ self::raiseExceptionOnError($result, 'insertid failed');
+ return $result;
}
/**
@@ -512,6 +568,8 @@ class OC_DB {
//clean up memory
unlink( $file2 );
+
+ self::raiseExceptionOnError($definition,'Failed to parse the database definition');
// Die in case something went wrong
if( $definition instanceof MDB2_Schema_Error ) {
@@ -528,11 +586,7 @@ class OC_DB {
$ret=self::$schema->createDatabase( $definition );
- // Die in case something went wrong
- if( $ret instanceof MDB2_Error ) {
- OC_Template::printErrorPage( self::$MDB2->getDebugOutput().' '.$ret->getMessage() . ': '
- . $ret->getUserInfo() );
- }
+ self::raiseExceptionOnError($ret,'Failed to create the database structure');
return true;
}
@@ -552,13 +606,7 @@ class OC_DB {
$content = file_get_contents( $file );
$previousSchema = self::$schema->getDefinitionFromDatabase();
- if (PEAR::isError($previousSchema)) {
- $error = $previousSchema->getMessage();
- $detail = $previousSchema->getDebugInfo();
- $message = 'Failed to get existing database structure for updating ('.$error.', '.$detail.')';
- OC_Log::write('core', $message, OC_Log::FATAL);
- throw new Exception($message);
- }
+ self::raiseExceptionOnError($previousSchema,'Failed to get existing database structure for updating');
// Make changes and save them to an in-memory file
$file2 = 'static://db_scheme';
@@ -582,13 +630,7 @@ class OC_DB {
//clean up memory
unlink( $file2 );
- if (PEAR::isError($op)) {
- $error = $op->getMessage();
- $detail = $op->getDebugInfo();
- $message = 'Failed to update database structure ('.$error.', '.$detail.')';
- OC_Log::write('core', $message, OC_Log::FATAL);
- throw new Exception($message);
- }
+ self::raiseExceptionOnError($op,'Failed to update database structure');
return true;
}
@@ -641,15 +683,9 @@ class OC_DB {
}
$query = substr($query, 0, strlen($query) - 5);
try {
- $stmt = self::prepare($query);
- $result = $stmt->execute($inserts);
-
- } catch(PDOException $e) {
- $entry = 'DB Error: "'.$e->getMessage() . '"<br />';
- $entry .= 'Offending command was: ' . $query . '<br />';
- OC_Log::write('core', $entry, OC_Log::FATAL);
- error_log('DB error: '.$entry);
- OC_Template::printErrorPage( $entry );
+ $result = self::executeAudited($query, $inserts);
+ } catch(DatabaseException $e) {
+ OC_Template::printExceptionErrorPage( $e );
}
if((int)$result->numRows() === 0) {
@@ -674,16 +710,12 @@ class OC_DB {
}
try {
- $result = self::prepare($query);
+ $result = self::executeAudited($query, $inserts);
} catch(PDOException $e) {
- $entry = 'DB Error: "'.$e->getMessage() . '"<br />';
- $entry .= 'Offending command was: ' . $query.'<br />';
- OC_Log::write('core', $entry, OC_Log::FATAL);
- error_log('DB error: ' . $entry);
- OC_Template::printErrorPage( $entry );
+ OC_Template::printExceptionErrorPage( $e );
}
- return $result->execute($inserts);
+ return $result;
}
/**
@@ -891,7 +923,33 @@ class OC_DB {
return false;
}
}
+ /**
+ * check if a result is an error, writes a log entry and throws an exception, works with MDB2 and PDOException
+ * @param mixed $result
+ * @param string message
+ * @return void
+ * @throws DatabaseException
+ */
+ public static function raiseExceptionOnError($result, $message = null) {
+ if(self::isError($result)) {
+ if ($message === null) {
+ $message = self::getErrorMessage($result);
+ } else {
+ $message .= ', Root cause:' . self::getErrorMessage($result);
+ }
+ OC_Log::write('db', $message, OC_Log::FATAL);
+ throw new DatabaseException($message, getErrorCode($result));
+ }
+ }
+ public static function getErrorCode($error) {
+ if ( self::$backend==self::BACKEND_MDB2 and PEAR::isError($error) ) {
+ $code = $error->getCode();
+ } elseif ( self::$backend==self::BACKEND_PDO and self::$PDO ) {
+ $code = self::$PDO->errorCode();
+ }
+ return $code;
+ }
/**
* returns the error code and message as a string for logging
* works with MDB2 and PDOException
@@ -901,9 +959,7 @@ class OC_DB {
public static function getErrorMessage($error) {
if ( self::$backend==self::BACKEND_MDB2 and PEAR::isError($error) ) {
$msg = $error->getCode() . ': ' . $error->getMessage();
- if (defined('DEBUG') && DEBUG) {
- $msg .= '(' . $error->getDebugInfo() . ')';
- }
+ $msg .= ' (' . $error->getDebugInfo() . ')';
} elseif (self::$backend==self::BACKEND_PDO and self::$PDO) {
$msg = self::$PDO->errorCode() . ': ';
$errorInfo = self::$PDO->errorInfo();
diff --git a/lib/template.php b/lib/template.php
index 9467dedb62a..01f0fc28b60 100644
--- a/lib/template.php
+++ b/lib/template.php
@@ -535,4 +535,25 @@ class OC_Template{
$content->printPage();
die();
}
+
+ /**
+ * print error page using Exception details
+ * @param Exception $exception
+ */
+
+ public static function printExceptionErrorPage(Exception $exception) {
+ $error_msg = $exception->getMessage();
+ if ($exception->getCode()) {
+ $error_msg = '['.$exception->getCode().'] '.$error_msg;
+ }
+ $hint = $exception->getTraceAsString();
+ while ($exception = $exception->previous()) {
+ $error_msg .= '<br/>Caused by: ';
+ if ($exception->getCode()) {
+ $error_msg .= '['.$exception->getCode().'] ';
+ }
+ $error_msg .= $exception->getMessage();
+ };
+ self::printErrorPage($error_msg, $hint);
+ }
}
--
GitLab