Merge pull request #209 from envms/fix/backslash-join

Update createUndefinedJoins() regex to accept all valid table names
envms · Oct 5, 2017 · ad319cd · ad319cd
2 parents ae60ac0 + d738e41
commit ad319cd
Show file tree

Hide file tree

Showing 12 changed files with 90 additions and 49 deletions.
diff --git a/FluentPDO/BaseQuery.php b/FluentPDO/BaseQuery.php
@@ -1,6 +1,7 @@
 <?php
 
-/** Base query builder
+/**
+ * Base query builder
  */
 abstract class BaseQuery implements IteratorAggregate
 {
@@ -74,13 +75,14 @@ protected function addStatement($clause, $statement, $parameters = array()) {
         if ($statement === null) {
             return $this->resetClause($clause);
         }
-        // $statement !== null
+
         if ($this->clauses[$clause]) {
             if (is_array($statement)) {
                 $this->statements[$clause] = array_merge($this->statements[$clause], $statement);
             } else {
                 $this->statements[$clause][] = $statement;
             }
+
             $this->parameters[$clause] = array_merge($this->parameters[$clause], $parameters);
         } else {
             $this->statements[$clause] = $statement;
@@ -182,8 +184,13 @@ private function debugger() {
                 $time = sprintf('%0.3f', $this->time * 1000) . ' ms';
                 $rows = ($this->result) ? $this->result->rowCount() : 0;
                 $finalString = "# $backtrace[file]:$backtrace[line] ($time; rows = $rows)\n$debug\n\n";
-                if (is_resource(STDERR)) { // if STDERR is set, send there, otherwise just output the string
-                    fwrite(STDERR, $finalString);
+                if (defined('STDERR')) { // if STDERR is set, send there, otherwise just output the string
+                    if (is_resource(STDERR)) {
+                        fwrite(STDERR, $finalString);
+                    }
+                    else {
+                        echo $finalString;
+                    }
                 }
                 else {
                     echo $finalString;
@@ -323,16 +330,20 @@ protected function quote($value) {
         if (!isset($value)) {
             return "NULL";
         }
+
         if (is_array($value)) { // (a, b) IN ((1, 2), (3, 4))
             return "(" . implode(", ", array_map(array($this, 'quote'), $value)) . ")";
         }
+
         $value = $this->formatValue($value);
         if (is_float($value)) {
             return sprintf("%F", $value); // otherwise depends on setlocale()
         }
+
         if ($value === false) {
             return "0";
         }
+
         if (is_int($value) || $value instanceof FluentLiteral) { // number or SQL code - for example "NOW()"
             return (string)$value;
         }
@@ -347,7 +358,7 @@ protected function quote($value) {
      */
     private function formatValue($val) {
         if ($val instanceof DateTime) {
-            return $val->format("Y-m-d H:i:s"); //! may be driver specific
+            return $val->format("Y-m-d H:i:s"); // may be driver specific
         }
 
         return $val;

diff --git a/FluentPDO/CommonQuery.php b/FluentPDO/CommonQuery.php
@@ -43,32 +43,36 @@ public function isSmartJoinEnabled() {
      * @param string $condition  possibly containing ? or :name (PDO syntax)
      * @param mixed  $parameters array or a scalar value
      *
-     * @return \SelectQuery
+     * @return CommonQuery
      */
     public function where($condition, $parameters = array()) {
         if ($condition === null) {
             return $this->resetClause('WHERE');
         }
+
         if (!$condition) {
             return $this;
         }
+
         if (is_array($condition)) { // where(array("column1" => 1, "column2 > ?" => 2))
             foreach ($condition as $key => $val) {
                 $this->where($key, $val);
             }
 
             return $this;
         }
+
         $args = func_get_args();
+
         if (count($args) == 1) {
             return $this->addStatement('WHERE', $condition);
         }
 
-        // check that there are 2 arguments, a condition and a parameter value
-        // if the condition contains a parameter simply add them
-        // since its up to the user if it's valid sql or not
-        // Otherwise we're probably with just an identifier. So lets
-        // construct a new condition based on the passed parameter value.
+        /*
+        Check that there are 2 arguments, a condition and a parameter value. If the condition contains
+        a parameter, add them; it's up to the dev to be valid sql. Otherwise it's probably
+        just an identifier, so construct a new condition based on the passed parameter value.
+        */
         if (count($args) == 2 && !preg_match('/(\?|:\w+)/i', $condition)) {
             // condition is column only
             if (is_null($parameters)) {
@@ -80,8 +84,10 @@ public function where($condition, $parameters = array()) {
 
                 return $this->addStatement('WHERE', "$condition IN $in");
             }
+
             $condition = "$condition = ?";
         }
+
         array_shift($args);
 
         return $this->addStatement('WHERE', $condition, $args);
@@ -95,6 +101,7 @@ public function where($condition, $parameters = array()) {
      */
     public function __call($clause, $parameters = array()) {
         $clause = FluentUtils::toUpperWords($clause);
+
         if ($clause == 'GROUP') {
             $clause = 'GROUP BY';
         }
@@ -104,7 +111,9 @@ public function __call($clause, $parameters = array()) {
         if ($clause == 'FOOT NOTE') {
             $clause = "\n--";
         }
+
         $statement = array_shift($parameters);
+
         if (strpos($clause, 'JOIN') !== false) {
             return $this->addJoinStatements($clause, $statement, $parameters);
         }
@@ -134,14 +143,16 @@ private function addJoinStatements($clause, $statement, $parameters = array()) {
 
             return $this->resetClause('JOIN');
         }
+
         if (array_search(substr($statement, 0, -1), $this->joins) !== false) {
             return $this;
         }
 
-        // match "tables AS alias"
+        // match "table AS alias"
         preg_match('/`?([a-z_][a-z0-9_\.:]*)`?(\s+AS)?(\s+`?([a-z_][a-z0-9_]*)`?)?/i', $statement, $matches);
         $joinAlias = '';
         $joinTable = '';
+
         if ($matches) {
             $joinTable = $matches[1];
             if (isset($matches[4]) && !in_array(strtoupper($matches[4]), array('ON', 'USING'))) {
@@ -170,22 +181,25 @@ private function addJoinStatements($clause, $statement, $parameters = array()) {
 
         preg_match_all('/([a-z_][a-z0-9_]*[\.:]?)/i', $joinTable, $matches);
         $mainTable = '';
+
         if (isset($this->statements['FROM'])) {
             $mainTable = $this->statements['FROM'];
         } elseif (isset($this->statements['UPDATE'])) {
             $mainTable = $this->statements['UPDATE'];
         }
+
         $lastItem = array_pop($matches[1]);
         array_push($matches[1], $lastItem);
+
         foreach ($matches[1] as $joinItem) {
             if ($mainTable == substr($joinItem, 0, -1)) {
                 continue;
             }
 
-            // use $joinAlias only for $lastItem
             $alias = '';
+
             if ($joinItem == $lastItem) {
-                $alias = $joinAlias;
+                $alias = $joinAlias; // use $joinAlias only for $lastItem
             }
 
             $newJoin = $this->createJoinStatement($clause, $mainTable, $joinItem, $alias);
@@ -212,22 +226,24 @@ private function createJoinStatement($clause, $mainTable, $joinTable, $joinAlias
         if (in_array(substr($mainTable, -1), array(':', '.'))) {
             $mainTable = substr($mainTable, 0, -1);
         }
+
         $referenceDirection = substr($joinTable, -1);
         $joinTable          = substr($joinTable, 0, -1);
         $asJoinAlias        = '';
+
         if ($joinAlias) {
             $asJoinAlias = " AS $joinAlias";
         } else {
             $joinAlias = $joinTable;
         }
-        if (in_array($joinAlias, $this->joins)) {
-            // if join exists don't create same again
+
+        if (in_array($joinAlias, $this->joins)) { // if the join exists don't create it again
             return '';
         } else {
             $this->joins[] = $joinAlias;
         }
-        if ($referenceDirection == ':') {
-            // back reference
+
+        if ($referenceDirection == ':') { // back reference
             $primaryKey = $this->getStructure()->getPrimaryKey($mainTable);
             $foreignKey = $this->getStructure()->getForeignKey($mainTable);
 
@@ -246,6 +262,7 @@ private function createJoinStatement($clause, $mainTable, $joinTable, $joinAlias
     protected function buildQuery() {
         // first create extra join from statements with columns with referenced tables
         $statementsWithReferences = array('WHERE', 'SELECT', 'GROUP BY', 'ORDER BY');
+
         foreach ($statementsWithReferences as $clause) {
             if (array_key_exists($clause, $this->statements)) {
                 $this->statements[$clause] = array_map(array($this, 'createUndefinedJoins'), $this->statements[$clause]);
@@ -260,14 +277,17 @@ protected function buildQuery() {
      *
      * @param string $statement
      *
-     * @return string  rewrited $statement (e.g. tab1.tab2:col => tab2.col)
+     * @return string - the rewritten $statement (e.g. tab1.tab2:col => tab2.col)
      */
     private function createUndefinedJoins($statement) {
         if (!$this->isSmartJoinEnabled) {
             return $statement;
         }
 
-        preg_match_all('/\\b([a-z_][a-z0-9_.:]*[.:])[a-z_]*/i', $statement, $matches);
+        // matches a table name made of any printable characters followed by a dot/colon,
+        // followed by any letters, numbers and most punctuation (to exclude '*')
+        preg_match_all('/([^[:space:]\(\)]+[.:])[\p{L}\p{N}\p{Pd}\p{Pi}\p{Pf}\p{Pc}]*/u', $statement, $matches);
+
         foreach ($matches[1] as $join) {
             if (!in_array(substr($join, 0, -1), $this->joins)) {
                 $this->addJoinStatements('LEFT JOIN', $join);
@@ -282,7 +302,7 @@ private function createUndefinedJoins($statement) {
         }
 
         // remove extra referenced tables (rewrite tab1.tab2:col => tab2.col)
-        $statement = preg_replace('/(?:\\b[a-z_][a-z0-9_.:]*[.:])?([a-z_][a-z0-9_]*)[.:]([a-z_*])/i', '\\1.\\2', $statement);
+        $statement = preg_replace('/(?:[^\s]*[.:])?([^\s]+)[.:]([^\s]*)/u', '$1.$2', $statement);
 
         return $statement;
     }

diff --git a/FluentPDO/FluentUtils.php b/FluentPDO/FluentUtils.php
@@ -24,11 +24,11 @@ public static function toUpperWords($string) {
      */
     public static function formatQuery($query) {
         $query = preg_replace(
-            '/\b(WHERE|FROM|GROUP BY|HAVING|ORDER BY|LIMIT|OFFSET|UNION|ON DUPLICATE KEY UPDATE|VALUES)/',
+            '/\b(WHERE|FROM|GROUP BY|HAVING|ORDER BY|LIMIT|OFFSET|UNION|ON DUPLICATE KEY UPDATE|VALUES|SET)/',
             "\n$0", $query
         );
         $query = preg_replace(
-            '/\b(INNER|LEFT|RIGHT|CASE|WHEN|END|ELSE|AND)/',
+            '/\b(INNER|OUTER|LEFT|RIGHT|FULL|CASE|WHEN|END|ELSE|AND)/',
             "\n    $0", $query
         );
 

diff --git a/FluentPDO/SelectQuery.php b/FluentPDO/SelectQuery.php
@@ -184,11 +184,11 @@ public function count() {
     }
 
     public function getIterator() {
-        if($this->convertTypes){
+        if ($this->convertTypes) {
             return new ArrayIterator($this->fetchAll());
-        }else{
+        } else {
             return $this->execute();
-        }       
+        }
     }
 
 }
diff --git a/tests/39-update-basic.phpt b/tests/39-update-basic.phpt
@@ -19,7 +19,8 @@ $query = $fpdo->from('country')->where('id', 1);
 print_r($query->fetch());
 ?>
 --EXPECTF--
-UPDATE country SET name = ?
+UPDATE country
+SET name = ?
 WHERE id = ?
 Array
 (

diff --git a/tests/40-update-literal.phpt b/tests/40-update-literal.phpt
@@ -10,7 +10,8 @@ echo $query->getQuery() . "\n";
 print_r($query->getParameters()) . "\n";
 ?>
 --EXPECTF--
-UPDATE article SET published_at = NOW()
+UPDATE article
+SET published_at = NOW()
 WHERE user_id = ?
 Array
 (

diff --git a/tests/41-update-from-array.phpt b/tests/41-update-from-array.phpt
@@ -14,7 +14,8 @@ $query = $fpdo->update('user')->set(array('name' => 'Marek', '`type`' => 'admin'
 $query->execute();
 ?>
 --EXPECTF--
-UPDATE user SET name = ?, `type` = ?
+UPDATE user
+SET name = ?, `type` = ?
 WHERE id = ?
 Array
 (

diff --git a/tests/42-update-left-join.phpt b/tests/42-update-left-join.phpt
@@ -14,7 +14,9 @@ echo $query->getQuery() . "\n";
 print_r($query->getParameters()) . "\n";
 ?>
 --EXPECTF--
-UPDATE user OUTER JOIN country ON country.id = user.country_id SET name = ?, `type` = ?
+UPDATE user
+    OUTER JOIN country ON country.id = user.country_id
+SET name = ?, `type` = ?
 WHERE id = ?
 Array
 (

diff --git a/tests/43-update-smart-join.phpt b/tests/43-update-smart-join.phpt
@@ -14,7 +14,8 @@ print_r($query->getParameters()) . "\n";
 ?>
 --EXPECTF--
 UPDATE user
-    LEFT JOIN country ON country.id = user.country_id SET type = ?
+    LEFT JOIN country ON country.id = user.country_id
+SET type = ?
 WHERE country.id = ?
 Array
 (

diff --git a/tests/44-update-order-limit.phpt b/tests/44-update-order-limit.phpt
@@ -15,7 +15,8 @@ echo $query->getQuery() . "\n";
 print_r($query->getParameters()) . "\n";
 ?>
 --EXPECTF--
-UPDATE user SET type = ?
+UPDATE user
+SET type = ?
 WHERE id = ?
 ORDER BY name
 LIMIT 1

diff --git a/tests/50-update-shortcut.phpt b/tests/50-update-shortcut.phpt
@@ -11,7 +11,8 @@ print_r($query->getParameters()) . "\n";
 
 ?>
 --EXPECTF--
-UPDATE user SET type = ?
+UPDATE user
+SET type = ?
 WHERE id = ?
 Array
 (