diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 243d7f25..971e6c22 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ jobs: strategy: matrix: - php-version: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5'] + php-version: ['7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5'] steps: - name: Checkout @@ -63,7 +63,7 @@ jobs: strategy: matrix: - php-version: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5'] + php-version: ['7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5'] steps: - name: Checkout @@ -114,7 +114,7 @@ jobs: strategy: matrix: - php-version: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5'] + php-version: ['7.4', '8.0', '8.1', '8.2', '8.3', '8.4', '8.5'] steps: - name: Checkout diff --git a/composer.json b/composer.json index f10288fe..d9ad0548 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,7 @@ } ], "require": { - "php": ">=7.2.5", + "php": ">=7.4", "ext-PDO": "*", "ext-json": "*", "ext-simplexml": "*", diff --git a/src/xPDO/Om/mysql/xPDOQuery.php b/src/xPDO/Om/mysql/xPDOQuery.php index 38988237..3d2ddc06 100644 --- a/src/xPDO/Om/mysql/xPDOQuery.php +++ b/src/xPDO/Om/mysql/xPDOQuery.php @@ -78,7 +78,9 @@ public function construct() { foreach ($this->query['set'] as $setKey => $setVal) { $value = $setVal['value']; $type = $setVal['type']; - if ($value !== null && in_array($type, array(\PDO::PARAM_INT, \PDO::PARAM_STR))) { + if ($value instanceof \xPDO\Om\xPDOExpression) { + $value = $value->getExpression(); + } elseif ($value !== null && in_array($type, array(\PDO::PARAM_INT, \PDO::PARAM_STR))) { $value = $this->xpdo->quote($value, $type); } elseif ($value === null) { $value = 'NULL'; diff --git a/src/xPDO/Om/pgsql/xPDOQuery.php b/src/xPDO/Om/pgsql/xPDOQuery.php index 095eeec0..baa3e0d4 100644 --- a/src/xPDO/Om/pgsql/xPDOQuery.php +++ b/src/xPDO/Om/pgsql/xPDOQuery.php @@ -69,7 +69,9 @@ public function construct() { foreach ($this->query['set'] as $setKey => $setVal) { $value = $setVal['value']; $type = $setVal['type']; - if ($value !== null && in_array($type, array(\PDO::PARAM_INT, \PDO::PARAM_STR))) { + if ($value instanceof \xPDO\Om\xPDOExpression) { + $value = $value->getExpression(); + } elseif ($value !== null && in_array($type, array(\PDO::PARAM_INT, \PDO::PARAM_STR))) { $value = $this->xpdo->quote($value, $type); } elseif ($value === null) { $value = 'NULL'; diff --git a/src/xPDO/Om/sqlite/xPDOQuery.php b/src/xPDO/Om/sqlite/xPDOQuery.php index f2364402..6da8bac8 100644 --- a/src/xPDO/Om/sqlite/xPDOQuery.php +++ b/src/xPDO/Om/sqlite/xPDOQuery.php @@ -78,7 +78,9 @@ public function construct() { foreach ($this->query['set'] as $setKey => $setVal) { $value = $setVal['value']; $type = $setVal['type']; - if ($value !== null && in_array($type, array(\PDO::PARAM_INT, \PDO::PARAM_STR))) { + if ($value instanceof \xPDO\Om\xPDOExpression) { + $value = $value->getExpression(); + } elseif ($value !== null && in_array($type, array(\PDO::PARAM_INT, \PDO::PARAM_STR))) { $value = $this->xpdo->quote($value, $type); } elseif ($value === null) { $value = 'NULL'; diff --git a/src/xPDO/Om/sqlsrv/xPDOQuery.php b/src/xPDO/Om/sqlsrv/xPDOQuery.php index d191ea95..8777c80b 100644 --- a/src/xPDO/Om/sqlsrv/xPDOQuery.php +++ b/src/xPDO/Om/sqlsrv/xPDOQuery.php @@ -249,7 +249,9 @@ public function construct() { foreach ($this->query['set'] as $setKey => $setVal) { $value = $setVal['value']; $type = $setVal['type']; - if ($value !== null && in_array($type, array(\PDO::PARAM_INT, \PDO::PARAM_STR))) { + if ($value instanceof \xPDO\Om\xPDOExpression) { + $value = $value->getExpression(); + } elseif ($value !== null && in_array($type, array(\PDO::PARAM_INT, \PDO::PARAM_STR))) { $value = $this->xpdo->quote($value, $type); } elseif ($value === null) { $value = 'NULL'; diff --git a/src/xPDO/Om/xPDOExpression.php b/src/xPDO/Om/xPDOExpression.php new file mode 100644 index 00000000..58d33be4 --- /dev/null +++ b/src/xPDO/Om/xPDOExpression.php @@ -0,0 +1,67 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace xPDO\Om; + +/** + * Wraps a raw SQL expression for use in query SET clauses, bypassing automatic quoting. + * + * Use this class to explicitly indicate that a value should be treated as a raw + * SQL fragment rather than a literal string to be quoted. This is analogous to + * Laravel's DB::raw(). + * + * Plain PHP strings passed to xPDOQuery::set() or xPDO::updateCollection() are + * always quoted/escaped for SQL injection safety. Wrap a value in xPDOExpression + * only when you intentionally need raw SQL, such as: + * + * + * // Increment a counter column using a SQL expression + * $xpdo->updateCollection('MyClass', [ + * 'view_count' => $xpdo->expression('view_count + 1'), + * ]); + * + * // Use a SQL function as a SET value + * $object->set('updated_at', $xpdo->expression('NOW()')); + * $object->save(); + * + * + * @package xPDO\Om + */ +final class xPDOExpression +{ + /** @var string The raw SQL expression string. */ + private string $expression; + + /** + * @param string $expression The raw SQL expression to use verbatim. + */ + public function __construct(string $expression) + { + $this->expression = $expression; + } + + /** + * Returns the raw SQL expression string. + * + * @return string + */ + public function getExpression(): string + { + return $this->expression; + } + + /** + * @return string + */ + public function __toString(): string + { + return $this->expression; + } +} diff --git a/src/xPDO/Om/xPDOObject.php b/src/xPDO/Om/xPDOObject.php index dcb4a641..1fdd59cd 100644 --- a/src/xPDO/Om/xPDOObject.php +++ b/src/xPDO/Om/xPDOObject.php @@ -798,7 +798,13 @@ public function set($k, $v= null, $vType= '') { $phptype= $this->_fieldMeta[$k]['phptype']; $dbtype= $this->_fieldMeta[$k]['dbtype']; $allowNull= isset($this->_fieldMeta[$k]['null']) ? (bool) $this->_fieldMeta[$k]['null'] : true; - if ($v === null) { + if ($v instanceof \xPDO\Om\xPDOExpression) { + // Raw SQL expression: bypass type coercion and store as-is. + // The expression will be inlined verbatim in save(). + $this->_fields[$k]= $v; + $set= true; + } + elseif ($v === null) { if ($allowNull) { $this->_fields[$k]= null; $set= true; @@ -1349,6 +1355,7 @@ public function save($cacheFlag= null) { if (!empty ($this->_dirty)) { $cols= array (); $bindings= array (); + $valuesSql= array (); $updateSql= array (); foreach (array_keys($this->_dirty) as $_k) { if (!array_key_exists($_k, $this->_fieldMeta)) { @@ -1360,12 +1367,22 @@ public function save($cacheFlag= null) { continue; } } - if ($this->_fieldMeta[$_k]['phptype'] === 'password') { + if ($this->_fieldMeta[$_k]['phptype'] === 'password' && !($this->_fields[$_k] instanceof \xPDO\Om\xPDOExpression)) { $this->_fields[$_k]= $this->encode($this->_fields[$_k], 'password'); } $fieldType= \PDO::PARAM_STR; $fieldValue= $this->_fields[$_k]; - if (in_array($this->_fieldMeta[$_k]['phptype'], array ('datetime', 'timestamp')) && !empty($this->_fieldMeta[$_k]['attributes']) && $this->_fieldMeta[$_k]['attributes'] == 'ON UPDATE CURRENT_TIMESTAMP') { + if ($fieldValue instanceof \xPDO\Om\xPDOExpression) { + // Raw SQL expression: inline verbatim, do not create a PDO binding. + if ($this->_new) { + $cols[$_k]= $this->xpdo->escape($_k); + $valuesSql[$_k]= $fieldValue->getExpression(); + } else { + $updateSql[]= $this->xpdo->escape($_k) . ' = ' . $fieldValue->getExpression(); + } + continue; + } + elseif (in_array($this->_fieldMeta[$_k]['phptype'], array ('datetime', 'timestamp')) && !empty($this->_fieldMeta[$_k]['attributes']) && $this->_fieldMeta[$_k]['attributes'] == 'ON UPDATE CURRENT_TIMESTAMP') { $this->_fields[$_k]= date('Y-m-d H:i:s'); continue; } @@ -1392,6 +1409,7 @@ public function save($cacheFlag= null) { $cols[$_k]= $this->xpdo->escape($_k); $bindings[":{$_k}"]['value']= $fieldValue; $bindings[":{$_k}"]['type']= $fieldType; + $valuesSql[$_k]= ":{$_k}"; } else { $bindings[":{$_k}"]['value']= $fieldValue; $bindings[":{$_k}"]['type']= $fieldType; @@ -1399,7 +1417,7 @@ public function save($cacheFlag= null) { } } if ($this->_new) { - $sql= "INSERT INTO {$this->_table} (" . implode(', ', array_values($cols)) . ") VALUES (" . implode(', ', array_keys($bindings)) . ")"; + $sql= "INSERT INTO {$this->_table} (" . implode(', ', array_values($cols)) . ") VALUES (" . implode(', ', array_values($valuesSql)) . ")"; } else { if ($pk && $pkn) { if (is_array($pkn)) { diff --git a/src/xPDO/Om/xPDOQuery.php b/src/xPDO/Om/xPDOQuery.php index c4e98907..47420cb4 100644 --- a/src/xPDO/Om/xPDOQuery.php +++ b/src/xPDO/Om/xPDOQuery.php @@ -258,13 +258,18 @@ public function set(array $values) { } } if (array_key_exists($key, $fieldMeta)) { - if ($value === null) { + if ($value instanceof xPDOExpression) { + // Raw SQL expression: leave $type as null so construct() uses it verbatim. + } + elseif ($value === null) { $type= \PDO::PARAM_NULL; } elseif (!in_array($fieldMeta[$key]['phptype'], $this->_quotable)) { $type= \PDO::PARAM_INT; } - elseif (strpos($value, '(') === false && !$this->isConditionalClause($value)) { + else { + // All plain string values are always quoted regardless of content. + // Use xPDOExpression to explicitly pass raw SQL expressions. $type= \PDO::PARAM_STR; } $this->query['set'][$key]= array('value' => $value, 'type' => $type); diff --git a/src/xPDO/xPDO.php b/src/xPDO/xPDO.php index 9567b3de..0c412319 100644 --- a/src/xPDO/xPDO.php +++ b/src/xPDO/xPDO.php @@ -2682,6 +2682,22 @@ public function newQuery($class, $criteria= null, $cacheFlag= true) { return $query; } + /** + * Creates an xPDOExpression wrapper to mark a value as a raw SQL fragment. + * + * Use this when you need to pass a raw SQL expression as the value in a SET + * clause via updateCollection() or xPDOQuery::set(), bypassing automatic + * quoting and SQL injection protection. Plain PHP strings should always be + * passed without this wrapper. + * + * @param string $expression The raw SQL expression. + * @return Om\xPDOExpression + */ + public function expression(string $expression): Om\xPDOExpression + { + return new Om\xPDOExpression($expression); + } + /** * Splits a string on a specified character, ignoring escaped content. * diff --git a/test/xPDO/Test/Om/xPDOExpressionTest.php b/test/xPDO/Test/Om/xPDOExpressionTest.php new file mode 100644 index 00000000..3f85f44a --- /dev/null +++ b/test/xPDO/Test/Om/xPDOExpressionTest.php @@ -0,0 +1,60 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace xPDO\Test\Om; + +use ReflectionClass; +use xPDO\Om\xPDOExpression; +use xPDO\TestCase; + +/** + * Tests for the xPDOExpression value object. + * + * @package xPDO\Test\Om + */ +class xPDOExpressionTest extends TestCase +{ + /** + * Test that the expression stores and returns the raw SQL string. + */ + public function testExpressionStoresValue() + { + $expr = new xPDOExpression('NOW()'); + $this->assertEquals('NOW()', $expr->getExpression()); + } + + /** + * Test that __toString() returns the raw SQL string. + */ + public function testExpressionToString() + { + $expr = new xPDOExpression('NOW()'); + $this->assertEquals('NOW()', (string)$expr); + } + + /** + * Test that xPDOExpression is declared final to prevent subclassing. + */ + public function testExpressionIsFinal() + { + $ref = new ReflectionClass(xPDOExpression::class); + $this->assertTrue($ref->isFinal()); + } + + /** + * Test the xPDO::expression() factory method returns an xPDOExpression. + */ + public function testXpdoFactoryMethod() + { + $expr = $this->xpdo->expression('CURRENT_TIMESTAMP'); + $this->assertInstanceOf(xPDOExpression::class, $expr); + $this->assertEquals('CURRENT_TIMESTAMP', $expr->getExpression()); + } +} diff --git a/test/xPDO/Test/Om/xPDOObjectTest.php b/test/xPDO/Test/Om/xPDOObjectTest.php index 2117a2f3..b1a5307f 100644 --- a/test/xPDO/Test/Om/xPDOObjectTest.php +++ b/test/xPDO/Test/Om/xPDOObjectTest.php @@ -10,6 +10,7 @@ namespace xPDO\Test\Om; +use xPDO\Om\xPDOExpression; use xPDO\Om\xPDOObject; use xPDO\Test\Sample\Person; use xPDO\TestCase; @@ -806,4 +807,42 @@ public function testRemoveCollection() } $this->assertTrue($result === 2, "Error removing a collection of objects."); } + + /** + * Test that xPDOObject::set() accepts an xPDOExpression and stores it without type coercion. + */ + public function testObjectSetAcceptsExpression() + { + $person = $this->xpdo->newObject('xPDO\\Test\\Sample\\Person'); + $expr = new xPDOExpression('UPPER(first_name)'); + $result = $person->set('username', $expr); + + $this->assertTrue($result, 'xPDOObject::set() should return true when setting an xPDOExpression.'); + $stored = $person->get('username'); + $this->assertInstanceOf(xPDOExpression::class, $stored, 'xPDOObject::get() should return the xPDOExpression instance as stored.'); + $this->assertSame($expr, $stored, 'The stored value must be the same xPDOExpression instance.'); + } + + /** + * Test that xPDOObject::save() handles an xPDOExpression in an UPDATE (existing object). + */ + public function testObjectSaveWithExpressionInUpdate() + { + $person = $this->xpdo->getObject('xPDO\\Test\\Sample\\Person', array('first_name' => 'Johnathon')); + $this->assertNotNull($person, 'Could not retrieve test person fixture.'); + + $originalLevel = (int)$person->get('security_level'); + + $person->set('security_level', $this->xpdo->expression('security_level + 5')); + $saveResult = $person->save(); + + $this->assertTrue($saveResult, 'xPDOObject::save() with an xPDOExpression should return true.'); + + $reloaded = $this->xpdo->getObject('xPDO\\Test\\Sample\\Person', $person->get('id')); + $this->assertEquals( + $originalLevel + 5, + (int)$reloaded->get('security_level'), + 'xPDOExpression in save() should have incremented security_level by 5.' + ); + } } diff --git a/test/xPDO/Test/Om/xPDOQueryTest.php b/test/xPDO/Test/Om/xPDOQueryTest.php index 26e49d24..8a6d9e94 100644 --- a/test/xPDO/Test/Om/xPDOQueryTest.php +++ b/test/xPDO/Test/Om/xPDOQueryTest.php @@ -10,6 +10,7 @@ namespace xPDO\Test\Om; +use xPDO\Om\xPDOExpression; use xPDO\Om\xPDOObject; use xPDO\Om\xPDOQuery; use xPDO\Om\xPDOQueryCondition; @@ -590,4 +591,86 @@ public function providerInvalidClause() { array("if(now()=sysdate(),sleep (20),0)"), ); } + + /** + * Test that updateCollection() succeeds when a string value contains SQL operator keywords. + * + * Regression test for https://github.com/modxcms/revolution/issues/9487 + */ + public function testUpdateCollectionWithSQLKeywordInString() + { + $person = $this->xpdo->getObject('xPDO\\Test\\Sample\\Person', array('first_name' => 'Johnathon')); + $this->assertNotNull($person, 'Could not retrieve test person fixture.'); + + $valueWithSQLKeyword = 'The word IN is IN this strINg'; + $result = $this->xpdo->updateCollection( + 'xPDO\\Test\\Sample\\Person', + array('username' => $valueWithSQLKeyword), + array('id' => $person->get('id')) + ); + + $this->assertNotFalse($result, 'updateCollection() returned false — SQL syntax error likely caused by SQL keyword in string value.'); + $this->assertEquals(1, $result, 'updateCollection() should have updated exactly 1 row.'); + + $reloaded = $this->xpdo->getObject('xPDO\\Test\\Sample\\Person', $person->get('id')); + $this->assertEquals($valueWithSQLKeyword, $reloaded->get('username'), 'The saved value should match the original string with SQL keywords.'); + } + + /** + * Test that updateCollection() with an xPDOExpression executes raw SQL. + */ + public function testUpdateCollectionWithExpression() + { + $person = $this->xpdo->getObject('xPDO\\Test\\Sample\\Person', array('first_name' => 'Johnathon')); + $this->assertNotNull($person, 'Could not retrieve test person fixture.'); + + $originalLevel = (int)$person->get('security_level'); + + $result = $this->xpdo->updateCollection( + 'xPDO\\Test\\Sample\\Person', + array('security_level' => $this->xpdo->expression('security_level + 1')), + array('id' => $person->get('id')) + ); + + $this->assertNotFalse($result, 'updateCollection() with an xPDOExpression returned false.'); + $this->assertEquals(1, $result, 'updateCollection() should have updated exactly 1 row.'); + + $reloaded = $this->xpdo->getObject('xPDO\\Test\\Sample\\Person', $person->get('id')); + $this->assertEquals($originalLevel + 1, (int)$reloaded->get('security_level'), 'xPDOExpression should have incremented security_level by 1.'); + } + + /** + * Test that xPDOQuery::set() assigns PDO::PARAM_STR to a string containing SQL keywords. + */ + public function testSetQueryStringWithSQLKeywordIsParamStr() + { + $query = $this->xpdo->newQuery('xPDO\\Test\\Sample\\Person'); + $query->command('UPDATE'); + $query->set(array('username' => 'The word IN is IN this strINg')); + + $this->assertArrayHasKey('username', $query->query['set'], 'Field should be present in query set.'); + $this->assertEquals( + \PDO::PARAM_STR, + $query->query['set']['username']['type'], + 'A plain string containing SQL keywords must be typed as PDO::PARAM_STR (always quoted).' + ); + } + + /** + * Test that xPDOQuery::set() assigns null type to an xPDOExpression (raw SQL sentinel). + */ + public function testSetQueryWithExpressionObjectHasNullType() + { + $expr = new xPDOExpression('NOW()'); + $query = $this->xpdo->newQuery('xPDO\\Test\\Sample\\Person'); + $query->command('UPDATE'); + $query->set(array('dob' => $expr)); + + $this->assertArrayHasKey('dob', $query->query['set'], 'Field should be present in query set.'); + $this->assertNull( + $query->query['set']['dob']['type'], + 'An xPDOExpression must have null type so construct() uses it verbatim.' + ); + $this->assertSame($expr, $query->query['set']['dob']['value'], 'The xPDOExpression instance must be stored as the value.'); + } }