From f25b88b14690fd0dbdba172b211b5d046327f472 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Wed, 14 Sep 2016 22:15:19 +0100 Subject: [PATCH 1/4] showqueries debugging tool now inserts parameters in place (#5885) --- .../07_Debugging/02_URL_Variable_Tools.md | 8 +-- model/DB.php | 56 ++++++++++++++++++- model/connect/Database.php | 10 +++- model/queries/SQLQuery.php | 30 +--------- 4 files changed, 68 insertions(+), 36 deletions(-) diff --git a/docs/en/02_Developer_Guides/07_Debugging/02_URL_Variable_Tools.md b/docs/en/02_Developer_Guides/07_Debugging/02_URL_Variable_Tools.md index 7ae530fb7..6adaa181d 100644 --- a/docs/en/02_Developer_Guides/07_Debugging/02_URL_Variable_Tools.md +++ b/docs/en/02_Developer_Guides/07_Debugging/02_URL_Variable_Tools.md @@ -38,10 +38,10 @@ Append the option and corresponding value to your URL in your browser's address ## Database - | URL Variable | | Values | | Description | - | ------------ | | ------ | | ----------- | - | showqueries | | 1 | | List all SQL queries executed | - | previewwrite | | 1 | | List all insert / update SQL queries, and **don't** execute them. Useful for previewing writes to the database. | + | URL Variable | | Values | | Description | + | ------------ | | --------- | | ----------- | + | showqueries | | 1\|inline | | List all SQL queries executed, the `inline` option will do a fudge replacement of parameterised queries | + | previewwrite | | 1 | | List all insert / update SQL queries, and **don't** execute them. Useful for previewing writes to the database. | ## Security Redirects diff --git a/model/DB.php b/model/DB.php index 11128ad17..998008fe2 100644 --- a/model/DB.php +++ b/model/DB.php @@ -282,7 +282,7 @@ class DB { * * @param array|integer $input An array of items needing placeholders, or a * number to specify the number of placeholders - * @param string The string to join each placeholder together with + * @param string $join The string to join each placeholder together with * @return string|null Either a list of placeholders, or null */ public static function placeholders($input, $join = ', ') { @@ -297,6 +297,60 @@ class DB { return implode($join, array_fill(0, $number, '?')); } + /** + * @param string $sql The parameterised query + * @param array $parameters The parameters to inject into the query + * + * @return string + */ + public static function inline_parameters($sql, $parameters) { + $segments = preg_split('/\?/', $sql); + $joined = ''; + $inString = false; + $numSegments = count($segments); + for($i = 0; $i < $numSegments; $i++) { + $input = $segments[$i]; + // Append next segment + $joined .= $segments[$i]; + // Don't add placeholder after last segment + if($i === $numSegments - 1) { + break; + } + // check string escape on previous fragment + // Remove escaped backslashes, count them! + $input = preg_replace('/\\\\\\\\/', '', $input); + // Count quotes + $totalQuotes = substr_count($input, "'"); // Includes double quote escaped quotes + $escapedQuotes = substr_count($input, "\\'"); + if((($totalQuotes - $escapedQuotes) % 2) !== 0) { + $inString = !$inString; + } + // Append placeholder replacement + if($inString) { + // Literal question mark + $joined .= '?'; + continue; + } + + // Encode and insert next parameter + $next = array_shift($parameters); + if(is_array($next) && isset($next['value'])) { + $next = $next['value']; + } + if (is_bool($next)) { + $value = $next ? '1' : '0'; + } + elseif (is_int($next)) { + $value = $next; + } + else { + $value = DB::is_active() ? Convert::raw2sql($next, true) : $next; + } + $joined .= $value; + } + return $joined; + } + /** * Execute the given SQL parameterised query with the specified arguments * diff --git a/model/connect/Database.php b/model/connect/Database.php index 45cc36985..455400437 100644 --- a/model/connect/Database.php +++ b/model/connect/Database.php @@ -141,7 +141,8 @@ abstract class SS_Database { $sql, function($sql) use($connector, $parameters, $errorLevel) { return $connector->preparedQuery($sql, $parameters, $errorLevel); - } + }, + $parameters ); } @@ -174,13 +175,18 @@ abstract class SS_Database { * * @param string $sql Query to run, and single parameter to callback * @param callable $callback Callback to execute code + * @param array $parameters Parameters for any parameterised query * @return mixed Result of query */ - protected function benchmarkQuery($sql, $callback) { + protected function benchmarkQuery($sql, $callback, $parameters = array()) { if (isset($_REQUEST['showqueries']) && Director::isDev()) { $starttime = microtime(true); $result = $callback($sql); $endtime = round(microtime(true) - $starttime, 4); + // replace parameters as closely as possible to what we'd expect the DB to put in + if (strtolower($_REQUEST['showqueries']) == 'inline') { + $sql = DB::inline_parameters($sql, $parameters); + } Debug::message("\n$sql\n{$endtime}s\n", false); return $result; } else { diff --git a/model/queries/SQLQuery.php b/model/queries/SQLQuery.php index 1ce4a835a..6ed537a9d 100644 --- a/model/queries/SQLQuery.php +++ b/model/queries/SQLQuery.php @@ -160,35 +160,7 @@ class SQLQuery_ParameterInjector { * @return string */ protected function injectValues($sql, $parameters) { - $segments = preg_split('/\?/', $sql); - $joined = ''; - $inString = false; - for($i = 0; $i < count($segments); $i++) { - // Append next segment - $joined .= $segments[$i]; - // Don't add placeholder after last segment - if($i === count($segments) - 1) { - break; - } - // check string escape on previous fragment - if($this->checkStringTogglesLiteral($segments[$i])) { - $inString = !$inString; - } - // Append placeholder replacement - if($inString) { - // Literal questionmark - $joined .= '?'; - continue; - } - - // Encode and insert next parameter - $next = array_shift($parameters); - if(is_array($next) && isset($next['value'])) { - $next = $next['value']; - } - $joined .= "'".Convert::raw2sql($next)."'"; - } - return $joined; + return DB::inline_parameters($sql, $parameters); } /** From cd8904e0454617b243c8e89c06c694844817f212 Mon Sep 17 00:00:00 2001 From: 3Dgoo Date: Thu, 15 Sep 2016 08:23:22 +0930 Subject: [PATCH 2/4] Fixing button destroy bug When you want to add a button to the CMS but don't want LeftAndMain to apply jQuery UI button to it we add the `data-button="true"` attribute to our button. The `onadd` function checks that this attribute does not exist before calling `this.button()`. The `onremove` incorrectly checks that this attribute does exist before calling `this.button('destroy')`. This should be checking that this attribute does not exist, just like the `onadd` function. What this causes is when you have a button with the `data-button` attribute a button is not created, but when you leave the page `destory` gets called on an item which doesn't exist. We end up with the following error: > Uncaught Error: cannot call methods on button prior to initialization; attempted to call method 'destroy' The other issue with this logic is buttons are never getting destroyed when `onremove` is called. The whole issue is caused by a missing `!` in the if statement. This change adds it in to fix the problem. --- admin/javascript/LeftAndMain.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin/javascript/LeftAndMain.js b/admin/javascript/LeftAndMain.js index 487467118..f065eb0d9 100644 --- a/admin/javascript/LeftAndMain.js +++ b/admin/javascript/LeftAndMain.js @@ -1081,7 +1081,7 @@ jQuery.noConflict(); this._super(); }, onremove: function() { - if(this.data('button')) this.button('destroy'); + if(!this.data('button')) this.button('destroy'); this._super(); } }); From 995d07756d0b7aeef611d61fe3a31663b70e4c51 Mon Sep 17 00:00:00 2001 From: Thomas Portelange Date: Thu, 15 Sep 2016 16:45:40 +0200 Subject: [PATCH 3/4] cache currentUser query (#6007) * cache currentUser query Various modules can call a lot of time Member::currentUser(). We can avoid querying the database multiple times. Cache is implemented as a static array inside the method and store the data byID, in case the currentUserID changes within the same request (not very likely, but..) --- security/Member.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/security/Member.php b/security/Member.php index 88b1cccf7..47e27395c 100644 --- a/security/Member.php +++ b/security/Member.php @@ -837,8 +837,9 @@ class Member extends DataObject implements TemplateGlobalProvider { $id = Member::currentUserID(); if($id) { - return Member::get()->byId($id); + return DataObject::get_by_id('Member', $id); } + return null; } /** From b87c668bf4184abb4d7348e77a63853038ad2de2 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 16 Sep 2016 11:39:29 +1200 Subject: [PATCH 4/4] API support dblib (#5996) --- model/connect/PDOConnector.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/model/connect/PDOConnector.php b/model/connect/PDOConnector.php index 6b423754e..f1007b2ca 100644 --- a/model/connect/PDOConnector.php +++ b/model/connect/PDOConnector.php @@ -123,6 +123,12 @@ class PDOConnector extends DBConnector { $server .= ",{$parameters['port']}"; } $dsn[] = "Server=$server"; + } elseif ($parameters['driver'] === 'dblib') { + $server = $parameters['server']; + if (!empty($parameters['port'])) { + $server .= ":{$parameters['port']}"; + } + $dsn[] = "host={$server}"; } else { if (!empty($parameters['server'])) { // Use Server instead of host for sqlsrv