From aaa8f6d3117edb8cd389c24ed958e6b0febf69f1 Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Wed, 22 Sep 2021 14:38:23 +0200 Subject: [PATCH 1/5] =?UTF-8?q?N=C2=B04215=20Fix=20call=20to=20a=20functio?= =?UTF-8?q?n=20on=20null=20error=20when=20setting=20TLS=20connection=20in?= =?UTF-8?q?=20the=20setup=20Regression=20introduced=20by=20b1ca1f2630ef919?= =?UTF-8?q?024ea630e57d9d4a8a3406aea=20/=20N=C2=B03513?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/cmdbsource.class.inc.php | 10 ++++++++-- test/core/CMDBSource/CMDBSourceTest.php | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index b82c665a0..2038e848f 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -154,6 +154,12 @@ class CMDBSource /** @var mysqli $m_oMysqli */ protected static $m_oMysqli; + /** + * @var mysqli $oMySQLiForQuery + * @see GetMySQLiForQuery + * @see SetMySQLiForQuery + * @since 2.7.5 N°3513 new var to allow mock in tests ({@see \Combodo\iTop\Test\UnitTest\Core\TransactionsTest}) + */ protected static $oMySQLiForQuery; /** @@ -348,9 +354,9 @@ class CMDBSource */ private static function IsOpenedDbConnectionUsingTls($oMysqli) { - if (self::$m_oMysqli == null) + if (is_null(self::GetMySQLiForQuery())) { - self::$m_oMysqli = $oMysqli; + self::SetMySQLiForQuery($oMysqli); } $bNonEmptySslVersionVar = self::IsMySqlVarNonEmpty('ssl_version'); diff --git a/test/core/CMDBSource/CMDBSourceTest.php b/test/core/CMDBSource/CMDBSourceTest.php index a787c0316..7f118f6ef 100644 --- a/test/core/CMDBSource/CMDBSourceTest.php +++ b/test/core/CMDBSource/CMDBSourceTest.php @@ -5,6 +5,7 @@ namespace Combodo\iTop\Test\UnitTest\Core; use CMDBSource; use Combodo\iTop\Test\UnitTest\ItopTestCase; +use utils; /** * @since 2.7.0 @@ -115,4 +116,23 @@ class CMDBSourceTest extends ItopTestCase ), ); } + + /** + * @throws \ConfigException + * @throws \CoreException + * @throws \MySQLException + * @since 3.0.0 N°4215 + */ + public function testIsOpenedDbConnectionUsingTls() { + $oConfig = utils::GetConfig(); + CMDBSource::InitFromConfig($oConfig); + $oMysqli = CMDBSource::GetMysqli(); + + // resets \CMDBSource::$oMySQLiForQuery to simulate call to \CMDBSource::Init with a LTS connexion + $this->InvokeNonPublicStaticMethod(CMDBSource::class, 'SetMySQLiForQuery',[null]); + + // before N°4215 fix, this was crashing : "Call to a member function query() on null" + $bIsTlsCnx = $this->InvokeNonPublicStaticMethod(CMDBSource::class, 'IsOpenedDbConnectionUsingTls',[$oMysqli]); + $this->assertFalse($bIsTlsCnx); + } } From cfdbc8ae625d6c4a9ee2e078b295c9b66ae5a753 Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Thu, 23 Sep 2021 11:59:34 +0200 Subject: [PATCH 2/5] =?UTF-8?q?N=C2=B04215=20When=20checking=20for=20TLS?= =?UTF-8?q?=20cnx,=20don't=20set=20anymore=20CMDBSource=20mysql=20attribut?= =?UTF-8?q?es=20!?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/cmdbsource.class.inc.php | 36 ++++++++++++++----------- test/core/CMDBSource/CMDBSourceTest.php | 2 +- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index 2038e848f..ba1bb5f61 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -241,6 +241,8 @@ class CMDBSource * * @return \mysqli * @throws \MySQLException + * + * @uses IsOpenedDbConnectionUsingTls when asking for a TLS connection, to check if it was really opened using TLS */ public static function GetMysqliInstance( $sDbHost, $sUser, $sPwd, $sSource = '', $bTlsEnabled = false, $sTlsCa = null, $bCheckTlsAfterConnection = false @@ -343,41 +345,41 @@ class CMDBSource * parameters were used.
* This method can be called to ensure that the DB connection really uses TLS. * - *

We're using this object connection : {@link self::$m_oMysqli} + *

We're using our own mysqli instance to do the check as this check is done when creating the mysqli instance : the consumer + * might want a dedicated object, and if so we don't want to overwrite the one saved in CMDBSource !
+ * This is the case for example with {@see \iTopMutex} ! * * @param \mysqli $oMysqli * - * @return boolean true if the connection was really established using TLS + * @return boolean true if the connection was really established using TLS, false otherwise * @throws \MySQLException * + * @used-by GetMysqliInstance * @uses IsMySqlVarNonEmpty + * @uses 'ssl_version' MySQL var + * @uses 'ssl_cipher' MySQL var */ private static function IsOpenedDbConnectionUsingTls($oMysqli) { - if (is_null(self::GetMySQLiForQuery())) - { - self::SetMySQLiForQuery($oMysqli); - } - - $bNonEmptySslVersionVar = self::IsMySqlVarNonEmpty('ssl_version'); - $bNonEmptySslCipherVar = self::IsMySqlVarNonEmpty('ssl_cipher'); + $bNonEmptySslVersionVar = self::IsMySqlVarNonEmpty('ssl_version', $oMysqli); + $bNonEmptySslCipherVar = self::IsMySqlVarNonEmpty('ssl_cipher', $oMysqli); return ($bNonEmptySslVersionVar && $bNonEmptySslCipherVar); } /** * @param string $sVarName + * @param mysqli $oMysqli connection to use for the query * * @return bool * @throws \MySQLException - * - * @uses SHOW STATUS queries + * @uses 'SHOW SESSION STATUS' queries */ - private static function IsMySqlVarNonEmpty($sVarName) + private static function IsMySqlVarNonEmpty($sVarName, $oMysqli) { try { - $sResult = self::QueryToScalar("SHOW SESSION STATUS LIKE '$sVarName'", 1); + $sResult = self::QueryToScalar("SHOW SESSION STATUS LIKE '$sVarName'", 1, $oMysqli); } catch (MySQLQueryHasNoResultException $e) { @@ -973,17 +975,21 @@ class CMDBSource /** * @param string $sSql * @param int $iCol beginning at 0 + * @param mysqli $oMysqli if not null will query using this connection, otherwise will use {@see GetMySQLiForQuery} * * @return string corresponding cell content on the first line * @throws \MySQLException * @throws \MySQLQueryHasNoResultException + * @since 2.7.5-2 2.7.6 3.0.0 N°4215 new optional mysqli param */ - public static function QueryToScalar($sSql, $iCol = 0) + public static function QueryToScalar($sSql, $iCol = 0, $oMysqli = null) { + $oMysqliToQuery = (is_null($oMysqli)) ? self::GetMySQLiForQuery() : $oMysqli; + $oKPI = new ExecutionKPI(); try { - $oResult = self::GetMySQLiForQuery()->query($sSql); + $oResult = $oMysqliToQuery->query($sSql); } catch(mysqli_sql_exception $e) { diff --git a/test/core/CMDBSource/CMDBSourceTest.php b/test/core/CMDBSource/CMDBSourceTest.php index 7f118f6ef..592786b02 100644 --- a/test/core/CMDBSource/CMDBSourceTest.php +++ b/test/core/CMDBSource/CMDBSourceTest.php @@ -128,7 +128,7 @@ class CMDBSourceTest extends ItopTestCase CMDBSource::InitFromConfig($oConfig); $oMysqli = CMDBSource::GetMysqli(); - // resets \CMDBSource::$oMySQLiForQuery to simulate call to \CMDBSource::Init with a LTS connexion + // resets \CMDBSource::$oMySQLiForQuery to simulate call to \CMDBSource::Init with a TLS connexion $this->InvokeNonPublicStaticMethod(CMDBSource::class, 'SetMySQLiForQuery',[null]); // before N°4215 fix, this was crashing : "Call to a member function query() on null" From 88290f9e91463e2b9b748aea58c1c1f0f87e5b3b Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Thu, 23 Sep 2021 13:55:23 +0200 Subject: [PATCH 3/5] =?UTF-8?q?N=C2=B04215=20N=C2=B03513=20Fix=20DB=20erro?= =?UTF-8?q?rs=20fetch=20from=20the=20wrong=20object?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/cmdbsource.class.inc.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index ba1bb5f61..bbfb5b16a 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -717,7 +717,7 @@ class CMDBSource { $aContext = array('query' => $sSql); - $iMySqlErrorNo = self::$m_oMysqli->errno; + $iMySqlErrorNo = self::GetMySQLiForQuery()->errno; $aMySqlHasGoneAwayErrorCodes = MySQLHasGoneAwayException::getErrorCodes(); if (in_array($iMySqlErrorNo, $aMySqlHasGoneAwayErrorCodes)) { From 47ed863da929a9b57c322132576382ebb2651d1c Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Thu, 23 Sep 2021 14:32:43 +0200 Subject: [PATCH 4/5] =?UTF-8?q?N=C2=B04215=20N=C2=B03513=20Fix=20DB=20erro?= =?UTF-8?q?rs=20fetch=20from=20the=20wrong=20object=20n=C2=B02?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/cmdbsource.class.inc.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index bbfb5b16a..9e3f6256a 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -739,7 +739,7 @@ class CMDBSource private static function LogDeadLock(Exception $e) { // checks MySQL error code - $iMySqlErrorNo = self::$m_oMysqli->errno; + $iMySqlErrorNo = self::GetMySQLiForQuery()->errno; if (!in_array($iMySqlErrorNo, array(self::MYSQL_ERRNO_WAIT_TIMEOUT, self::MYSQL_ERRNO_DEADLOCK))) { return; @@ -747,7 +747,7 @@ class CMDBSource // Get error info $sUser = UserRights::GetUser(); - $oError = self::$m_oMysqli->query('SHOW ENGINE INNODB STATUS'); + $oError = self::GetMySQLiForQuery()->query('SHOW ENGINE INNODB STATUS'); if ($oError !== false) { $aData = $oError->fetch_all(MYSQLI_ASSOC); From ec1dcc8df6d6f72d5ff7db46097306c07c31e674 Mon Sep 17 00:00:00 2001 From: Pierre Goiffon Date: Thu, 23 Sep 2021 14:42:16 +0200 Subject: [PATCH 5/5] =?UTF-8?q?:bulb:=20N=C2=B03513=20PHPDoc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/cmdbsource.class.inc.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/cmdbsource.class.inc.php b/core/cmdbsource.class.inc.php index 9e3f6256a..cfb0cbd9e 100644 --- a/core/cmdbsource.class.inc.php +++ b/core/cmdbsource.class.inc.php @@ -155,6 +155,10 @@ class CMDBSource /** @var mysqli $m_oMysqli */ protected static $m_oMysqli; /** + * The mysqli object is really hard to mock ! This attribute is used only in certain methods, so that we can mock only a very little subset of the mysqli object. + * We are setting it in {@see Init}, by default it is a copy of {@see $m_oMysqli} + * The mock can be injected using the setter {@see SetMySQLiForQuery} + * * @var mysqli $oMySQLiForQuery * @see GetMySQLiForQuery * @see SetMySQLiForQuery