From 383e4c62b578996b343b540e03c7afed61be644e Mon Sep 17 00:00:00 2001 From: Thomas Mueller Date: Mon, 17 Jun 2013 00:02:42 +0200 Subject: [PATCH 01/15] in case $_SERVER['HTTP_HOST']) is not set let's return localhost - better than nothing --- lib/request.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/request.php b/lib/request.php index 4d8380eb9ac..aa5f53c08eb 100755 --- a/lib/request.php +++ b/lib/request.php @@ -19,7 +19,7 @@ class OC_Request { /** * @brief Returns the server host - * @returns the server host + * @returns string the server host * * Returns the server host, even if the website uses one or more * reverse proxies @@ -40,7 +40,7 @@ class OC_Request { } } else{ - $host = $_SERVER['HTTP_HOST']; + $host = isset($_SERVER['HTTP_HOST']) ? $_SERVER['HTTP_HOST'] : 'localhost'; } return $host; } @@ -48,7 +48,7 @@ class OC_Request { /** * @brief Returns the server protocol - * @returns the server protocol + * @returns string the server protocol * * Returns the server protocol. It respects reverse proxy servers and load balancers */ @@ -70,7 +70,7 @@ class OC_Request { /** * @brief Returns the request uri - * @returns the request uri + * @returns string the request uri * * Returns the request uri, even if the website uses one or more * reverse proxies @@ -85,7 +85,7 @@ class OC_Request { /** * @brief Returns the script name - * @returns the script name + * @returns string the script name * * Returns the script name, even if the website uses one or more * reverse proxies @@ -139,7 +139,7 @@ class OC_Request { /** * @brief Check if this is a no-cache request - * @returns true for no-cache + * @returns boolean true for no-cache */ static public function isNoCache() { if (!isset($_SERVER['HTTP_CACHE_CONTROL'])) { @@ -150,7 +150,7 @@ class OC_Request { /** * @brief Check if the requestor understands gzip - * @returns true for gzip encoding supported + * @returns boolean true for gzip encoding supported */ static public function acceptGZip() { if (!isset($_SERVER['HTTP_ACCEPT_ENCODING'])) { From 372f261fe304463a67fc347b0e1c345653332ed3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 10:27:02 +0200 Subject: [PATCH 02/15] remove unnecessary exception handling --- lib/db.php | 137 +++++++++++++++++++++++------------------------------ 1 file changed, 58 insertions(+), 79 deletions(-) diff --git a/lib/db.php b/lib/db.php index a6b81aaba69..66900f46be9 100644 --- a/lib/db.php +++ b/lib/db.php @@ -180,28 +180,18 @@ class OC_DB { $dsn = 'oci:dbname=//' . $host . '/' . $name; } break; - case 'mssql': + case 'mssql': if ($port) { $dsn='sqlsrv:Server='.$host.','.$port.';Database='.$name; } else { $dsn='sqlsrv:Server='.$host.';Database='.$name; } - break; + break; default: return false; } - try{ - self::$PDO=new PDO($dsn, $user, $pass, $opts); - }catch(PDOException $e) { - OC_Log::write('core', $e->getMessage(), OC_Log::FATAL); - OC_User::setUserId(null); - - // send http status 503 - header('HTTP/1.1 503 Service Temporarily Unavailable'); - header('Status: 503 Service Temporarily Unavailable'); - OC_Template::printErrorPage('Failed to connect to database'); - die(); - } + self::$PDO=new PDO($dsn, $user, $pass, $opts); + // We always, really always want associative arrays self::$PDO->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_ASSOC); self::$PDO->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); @@ -299,19 +289,8 @@ class OC_DB { // Try to establish connection self::$MDB2 = MDB2::factory( $dsn, $options ); - - // Die if we could not connect - if( PEAR::isError( self::$MDB2 )) { - OC_Log::write('core', self::$MDB2->getUserInfo(), OC_Log::FATAL); - OC_Log::write('core', self::$MDB2->getMessage(), OC_Log::FATAL); - OC_User::setUserId(null); - - // send http status 503 - header('HTTP/1.1 503 Service Temporarily Unavailable'); - header('Status: 503 Service Temporarily Unavailable'); - OC_Template::printErrorPage('Failed to connect to database'); - die(); - } + + self::raiseExceptionOnError( self::$MDB2 ); // We always, really always want associative arrays self::$MDB2->setFetchMode(MDB2_FETCHMODE_ASSOC); @@ -760,9 +739,9 @@ class OC_DB { $query = str_replace( 'now()', 'CURRENT_TIMESTAMP', $query ); $query = str_replace( 'LENGTH(', 'LEN(', $query ); $query = str_replace( 'SUBSTR(', 'SUBSTRING(', $query ); - - $query = self::fixLimitClauseForMSSQL($query); - } + + $query = self::fixLimitClauseForMSSQL($query); + } // replace table name prefix $query = str_replace( '*PREFIX*', $prefix, $query ); @@ -770,60 +749,60 @@ class OC_DB { return $query; } - private static function fixLimitClauseForMSSQL($query) { - $limitLocation = stripos ($query, "LIMIT"); - - if ( $limitLocation === false ) { - return $query; - } - - // total == 0 means all results - not zero results - // - // First number is either total or offset, locate it by first space - // - $offset = substr ($query, $limitLocation + 5); - $offset = substr ($offset, 0, stripos ($offset, ' ')); - $offset = trim ($offset); + private static function fixLimitClauseForMSSQL($query) { + $limitLocation = stripos ($query, "LIMIT"); - // check for another parameter - if (stripos ($offset, ',') === false) { - // no more parameters - $offset = 0; - $total = intval ($offset); - } else { - // found another parameter - $offset = intval ($offset); + if ( $limitLocation === false ) { + return $query; + } - $total = substr ($query, $limitLocation + 5); - $total = substr ($total, stripos ($total, ',')); + // total == 0 means all results - not zero results + // + // First number is either total or offset, locate it by first space + // + $offset = substr ($query, $limitLocation + 5); + $offset = substr ($offset, 0, stripos ($offset, ' ')); + $offset = trim ($offset); - $total = substr ($total, 0, stripos ($total, ' ')); - $total = intval ($total); - } + // check for another parameter + if (stripos ($offset, ',') === false) { + // no more parameters + $offset = 0; + $total = intval ($offset); + } else { + // found another parameter + $offset = intval ($offset); - $query = trim (substr ($query, 0, $limitLocation)); + $total = substr ($query, $limitLocation + 5); + $total = substr ($total, stripos ($total, ',')); - if ($offset == 0 && $total !== 0) { - if (strpos($query, "SELECT") === false) { - $query = "TOP {$total} " . $query; - } else { - $query = preg_replace('/SELECT(\s*DISTINCT)?/Dsi', 'SELECT$1 TOP '.$total, $query); - } - } else if ($offset > 0) { - $query = preg_replace('/SELECT(\s*DISTINCT)?/Dsi', 'SELECT$1 TOP(10000000) ', $query); - $query = 'SELECT * - FROM (SELECT sub2.*, ROW_NUMBER() OVER(ORDER BY sub2.line2) AS line3 - FROM (SELECT 1 AS line2, sub1.* FROM (' . $query . ') AS sub1) as sub2) AS sub3'; + $total = substr ($total, 0, stripos ($total, ' ')); + $total = intval ($total); + } + + $query = trim (substr ($query, 0, $limitLocation)); + + if ($offset == 0 && $total !== 0) { + if (strpos($query, "SELECT") === false) { + $query = "TOP {$total} " . $query; + } else { + $query = preg_replace('/SELECT(\s*DISTINCT)?/Dsi', 'SELECT$1 TOP '.$total, $query); + } + } else if ($offset > 0) { + $query = preg_replace('/SELECT(\s*DISTINCT)?/Dsi', 'SELECT$1 TOP(10000000) ', $query); + $query = 'SELECT * + FROM (SELECT sub2.*, ROW_NUMBER() OVER(ORDER BY sub2.line2) AS line3 + FROM (SELECT 1 AS line2, sub1.* FROM (' . $query . ') AS sub1) as sub2) AS sub3'; + + if ($total > 0) { + $query .= ' WHERE line3 BETWEEN ' . ($offset + 1) . ' AND ' . ($offset + $total); + } else { + $query .= ' WHERE line3 > ' . $offset; + } + } + return $query; + } - if ($total > 0) { - $query .= ' WHERE line3 BETWEEN ' . ($offset + 1) . ' AND ' . ($offset + $total); - } else { - $query .= ' WHERE line3 > ' . $offset; - } - } - return $query; - } - /** * @brief drop a table * @param string $tableName the table to drop @@ -1124,7 +1103,7 @@ class PDOStatementWrapper{ die ($entry); } } - + /** * provide numRows */ From 7b6fcddbc5314e7401e4f5579853aa6353d462f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 18 Jun 2013 18:24:48 +0200 Subject: [PATCH 03/15] use executeAudited, add table name to assert message, skip schema changing test on oracle --- tests/lib/dbschema.php | 87 +++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 43 deletions(-) diff --git a/tests/lib/dbschema.php b/tests/lib/dbschema.php index 59f203993ef..5d52db6a5ab 100644 --- a/tests/lib/dbschema.php +++ b/tests/lib/dbschema.php @@ -7,9 +7,8 @@ */ class Test_DBSchema extends PHPUnit_Framework_TestCase { - protected static $schema_file = 'static://test_db_scheme'; - protected static $schema_file2 = 'static://test_db_scheme2'; - protected $test_prefix; + protected $schema_file = 'static://test_db_scheme'; + protected $schema_file2 = 'static://test_db_scheme2'; protected $table1; protected $table2; @@ -20,19 +19,20 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { $r = '_'.OC_Util::generate_random_bytes('4').'_'; $content = file_get_contents( $dbfile ); $content = str_replace( '*dbprefix*', '*dbprefix*'.$r, $content ); - file_put_contents( self::$schema_file, $content ); + file_put_contents( $this->schema_file, $content ); $content = file_get_contents( $dbfile2 ); $content = str_replace( '*dbprefix*', '*dbprefix*'.$r, $content ); - file_put_contents( self::$schema_file2, $content ); + file_put_contents( $this->schema_file2, $content ); - $this->test_prefix = $r; - $this->table1 = $this->test_prefix.'cntcts_addrsbks'; - $this->table2 = $this->test_prefix.'cntcts_cards'; + $prefix = OC_Config::getValue( "dbtableprefix", "oc_" ); + + $this->table1 = $prefix.$r.'cntcts_addrsbks'; + $this->table2 = $prefix.$r.'cntcts_cards'; } public function tearDown() { - unlink(self::$schema_file); - unlink(self::$schema_file2); + unlink($this->schema_file); + unlink($this->schema_file2); } // everything in one test, they depend on each other @@ -47,13 +47,19 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { } public function doTestSchemaCreating() { - OC_DB::createDbFromStructure(self::$schema_file); + OC_DB::createDbFromStructure($this->schema_file); $this->assertTableExist($this->table1); $this->assertTableExist($this->table2); } public function doTestSchemaChanging() { - OC_DB::updateDbFromStructure(self::$schema_file2); + if (OC_Config::getValue( 'dbtype', 'sqlite' ) === 'oci') { + $this->markTestSkipped( + // see http://abhijitbashetti.blogspot.de/2011/10/converting-varchar2-to-clob-and-clob-to.html + 'Oracle does not simply ALTER a VARCHAR into a CLOB.' + ); + } + OC_DB::updateDbFromStructure($this->schema_file2); $this->assertTableExist($this->table2); } @@ -66,67 +72,62 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { } public function doTestSchemaRemoving() { - OC_DB::removeDBStructure(self::$schema_file); + OC_DB::removeDBStructure($this->schema_file); $this->assertTableNotExist($this->table1); $this->assertTableNotExist($this->table2); } public function tableExist($table) { - $table = '*PREFIX*' . $table; switch (OC_Config::getValue( 'dbtype', 'sqlite' )) { case 'sqlite': case 'sqlite3': $sql = "SELECT name FROM sqlite_master " - . "WHERE type = 'table' AND name != 'sqlite_sequence' " - . "AND name != 'geometry_columns' AND name != 'spatial_ref_sys' " - . "UNION ALL SELECT name FROM sqlite_temp_master " - . "WHERE type = 'table' AND name = '".$table."'"; - $query = OC_DB::prepare($sql); - $result = $query->execute(array()); - $exists = $result && $result->fetchOne(); + . "WHERE type = 'table' AND name = ? " + . "UNION ALL SELECT name FROM sqlite_temp_master " + . "WHERE type = 'table' AND name = ?"; + $result = \OC_DB::executeAudited($sql, array($table, $table)); break; case 'mysql': - $sql = 'SHOW TABLES LIKE "'.$table.'"'; - $query = OC_DB::prepare($sql); - $result = $query->execute(array()); - $exists = $result && $result->fetchOne(); + $sql = 'SHOW TABLES LIKE ?'; + $result = \OC_DB::executeAudited($sql, array($table)); break; case 'pgsql': - $sql = "SELECT tablename AS table_name, schemaname AS schema_name " - . "FROM pg_tables WHERE schemaname NOT LIKE 'pg_%' " - . "AND schemaname != 'information_schema' " - . "AND tablename = '".$table."'"; - $query = OC_DB::prepare($sql); - $result = $query->execute(array()); - $exists = $result && $result->fetchOne(); + $sql = 'SELECT tablename AS table_name, schemaname AS schema_name ' + . 'FROM pg_tables WHERE schemaname NOT LIKE \'pg_%\' ' + . 'AND schemaname != \'information_schema\' ' + . 'AND tablename = ?'; + $result = \OC_DB::executeAudited($sql, array($table)); break; case 'oci': - $sql = 'SELECT table_name FROM user_tables WHERE table_name = ?'; + $sql = 'SELECT TABLE_NAME FROM USER_TABLES WHERE TABLE_NAME = ?'; $result = \OC_DB::executeAudited($sql, array($table)); - $exists = (bool)$result->fetchOne(); //oracle uses MDB2 and returns null break; case 'mssql': - $sql = "SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = '{$table}'"; - $query = OC_DB::prepare($sql); - $result = $query->execute(array()); - $exists = $result && $result->fetchOne(); + $sql = 'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = ?'; + $result = \OC_DB::executeAudited($sql, array($table)); break; } - return $exists; + + $name = $result->fetchOne(); //FIXME checking with '$result->numRows() === 1' does not seem to work? + OC_DB::raiseExceptionOnError($name); + if ($name === $table) { + return true; + } else { + return false; + } } public function assertTableExist($table) { - $this->assertTrue($this->tableExist($table)); + $this->assertTrue($this->tableExist($table), 'Table ' . $table . ' does not exist'); } public function assertTableNotExist($table) { $type=OC_Config::getValue( "dbtype", "sqlite" ); if( $type == 'sqlite' || $type == 'sqlite3' ) { // sqlite removes the tables after closing the DB - } - else { - $this->assertFalse($this->tableExist($table)); + } else { + $this->assertFalse($this->tableExist($table), 'Table ' . $table . ' exists.'); } } } From e62eb2e8d1937f1708ce7efdd6ab8a31e12c6f28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 12:31:23 +0300 Subject: [PATCH 04/15] correctly handle error results of PDO and MDB2 backends --- lib/db.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/db.php b/lib/db.php index 4d6788f2bda..5e624bf30b9 100644 --- a/lib/db.php +++ b/lib/db.php @@ -962,11 +962,14 @@ class OC_DB { * @return bool */ public static function isError($result) { - if(self::$backend==self::BACKEND_PDO and $result === false) { + //PDO returns false on error (and throws an exception) + if (self::$backend===self::BACKEND_PDO and $result === false) { return true; - }elseif(self::$backend==self::BACKEND_MDB2 and PEAR::isError($result)) { + } else + //MDB2 returns an MDB2_Error object + if (self::$backend===self::BACKEND_MDB2 and PEAR::isError($result)) { return true; - }else{ + } else { return false; } } From 4bbdd67a22fd5ac0efc6b8c9dc3e024a0af45c90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 20 Jun 2013 16:58:49 +0200 Subject: [PATCH 05/15] remove wrong check here --- tests/lib/dbschema.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/lib/dbschema.php b/tests/lib/dbschema.php index 5d52db6a5ab..103909e2e25 100644 --- a/tests/lib/dbschema.php +++ b/tests/lib/dbschema.php @@ -110,7 +110,6 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { } $name = $result->fetchOne(); //FIXME checking with '$result->numRows() === 1' does not seem to work? - OC_DB::raiseExceptionOnError($name); if ($name === $table) { return true; } else { From 66e1eaac9379e16bb0c9e60b2794b03bda3b9eec Mon Sep 17 00:00:00 2001 From: Thomas Mueller Date: Thu, 27 Jun 2013 16:48:09 +0200 Subject: [PATCH 06/15] isError should detect a PEAR_Error even if the backend is PDO. This can happen on errors during schema migration - which is always done with MDB2 --- lib/db.php | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/db.php b/lib/db.php index 5e624bf30b9..515563b78e1 100644 --- a/lib/db.php +++ b/lib/db.php @@ -962,21 +962,21 @@ class OC_DB { * @return bool */ public static function isError($result) { + //MDB2 returns an MDB2_Error object + if (class_exists('PEAR') === true && PEAR::isError($result)) { + return true; + } //PDO returns false on error (and throws an exception) if (self::$backend===self::BACKEND_PDO and $result === false) { return true; - } else - //MDB2 returns an MDB2_Error object - if (self::$backend===self::BACKEND_MDB2 and PEAR::isError($result)) { - return true; - } else { - return false; } + + return false; } /** * check if a result is an error and throws an exception, works with MDB2 and PDOException * @param mixed $result - * @param string message + * @param string $message * @return void * @throws DatabaseException */ @@ -992,12 +992,15 @@ class OC_DB { } 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(); + if ( class_exists('PEAR') === true && PEAR::isError($error) ) { + /** @var $error PEAR_Error */ + return $error->getCode(); } - return $code; + if ( self::$backend==self::BACKEND_PDO and self::$PDO ) { + return self::$PDO->errorCode(); + } + + return -1; } /** * returns the error code and message as a string for logging @@ -1006,23 +1009,24 @@ class OC_DB { * @return string */ public static function getErrorMessage($error) { - if ( self::$backend==self::BACKEND_MDB2 and PEAR::isError($error) ) { + if ( class_exists('PEAR') === true && PEAR::isError($error) ) { $msg = $error->getCode() . ': ' . $error->getMessage(); $msg .= ' (' . $error->getDebugInfo() . ')'; - } elseif (self::$backend==self::BACKEND_PDO and self::$PDO) { + + return $msg; + } + if (self::$backend==self::BACKEND_PDO and self::$PDO) { $msg = self::$PDO->errorCode() . ': '; $errorInfo = self::$PDO->errorInfo(); if (is_array($errorInfo)) { $msg .= 'SQLSTATE = '.$errorInfo[0] . ', '; $msg .= 'Driver Code = '.$errorInfo[1] . ', '; $msg .= 'Driver Message = '.$errorInfo[2]; - }else{ - $msg = ''; } - }else{ - $msg = ''; + return $msg; } - return $msg; + + return ''; } /** From c80e76720f4e65a5f7af31f67d70ec66019bcb68 Mon Sep 17 00:00:00 2001 From: Bart Visscher Date: Fri, 28 Jun 2013 16:07:22 +0200 Subject: [PATCH 07/15] Going from text to clob is not something we do. Also Oracle DB has problems with this, see http://abhijitbashetti.blogspot.de/2011/10/converting-varchar2-to-clob-and-clob-to.html --- tests/data/db_structure2.xml | 3 ++- tests/lib/dbschema.php | 6 ------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/data/db_structure2.xml b/tests/data/db_structure2.xml index fc6fe0bba7d..6f12f81f477 100644 --- a/tests/data/db_structure2.xml +++ b/tests/data/db_structure2.xml @@ -49,8 +49,9 @@ description - clob + text false + 1024 diff --git a/tests/lib/dbschema.php b/tests/lib/dbschema.php index 103909e2e25..c2e55eabf4b 100644 --- a/tests/lib/dbschema.php +++ b/tests/lib/dbschema.php @@ -53,12 +53,6 @@ class Test_DBSchema extends PHPUnit_Framework_TestCase { } public function doTestSchemaChanging() { - if (OC_Config::getValue( 'dbtype', 'sqlite' ) === 'oci') { - $this->markTestSkipped( - // see http://abhijitbashetti.blogspot.de/2011/10/converting-varchar2-to-clob-and-clob-to.html - 'Oracle does not simply ALTER a VARCHAR into a CLOB.' - ); - } OC_DB::updateDbFromStructure($this->schema_file2); $this->assertTableExist($this->table2); } From d110e60316f7dd28a2b5876e8911f983fea0883c Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Fri, 28 Jun 2013 21:53:56 +0300 Subject: [PATCH 08/15] Hide a ghost image on the apps management page --- settings/js/apps.js | 5 +++++ settings/templates/apps.php | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/settings/js/apps.js b/settings/js/apps.js index 9c1604cfcd9..bdeddfb84c2 100644 --- a/settings/js/apps.js +++ b/settings/js/apps.js @@ -20,6 +20,11 @@ OC.Settings.Apps = OC.Settings.Apps || { page.find('span.score').html(app.score); page.find('p.description').text(app.description); page.find('img.preview').attr('src', app.preview); + if (app.preview && app.preview.length) { + page.find('img.preview').show(); + } else { + page.find('img.preview').hide(); + } page.find('small.externalapp').attr('style', 'visibility:visible'); page.find('span.author').text(app.author); page.find('span.licence').text(app.licence); diff --git a/settings/templates/apps.php b/settings/templates/apps.php index 0903b9bd5c4..d60fd82f917 100644 --- a/settings/templates/apps.php +++ b/settings/templates/apps.php @@ -34,7 +34,7 @@ class="version">

- +