From 00fc907fdaacb3869faacf092f2aeed6d61e1932 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 01:01:40 +0600 Subject: [PATCH 01/11] feat(query)!: harden identifier rendering Preserve logical-to-physical qualification while validating and quoting every structured column segment. BREAKING CHANGE: unknown or schema-qualified names and pre-quoted structured identifiers now fail closed; register the table or alias, or use a reviewed raw API. --- src/Concerns/QueriesRelationships.php | 2 +- src/Query/Grammar.php | 20 ++- src/Query/Identifier.php | 44 ++++++ src/QueryBuilder.php | 87 +++++++---- tests/BelongsToManyPivotTest.php | 10 +- .../IdentifierQualificationRegressionTest.php | 109 ++++++++++++++ tests/OrderGroupInjectionFixTest.php | 24 +-- tests/QualifiedColumnPrefixTest.php | 47 +++--- tests/Query/GrammarTest.php | 14 +- tests/Query/IdentifierTest.php | 141 ++++++++++++++++++ tests/QueryFeaturesTest.php | 4 +- tests/SelectJoinAggregateEdgeTest.php | 33 ++-- 12 files changed, 435 insertions(+), 100 deletions(-) create mode 100644 src/Query/Identifier.php create mode 100644 tests/IdentifierQualificationRegressionTest.php create mode 100644 tests/Query/IdentifierTest.php diff --git a/src/Concerns/QueriesRelationships.php b/src/Concerns/QueriesRelationships.php index 3cce9cd..35a5fe5 100644 --- a/src/Concerns/QueriesRelationships.php +++ b/src/Concerns/QueriesRelationships.php @@ -147,7 +147,7 @@ public function withAggregate($relation, $column, $function) $this->assertSafeAggregateFunction($function); if (empty($this->select)) { - $this->select = ["`{$this->_model->getTable()}`.*"]; + $this->select = [$this->_model->getTable() . '.*']; } [$relations, $columns] = $this->normalizeAggregateRelations((array) $relation, $column); diff --git a/src/Query/Grammar.php b/src/Query/Grammar.php index ea1441b..c5b2479 100644 --- a/src/Query/Grammar.php +++ b/src/Query/Grammar.php @@ -28,8 +28,10 @@ public function compileSelect(QueryBuilder $query): string { $query->resetBindings(); - $columns = array_map([$query, 'resolveQualifier'], $query->select); - $sql = 'SELECT ' . ($query->isDistinct() ? 'DISTINCT ' : '') . implode(',', $columns); + $columns = array_map(static function ($column) use ($query) { + return $query->renderIdentifier($column, true, true); + }, $query->select); + $sql = 'SELECT ' . ($query->isDistinct() ? 'DISTINCT ' : '') . implode(',', $columns); $sql .= $this->prepareRawSelect($query); $sql .= ' FROM ' . $query->getTable(); $sql .= $this->getFrom($query); @@ -149,7 +151,9 @@ private function getGroupBy(QueryBuilder $query) return ''; } - return ' GROUP BY ' . implode(',', array_map([$query, 'resolveQualifier'], $groupBy)); + return ' GROUP BY ' . implode(',', array_map(static function ($column) use ($query) { + return $query->renderIdentifier($column); + }, $groupBy)); } /** @@ -185,7 +189,7 @@ private function getOrderBy(QueryBuilder $query) $sql .= $order['raw'] . ', '; $query->addBindings($order['bindings']); } elseif (isset($order['column'])) { - $sql .= $query->resolveQualifier($order['column']) . ' ' . $order['direction'] . ', '; + $sql .= $query->renderIdentifier($order['column']) . ' ' . $order['direction'] . ', '; } } @@ -259,7 +263,7 @@ private function prepareRawSelect(QueryBuilder $query) private function prepareColumnForWhere(QueryBuilder $query, $clause) { if (isset($clause['column'])) { - return ' ' . $query->resolveQualifier($query->prepareColumnName($clause['column'])); + return ' ' . $query->renderIdentifier($clause['column']); } } @@ -301,7 +305,11 @@ private function prepareValueForWhere(QueryBuilder $query, $clause) { $sql = ''; if (isset($clause['secondColumn'])) { - return ' ' . $query->resolveQualifier($clause['secondColumn']); + if (preg_match('/^[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)?$/', $clause['secondColumn'])) { + return ' ' . $query->renderIdentifier($clause['secondColumn']); + } + + return ' ' . $clause['secondColumn']; } if (!isset($clause['value'])) { diff --git a/src/Query/Identifier.php b/src/Query/Identifier.php new file mode 100644 index 0000000..5f03124 --- /dev/null +++ b/src/Query/Identifier.php @@ -0,0 +1,44 @@ + $segment) { + if ($segment === '*' && $allowWildcard && $index === $last) { + continue; + } + + self::assertSimple($segment); + } + + return implode('.', array_map(static function ($segment) { + return $segment === '*' ? '*' : '`' . $segment . '`'; + }, $segments)); + } + + public static function quoteAlias(string $value): string + { + self::assertSimple($value); + + return '`' . $value . '`'; + } +} diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 1e97568..cc127a7 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -4,6 +4,7 @@ use BitApps\WPDatabase\Concerns\QueriesRelationships; use BitApps\WPDatabase\Query\Grammar; +use BitApps\WPDatabase\Query\Identifier; use Closure; use DateTime; @@ -363,20 +364,7 @@ public function all($columns = ['*']) public function prepareColumnName(string $column) { - if (preg_match('/^(.+?)\s+as\s+(.+)$/i', $column, $matches)) { - return $this->prepareColumnName(trim($matches[1])) . ' AS `' . trim($matches[2], " `") . '`'; - } - - if (strpos($column, '.') !== false) { - return $column; - } - - $table = "`{$this->table}`."; - if ($column != '*') { - $column = "`{$column}`"; - } - - return $table . $column; + return $this->renderIdentifier($column, true, true); } /** @@ -392,7 +380,7 @@ public function select($columns = ['*']) $this->select = []; foreach ($select as $column) { - $this->select[] = $this->prepareColumnName($column); + $this->select[] = $column; } return $this; @@ -437,7 +425,7 @@ public function addSelect($columns) if (\in_array($column, $this->select, true)) { continue; } - $this->select[] = $this->prepareColumnName($column); + $this->select[] = $column; } return $this; @@ -895,10 +883,13 @@ public function getTableAliases() } /** - * Rewrites a qualified column whose table part is an unprefixed name this - * query owns (`users.id` -> `` `wp_users`.id ``). Aliases win over the map, - * and unknown / already-physical qualifiers pass through unchanged. - * Idempotent: a physical qualifier is never a map key. + * Renders a structured column identifier in this query's table context. + * + * Bare columns are qualified with the model's physical table. A qualified + * column must name a registered logical table, its mapped physical table, + * or an alias in scope. Unknown and schema-qualified names intentionally + * fail closed; callers needing those rare forms must register the table or + * alias, or use a reviewed raw-expression API. * * @param mixed $column * @@ -910,21 +901,55 @@ public function resolveQualifier($column) return $column; } - $dot = strpos($column, '.'); - if ($dot === false) { - return $column; + return $this->renderIdentifier($column, true, true); + } + + /** + * Validates, resolves, and quotes a column at the SQL rendering boundary. + * + * @param string $column + * @param bool $allowWildcard + * @param bool $allowAlias + * + * @return string + */ + public function renderIdentifier(string $column, bool $allowWildcard = false, bool $allowAlias = false): string + { + if (preg_match('/^(.+?)\s+AS\s+(.+)$/i', $column, $matches)) { + if (!$allowAlias) { + throw new RuntimeException('Aliases are not allowed in this SQL identifier context.'); + } + + return $this->renderIdentifier($matches[1], $allowWildcard) + . ' AS ' . Identifier::quoteAlias($matches[2]); + } + + $segments = explode('.', $column); + if (\count($segments) > 2) { + throw new RuntimeException('Unknown or schema-qualified SQL identifier.'); } - $left = trim(substr($column, 0, $dot), '`'); - $right = substr($column, $dot + 1); + Identifier::quoteQualified($column, $allowWildcard); - if (\in_array($left, $this->getTableAliases(), true)) { - return $column; + if (\count($segments) === 1) { + return Identifier::quoteQualified($this->table . '.' . $column, $allowWildcard); } - $map = $this->getTableMap(); + [$qualifier, $name] = $segments; + $aliases = $this->getTableAliases(); + $map = $this->getTableMap(); - return isset($map[$left]) ? '`' . $map[$left] . '`.' . $right : $column; + if (\in_array($qualifier, $aliases, true)) { + $resolved = $qualifier; + } elseif (isset($map[$qualifier])) { + $resolved = $map[$qualifier]; + } elseif (\in_array($qualifier, array_values($map), true)) { + $resolved = $qualifier; + } else { + throw new RuntimeException('Unknown SQL identifier qualifier.'); + } + + return Identifier::quoteQualified($resolved . '.' . $name, $allowWildcard); } /** @@ -1874,9 +1899,11 @@ private function nullOperator($operator) */ private function assertSafeIdentifier($column) { - if (!\is_string($column) || !preg_match('/^[A-Za-z0-9_.`]+$/', $column)) { + if (!\is_string($column)) { throw new RuntimeException('Unsafe column passed to order/group by clause.'); } + + Identifier::quoteQualified($column); } /** diff --git a/tests/BelongsToManyPivotTest.php b/tests/BelongsToManyPivotTest.php index b762189..5e089c0 100644 --- a/tests/BelongsToManyPivotTest.php +++ b/tests/BelongsToManyPivotTest.php @@ -65,7 +65,7 @@ public function testEagerSqlShape(): void $this->assertStringContainsString('SELECT `wp_roles`.*', $sql); $this->assertStringContainsString('wp_role_user.member_id as `pivot_member_id`', $sql); $this->assertStringContainsString('INNER JOIN wp_role_user', $sql); - $this->assertStringContainsString('wp_role_user.role_id = wp_roles.id', $sql); + $this->assertStringContainsString('`wp_role_user`.`role_id` = `wp_roles`.`id`', $sql); // Inner fragment without a leading `WHERE ` boundary: the grammar emits `WHERE ` (double space). $this->assertStringContainsString( 'wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM wp_members) AS subquery )', @@ -74,7 +74,7 @@ public function testEagerSqlShape(): void // Exact pin (absorbs the double-space WHERE/ON grammar artifacts). $expected = 'SELECT `wp_roles`.*, wp_role_user.member_id as `pivot_member_id`' - . ' FROM wp_roles INNER JOIN wp_role_user ON wp_role_user.role_id = wp_roles.id' + . ' FROM wp_roles INNER JOIN wp_role_user ON `wp_role_user`.`role_id` = `wp_roles`.`id`' . ' WHERE wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM wp_members) AS subquery )'; $this->assertSame($expected, $sql); } @@ -93,8 +93,8 @@ public function testLazyAccessEmitsSingleValuePredicate(): void // Exact pin: single-value predicate, double-space WHERE/ON/`=` grammar artifacts. $expected = 'SELECT `wp_roles`.*, wp_role_user.member_id as `pivot_member_id`' - . ' FROM wp_roles INNER JOIN wp_role_user ON wp_role_user.role_id = wp_roles.id' - . ' WHERE wp_role_user.member_id = 1'; + . ' FROM wp_roles INNER JOIN wp_role_user ON `wp_role_user`.`role_id` = `wp_roles`.`id`' + . ' WHERE `wp_role_user`.`member_id` = 1'; $this->assertSame($expected, $sql); } @@ -116,7 +116,7 @@ public function testDefaultKeyDerivationUsesForeignKeyConvention(): void $sql = $GLOBALS['wpdb']->queries[1]; $this->assertStringContainsString('wp_role_user.members_id as `pivot_members_id`', $sql); - $this->assertStringContainsString('wp_role_user.roles_id = wp_roles.id', $sql); + $this->assertStringContainsString('`wp_role_user`.`roles_id` = `wp_roles`.`id`', $sql); $this->assertCount(2, $members[0]->rolesDefaultKeys); $this->assertCount(1, $members[1]->rolesDefaultKeys); } diff --git a/tests/IdentifierQualificationRegressionTest.php b/tests/IdentifierQualificationRegressionTest.php new file mode 100644 index 0000000..a04328b --- /dev/null +++ b/tests/IdentifierQualificationRegressionTest.php @@ -0,0 +1,109 @@ +assertSame( + 'SELECT `wp_users`.`id` FROM wp_users', + (new User())->select('id')->toSql() + ); + } + + public function testLogicalModelQualifierMapsToPhysicalTable(): void + { + $this->assertSame( + 'SELECT `wp_users`.`id` FROM wp_users', + (new User())->select('users.id')->toSql() + ); + } + + public function testPhysicalModelQualifierIsRecognizedAndQuoted(): void + { + $this->assertSame( + 'SELECT `wp_users`.`id` FROM wp_users', + (new User())->select('wp_users.id')->toSql() + ); + } + + public function testJoinAliasQualifierIsPreservedAndQuoted(): void + { + $this->assertSame( + 'SELECT `p`.`id` FROM wp_users INNER JOIN wp_posts as p' + . ' ON `wp_users`.`user_id` = `p`.`id`', + (new User())->join('posts as p', 'user_id', '=', 'id')->select('p.id')->toSql() + ); + } + + public function testLogicalJoinedTableMapsWhenSelectPrecedesJoin(): void + { + $this->assertSame( + 'SELECT `wp_posts`.`title` FROM wp_users INNER JOIN wp_posts' + . ' ON `wp_users`.`user_id` = `wp_posts`.`id`', + (new User())->select('posts.title')->join('posts', 'user_id', '=', 'id')->toSql() + ); + } + + public function testQualifiedWildcardResolvesOnlyAtSelectBoundary(): void + { + $this->assertSame( + 'SELECT `wp_users`.* FROM wp_users', + (new User())->select('users.*')->toSql() + ); + } + + public function testExplicitColumnAliasValidatesAndQuotesBothSides(): void + { + $this->assertSame( + 'SELECT `wp_users`.`id` AS `user_id` FROM wp_users', + (new User())->select('id AS user_id')->toSql() + ); + } + + public function testFromAliasQualifierIsRecognizedAndQuoted(): void + { + $this->assertSame( + 'SELECT `u`.`id` FROM wp_users u', + (new User())->from('u')->select('u.id')->toSql() + ); + } + + public function testUnknownQualifierFailsClosed(): void + { + $this->expectException(RuntimeException::class); + + (new User())->select('unknown.id')->toSql(); + } + + public function testSchemaQualifiedIdentifierFailsClosed(): void + { + $this->expectException(RuntimeException::class); + + (new User())->select('catalog.users.id')->toSql(); + } + + public function testImplicitColumnAliasIsRejected(): void + { + $this->expectException(RuntimeException::class); + + (new User())->select('id user_id')->toSql(); + } + + public function testPreBacktickedColumnIsRejected(): void + { + $this->expectException(RuntimeException::class); + + (new User())->select('`id`')->toSql(); + } +} diff --git a/tests/OrderGroupInjectionFixTest.php b/tests/OrderGroupInjectionFixTest.php index 9d34158..9b70fb9 100644 --- a/tests/OrderGroupInjectionFixTest.php +++ b/tests/OrderGroupInjectionFixTest.php @@ -8,8 +8,8 @@ use RuntimeException; /** - * E2: orderBy()/groupBy() reject non-identifier columns (injection guard) while - * leaving every currently-valid identifier byte-identical. + * E2: orderBy()/groupBy() reject non-identifiers and qualifiers outside the + * query context, while valid columns render as fully qualified identifiers. */ final class OrderGroupInjectionFixTest extends TestCase { @@ -37,31 +37,31 @@ public function testGroupByRejectsInjectionPayload(): void User::query()->groupBy('a); DROP'); } - public function testOrderByPlainColumnUnchanged(): void + public function testOrderByPlainColumnIsQualified(): void { $sql = User::query()->orderBy('id', 'DESC')->toSql(); - $this->assertStringContainsString('ORDER BY id ASC', $sql); + $this->assertStringContainsString('ORDER BY `wp_users`.`id` ASC', $sql); } - public function testOrderByQualifiedColumnUnchanged(): void + public function testOrderByUnknownQualifiedColumnFailsClosed(): void { - $sql = User::query()->orderBy('t.col')->toSql(); + $this->expectException(RuntimeException::class); - $this->assertStringContainsString('ORDER BY t.col ASC', $sql); + User::query()->orderBy('t.col')->toSql(); } - public function testGroupByPlainColumnUnchanged(): void + public function testGroupByPlainColumnIsQualified(): void { $sql = User::query()->groupBy('contact_id')->toSql(); - $this->assertStringContainsString('GROUP BY contact_id', $sql); + $this->assertStringContainsString('GROUP BY `wp_users`.`contact_id`', $sql); } - public function testGroupByQualifiedColumnUnchanged(): void + public function testGroupByUnknownQualifiedColumnFailsClosed(): void { - $sql = User::query()->groupBy('wp_x.module')->toSql(); + $this->expectException(RuntimeException::class); - $this->assertStringContainsString('GROUP BY wp_x.module', $sql); + User::query()->groupBy('wp_x.module')->toSql(); } } diff --git a/tests/QualifiedColumnPrefixTest.php b/tests/QualifiedColumnPrefixTest.php index 1c92e94..13ca193 100644 --- a/tests/QualifiedColumnPrefixTest.php +++ b/tests/QualifiedColumnPrefixTest.php @@ -5,19 +5,20 @@ use BitApps\WPDatabase\Tests\Fixtures\SoftPost; use BitApps\WPDatabase\Tests\Fixtures\User; use PHPUnit\Framework\TestCase; +use RuntimeException; /** * A column qualified by the *unprefixed* model or joined table name * (e.g. `users.id` when the physical table is `wp_users`) must resolve to the - * physical, back-ticked table. Already-physical names, aliases, and unknown - * tables are left untouched. Pure compilation guards via toSql(). + * physical table. Already-physical names and aliases are recognized, while + * unknown qualifiers fail closed. Every accepted segment is quoted. */ final class QualifiedColumnPrefixTest extends TestCase { public function testWhereResolvesUnprefixedModelTable(): void { $sql = (new User())->where('users.status', 1)->toSql(); - $this->assertStringContainsString('`wp_users`.status', $sql); + $this->assertStringContainsString('`wp_users`.`status`', $sql); } public function testSelectResolvesModelAndJoinedTable(): void @@ -25,8 +26,8 @@ public function testSelectResolvesModelAndJoinedTable(): void $sql = (new User())->join('posts', 'user_id', '=', 'id') ->select('users.id', 'posts.title')->toSql(); - $this->assertStringContainsString('`wp_users`.id', $sql); - $this->assertStringContainsString('`wp_posts`.title', $sql); + $this->assertStringContainsString('`wp_users`.`id`', $sql); + $this->assertStringContainsString('`wp_posts`.`title`', $sql); } public function testSelectResolvesRegardlessOfJoinOrder(): void @@ -34,33 +35,32 @@ public function testSelectResolvesRegardlessOfJoinOrder(): void $sql = (new User())->select('posts.title') ->join('posts', 'user_id', '=', 'id')->toSql(); - $this->assertStringContainsString('`wp_posts`.title', $sql); + $this->assertStringContainsString('`wp_posts`.`title`', $sql); } public function testJoinOnResolvesBothSides(): void { $sql = (new User())->join('posts', 'posts.user_id', '=', 'users.id')->toSql(); - $this->assertStringContainsString('`wp_posts`.user_id', $sql); - $this->assertStringContainsString('`wp_users`.id', $sql); + $this->assertStringContainsString('`wp_posts`.`user_id`', $sql); + $this->assertStringContainsString('`wp_users`.`id`', $sql); $this->assertStringNotContainsString(' users.id', $sql); } - public function testAlreadyPhysicalNameUntouched(): void + public function testAlreadyPhysicalNameIsRecognizedAndQuoted(): void { $sql = (new User())->where('wp_users.id', 5)->toSql(); - $this->assertStringContainsString('wp_users.id', $sql); + $this->assertStringContainsString('`wp_users`.`id`', $sql); $this->assertStringNotContainsString('wp_wp_users', $sql); - $this->assertStringNotContainsString('`wp_users`.id', $sql); } public function testAliasShadowingModelNameWins(): void { $sql = (new User())->from('users')->where('users.id', 5)->toSql(); - $this->assertStringContainsString(' users.id', $sql); - $this->assertStringNotContainsString('`wp_users`.id', $sql); + $this->assertStringContainsString(' `users`.`id`', $sql); + $this->assertStringNotContainsString('`wp_users`.`id`', $sql); } public function testGroupByResolvesJoinedTable(): void @@ -68,7 +68,7 @@ public function testGroupByResolvesJoinedTable(): void $sql = (new User())->join('posts', 'user_id', '=', 'id') ->groupBy('posts.status')->toSql(); - $this->assertStringContainsString('GROUP BY `wp_posts`.status', $sql); + $this->assertStringContainsString('GROUP BY `wp_posts`.`status`', $sql); } public function testOrderByResolvesJoinedTable(): void @@ -76,22 +76,19 @@ public function testOrderByResolvesJoinedTable(): void $sql = (new User())->join('posts', 'user_id', '=', 'id') ->orderBy('posts.title')->toSql(); - $this->assertStringContainsString('ORDER BY `wp_posts`.title', $sql); + $this->assertStringContainsString('ORDER BY `wp_posts`.`title`', $sql); } - public function testResolveQualifierIsIdempotent(): void + public function testResolveQualifierRejectsPreQuotedInput(): void { - $qb = User::query(); - $once = $qb->resolveQualifier('users.id'); - $twice = $qb->resolveQualifier($once); + $this->expectException(RuntimeException::class); - $this->assertSame('`wp_users`.id', $once); - $this->assertSame($once, $twice); + User::query()->resolveQualifier('`wp_users`.`id`'); } - public function testResolveQualifierLeavesUnqualifiedColumnAlone(): void + public function testResolveQualifierQualifiesBareColumn(): void { - $this->assertSame('id', User::query()->resolveQualifier('id')); + $this->assertSame('`wp_users`.`id`', User::query()->resolveQualifier('id')); } public function testJoinedTableResolvesInsideNestedClosure(): void @@ -101,7 +98,7 @@ public function testJoinedTableResolvesInsideNestedClosure(): void $q->where('posts.status', 1); })->toSql(); - $this->assertStringContainsString('`wp_posts`.status', $sql); + $this->assertStringContainsString('`wp_posts`.`status`', $sql); } public function testJoinedTableResolvesUnderSoftDeleteScope(): void @@ -109,6 +106,6 @@ public function testJoinedTableResolvesUnderSoftDeleteScope(): void $sql = (new SoftPost())->join('users', 'post_id', '=', 'id') ->where('users.role', 'admin')->toSql(); - $this->assertStringContainsString('`wp_users`.role', $sql); + $this->assertStringContainsString('`wp_users`.`role`', $sql); } } diff --git a/tests/Query/GrammarTest.php b/tests/Query/GrammarTest.php index 855e2df..bd00378 100644 --- a/tests/Query/GrammarTest.php +++ b/tests/Query/GrammarTest.php @@ -72,7 +72,7 @@ public function testNestedClosureWhereProducesParenthesizedGroup(): void public function testJoin(): void { $this->assertSame( - 'SELECT FROM wp_users INNER JOIN wp_posts ON `wp_users`.`user_id` = wp_posts.id', + 'SELECT FROM wp_users INNER JOIN wp_posts ON `wp_users`.`user_id` = `wp_posts`.`id`', (new User())->join('posts', 'user_id', '=', 'id')->toSql() ); } @@ -81,21 +81,21 @@ public function testJoinWithAlias(): void { $sql = (new User())->join('posts as p', 'user_id', '=', 'id')->toSql(); $this->assertStringContainsString('INNER JOIN wp_posts as p ON', $sql); - $this->assertStringContainsString('= p.id', $sql); + $this->assertStringContainsString('= `p`.`id`', $sql); } public function testJoinWithDottedColumnsResolvesKnownTables(): void { // Unprefixed model/join table names in ON columns resolve to physical. $sql = (new User())->join('posts', 'posts.user_id', '=', 'users.id')->toSql(); - $this->assertStringContainsString('`wp_posts`.user_id = `wp_users`.id', $sql); + $this->assertStringContainsString('`wp_posts`.`user_id` = `wp_users`.`id`', $sql); } public function testJoinAliasSplitToleratesExtraWhitespace(): void { $sql = (new User())->join('posts as p', 'user_id', '=', 'id')->toSql(); $this->assertStringContainsString('INNER JOIN wp_posts as p ON', $sql); - $this->assertStringContainsString('= p.id', $sql); + $this->assertStringContainsString('= `p`.`id`', $sql); } public function testJoinOnConstantSecondOperandNotPrefixed(): void @@ -115,7 +115,7 @@ public function testJoinOnFunctionSecondOperandNotPrefixed(): void public function testGroupBy(): void { $this->assertSame( - 'SELECT FROM wp_users GROUP BY status', + 'SELECT FROM wp_users GROUP BY `wp_users`.`status`', (new User())->groupBy('status')->toSql() ); } @@ -123,7 +123,7 @@ public function testGroupBy(): void public function testHaving(): void { $this->assertSame( - 'SELECT FROM wp_users GROUP BY status HAVING `wp_users`.`id` > %d', + 'SELECT FROM wp_users GROUP BY `wp_users`.`status` HAVING `wp_users`.`id` > %d', (new User())->groupBy('status')->having('id', '>', 1)->toSql() ); } @@ -131,7 +131,7 @@ public function testHaving(): void public function testOrderByDesc(): void { $this->assertSame( - 'SELECT FROM wp_users ORDER BY id DESC', + 'SELECT FROM wp_users ORDER BY `wp_users`.`id` DESC', (new User())->orderBy('id')->desc()->toSql() ); } diff --git a/tests/Query/IdentifierTest.php b/tests/Query/IdentifierTest.php new file mode 100644 index 0000000..2a956ca --- /dev/null +++ b/tests/Query/IdentifierTest.php @@ -0,0 +1,141 @@ + ['id'], + 'underscore' => ['_private'], + 'mixed' => ['Custom_Field_12'], + ]; + } + + #[DataProvider('simpleIdentifierProvider')] + public function testAcceptsSimpleIdentifierSegments(string $identifier): void + { + Identifier::assertSimple($identifier); + + $this->addToAssertionCount(1); + } + + public static function invalidSimpleIdentifierProvider(): array + { + return [ + 'empty' => [''], + 'leading digit' => ['1id'], + 'empty segment' => ['user..id'], + 'implicit alias' => ['id alias'], + 'pre-backticked' => ['`id`'], + 'comment' => ['id/**/DESC'], + 'line comment' => ['id--comment'], + 'expression' => ['LOWER(id)'], + 'reported payload' => ['id; DROP TABLE x'], + 'reported payload2' => ['a); DROP'], + ]; + } + + #[DataProvider('invalidSimpleIdentifierProvider')] + public function testRejectsInvalidSimpleIdentifierSegments(string $identifier): void + { + $this->expectException(RuntimeException::class); + + Identifier::assertSimple($identifier); + } + + public static function qualifiedIdentifierProvider(): array + { + return [ + 'simple' => ['id', '`id`'], + 'qualified' => ['users.id', '`users`.`id`'], + 'multiple levels' => ['catalog.users.id', '`catalog`.`users`.`id`'], + ]; + } + + #[DataProvider('qualifiedIdentifierProvider')] + public function testQuotesEveryQualifiedIdentifierSegment(string $identifier, string $expected): void + { + $this->assertSame($expected, Identifier::quoteQualified($identifier)); + } + + public function testQuotesWildcardOnlyWhenExplicitlyAllowed(): void + { + $this->assertSame('*', Identifier::quoteQualified('*', true)); + $this->assertSame('`users`.*', Identifier::quoteQualified('users.*', true)); + } + + public static function forbiddenWildcardProvider(): array + { + return [ + 'bare wildcard' => ['*', false], + 'qualified wildcard' => ['users.*', false], + 'leading wildcard' => ['*.id', true], + 'embedded wildcard' => ['users.*.id', true], + 'partial wildcard' => ['users.i*', true], + ]; + } + + #[DataProvider('forbiddenWildcardProvider')] + public function testRejectsWildcardOutsidePermittedPosition(string $identifier, bool $allowWildcard): void + { + $this->expectException(RuntimeException::class); + + Identifier::quoteQualified($identifier, $allowWildcard); + } + + public static function invalidQualifiedIdentifierProvider(): array + { + return [ + 'leading dot' => ['.id'], + 'trailing dot' => ['users.'], + 'empty segment' => ['users..id'], + 'whitespace' => ['users .id'], + 'pre-backticked' => ['`users`.id'], + 'comment' => ['users/**/.id'], + 'expression' => ['LOWER(users.id)'], + ]; + } + + #[DataProvider('invalidQualifiedIdentifierProvider')] + public function testRejectsInvalidQualifiedIdentifiers(string $identifier): void + { + $this->expectException(RuntimeException::class); + + Identifier::quoteQualified($identifier); + } + + public function testQuotesOnlySimpleAliases(): void + { + $this->assertSame('`user_id`', Identifier::quoteAlias('user_id')); + } + + public static function invalidAliasProvider(): array + { + return [ + 'qualified' => ['users.id'], + 'implicit' => ['id alias'], + 'pre-backticked' => ['`alias`'], + 'comment' => ['alias--'], + ]; + } + + #[DataProvider('invalidAliasProvider')] + public function testRejectsInvalidAliases(string $alias): void + { + $this->expectException(RuntimeException::class); + + Identifier::quoteAlias($alias); + } +} diff --git a/tests/QueryFeaturesTest.php b/tests/QueryFeaturesTest.php index 56ffc97..509a5dd 100644 --- a/tests/QueryFeaturesTest.php +++ b/tests/QueryFeaturesTest.php @@ -44,8 +44,8 @@ public function testInnerJoinCompilesWithOnClause(): void $this->assertStringContainsString('INNER JOIN', $sql); // Unprefixed model/join table names in ON columns resolve to physical. - $this->assertStringContainsString('`wp_posts`.user_id', $sql); - $this->assertStringContainsString('`wp_users`.id', $sql); + $this->assertStringContainsString('`wp_posts`.`user_id`', $sql); + $this->assertStringContainsString('`wp_users`.`id`', $sql); } public function testLeftJoinThenWhereKeepsBindingsInOrder(): void diff --git a/tests/SelectJoinAggregateEdgeTest.php b/tests/SelectJoinAggregateEdgeTest.php index 7f46c2d..584862c 100644 --- a/tests/SelectJoinAggregateEdgeTest.php +++ b/tests/SelectJoinAggregateEdgeTest.php @@ -6,6 +6,7 @@ use BitApps\WPDatabase\Tests\Fixtures\User; use FakeWpdb; use PHPUnit\Framework\TestCase; +use RuntimeException; /** * Characterization of SELECT column preparation (aliases), JOIN variants/ON @@ -25,9 +26,11 @@ protected function tearDown(): void // --- Select aliases ------------------------------------------------------ - public function testDottedColumnAliasKeepsDotUnquotedAliasBackticked(): void + public function testUnknownDottedColumnAliasFailsClosed(): void { - $this->assertStringContainsString('t.col AS `a`', (new User())->select(['t.col AS a'])->toSql()); + $this->expectException(RuntimeException::class); + + (new User())->select(['t.col AS a'])->toSql(); } public function testLowercaseAsNormalisesToUppercase(): void @@ -35,19 +38,25 @@ public function testLowercaseAsNormalisesToUppercase(): void $this->assertStringContainsString('`wp_users`.`col` AS `a`', (new User())->select(['col as a'])->toSql()); } - public function testAlreadyBacktickedAliasIsNotDoubleQuoted(): void + public function testAlreadyBacktickedAliasIsRejected(): void { - $this->assertStringContainsString('`wp_users`.`col` AS `a`', (new User())->select(['col AS `a`'])->toSql()); + $this->expectException(RuntimeException::class); + + (new User())->select(['col AS `a`'])->toSql(); } - public function testAliasWithSpacesIsBacktickedWhole(): void + public function testAliasWithSpacesIsRejected(): void { - $this->assertStringContainsString('`wp_users`.`col` AS `the alias`', (new User())->select(['col AS the alias'])->toSql()); + $this->expectException(RuntimeException::class); + + (new User())->select(['col AS the alias'])->toSql(); } - public function testMultipleAsSplitsOnFirst(): void + public function testMultipleAsIsRejected(): void { - $this->assertStringContainsString('`wp_users`.`a` AS `b as c`', (new User())->select(['a as b as c'])->toSql()); + $this->expectException(RuntimeException::class); + + (new User())->select(['a as b as c'])->toSql(); } public function testSelectThenSelectRawJoinedByComma(): void @@ -78,10 +87,10 @@ public function testOnAndOrOnAppendToSameJoin(): void { // Unprefixed model/join table names in ON columns resolve to physical. $and = (new User())->join('posts', 'posts.user_id', '=', 'users.id')->on('posts.status', '=', 'users.state')->toSql(); - $this->assertStringContainsString('`wp_posts`.user_id = `wp_users`.id AND `wp_posts`.status = `wp_users`.state', $and); + $this->assertStringContainsString('`wp_posts`.`user_id` = `wp_users`.`id` AND `wp_posts`.`status` = `wp_users`.`state`', $and); $or = (new User())->join('posts', 'posts.user_id', '=', 'users.id')->orOn('posts.status', '=', 'users.state')->toSql(); - $this->assertStringContainsString('`wp_posts`.user_id = `wp_users`.id OR `wp_posts`.status = `wp_users`.state', $or); + $this->assertStringContainsString('`wp_posts`.`user_id` = `wp_users`.`id` OR `wp_posts`.`status` = `wp_users`.`state`', $or); } public function testJoinOnCustomPrefixModelQualifiesBaseColumnWithFullPrefix(): void @@ -89,7 +98,7 @@ public function testJoinOnCustomPrefixModelQualifiesBaseColumnWithFullPrefix(): $sql = (new PrefixedModel())->join('gadgets', 'gid', '=', 'wid')->toSql(); $this->assertStringContainsString('INNER JOIN wp_crm_gadgets', $sql); - $this->assertStringContainsString('`wp_crm_widgets`.`gid` = wp_crm_gadgets.wid', $sql); + $this->assertStringContainsString('`wp_crm_widgets`.`gid` = `wp_crm_gadgets`.`wid`', $sql); } // --- Aggregates / ordering ---------------------------------------------- @@ -143,7 +152,7 @@ public function testAggregateCarriesWhereClause(): void public function testAscFallsBackToPrimaryKey(): void { - $this->assertStringContainsString('ORDER BY id ASC', (new User())->asc()->toSql()); + $this->assertStringContainsString('ORDER BY `wp_users`.`id` ASC', (new User())->asc()->toSql()); } public function testSkipWithoutTakeOmitsOffset(): void From 59b9b102afd78f95caf190121c84f944f348b86b Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 01:25:15 +0600 Subject: [PATCH 02/11] feat(query)!: harden structured SQL clauses Validate finite operators, join types, identifiers, and boolean connectors before execution. Split join operands into column, bound-value, and explicit raw APIs. BREAKING CHANGE: structured table declarations are quoted and ordinary join/on operands must be columns; use joinWhere/onValue for values and onRaw for expressions. --- docs/breaking-changes.md | 50 ++- docs/usage.md | 24 +- src/Model.php | 5 + src/Query/Grammar.php | 36 ++- src/Query/JoinType.php | 34 ++ src/Query/SqlOperator.php | 43 +++ src/QueryBuilder.php | 257 ++++++++++++++-- tests/BelongsToManyPivotTest.php | 10 +- tests/BuilderEntryTest.php | 2 +- tests/EagerLoadIntegrationTest.php | 2 +- .../IdentifierQualificationRegressionTest.php | 16 +- tests/Query/GrammarTest.php | 50 +-- tests/QueryFeaturesTest.php | 2 +- tests/RelationDefinitionRegressionTest.php | 2 +- tests/RelationStaticAccessTest.php | 12 +- tests/SelectJoinAggregateEdgeTest.php | 10 +- tests/StructuredClauseSafetyTest.php | 291 ++++++++++++++++++ tests/TablePrefixTest.php | 2 +- tests/WhereClauseEdgeTest.php | 4 +- 19 files changed, 737 insertions(+), 115 deletions(-) create mode 100644 src/Query/JoinType.php create mode 100644 src/Query/SqlOperator.php create mode 100644 tests/StructuredClauseSafetyTest.php diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 59254b7..c06d546 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -31,6 +31,7 @@ API and runtime behavior change, it is a **major** version bump. | 12 | QueryBuilder | exception type/message changed in `exec()` | Low | | 13 | Model | soft-delete reads exclude trashed by default (opt out with `$soft_delete_scope = false`) | Medium | | 14 | Composer | minimum PHP raised to **8.0** (was 7.4) — package no longer installs on PHP < 8.0 | High | +| 15 | QueryBuilder | structured identifiers/operators/joins now fail closed | High | --- @@ -134,19 +135,20 @@ Model object on success. ### 2.4 `select()` back-tick qualifies columns — raw expressions break -`select()` and `addSelect()` pass every column through `prepareColumnName()`, -which wraps the name as `` `table`.`column` `` unless it already contains a `.`. +`select()` and `addSelect()` accept only plain/qualified identifiers, an optional +final wildcard, and explicit `column AS alias`. Every accepted segment is quoted +and a bare column is qualified with the model table. ```php // Before ->select('COUNT(*) as total') // emitted: COUNT(*) as total // After -->select('COUNT(*) as total') // emitted: `COUNT(*) as total` ❌ invalid SQL +->select('COUNT(*) as total') // throws RuntimeException ``` -**Why it breaks:** a raw SQL expression or function call passed to `select()` is -quoted as a single identifier. +**Why it breaks:** raw expressions, pre-backticked input, implicit aliases, and +unknown/schema qualifiers now fail closed instead of passing through. A plain `column AS alias` **is** handled — `prepareColumnName()` qualifies the column and keeps the alias separate, so `->select(['id', 'title AS t'])` emits @@ -159,13 +161,6 @@ column and keeps the alias separate, so `->select(['id', 'title AS t'])` emits ->selectRaw('SUM(amount) as amt', $bindings) ``` -> **Gotcha — an expression may "accidentally" survive:** `prepareColumnName()` -> passes a column through untouched only when it already contains a `.`. So -> `select(['CONCAT("https://example.com/…", col) as x'])` emits valid SQL *merely* -> because the URL contains a dot — the identical code breaks on a dotless host -> (`http://localhost/…`). Never rely on this; route any function/expression -> through `selectRaw()`. - --- ### 2.5 `delete()` with no `WHERE` clause no longer wipes the table @@ -358,6 +353,37 @@ targets `8.0-`. --- +### 2.15 Structured identifiers, operators, and joins fail closed + +Structured query paths now validate and quote base/from/join tables, aliases, +columns, comparison operators, boolean connectors, and join types. Operators are +case-normalized from a finite set; comments, extra whitespace, and arbitrary SQL +fragments throw before a query executes. Base and joined table declarations now +emit quoted identifiers, so SQL snapshots change even when query semantics do not. + +`join()`, `on()`, and `orOn()` are now strictly column-to-column. The previous +right-operand guessing accepted constants and function calls as unbound SQL. +Migrate constants to the bound-value APIs and expressions to the explicit raw API: + +```php +// Before: guessed and interpolated as SQL +->join('orders', 'orders.status', '=', "'open'") +->on('orders.created_at', '<', 'NOW()') + +// After: values are bound +->joinWhere('orders', 'orders.status', '=', 'open') +->onValue('orders.priority', '>=', 10) + +// After: reviewed developer-authored expression is visibly raw +->join('orders', 'orders.contact_id', '=', 'contacts.id') +->onRaw('`wp_orders`.`created_at` < NOW()') +``` + +`whereBetween()` and `orWhereBetween()` now retain structured column state and +bind both bounds; hostile column text can no longer enter their SQL fragment. + +--- + ## 3. Behavioral changes Not signature breaks, but observable runtime differences. diff --git a/docs/usage.md b/docs/usage.md index 141884b..011669b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -273,6 +273,20 @@ Contact::query() ->leftJoin('notes', 'notes.contact_id', '=', 'contacts.id') ->get(); // also: rightJoin(), fullJoin(), crossJoin(), on(), orOn() + +// Compare a join column with bound values (never interpolated as SQL). +Contact::query() + ->joinWhere('orders', 'orders.status', '=', 'open') + ->onValue('orders.priority', '>=', 10) + ->orOnValue('orders.kind', '=', 'featured') + ->get(); + +// Developer-controlled expression. Bind every dynamic value. +Contact::query() + ->join('orders', 'orders.contact_id', '=', 'contacts.id') + ->onRaw('`wp_orders`.`created_at` < NOW()') + ->orOnRaw('`wp_orders`.`priority` > %d', [10]) + ->get(); ``` Pass **unprefixed** table names — `join()` prepends the model's full table prefix @@ -280,8 +294,13 @@ Pass **unprefixed** table names — `join()` prepends the model's full table pre `$prefix`). Qualify columns as `table.column` using the **unprefixed** table name: the builder resolves a qualifier that matches the model's own table or a joined table to its physical, prefixed name in `select`, `where`/`having`, `ON`, `groupBy` -and `orderBy`. Already-prefixed names, table aliases, and unknown tables are left -untouched. +and `orderBy`. Already-prefixed names and registered aliases are recognized; +unknown/schema-qualified names fail closed. Use `table AS alias` when declaring a +join alias and `from('alias')` for the base-table alias. + +`join()`, `on()`, and `orOn()` are column-to-column APIs. They no longer guess +whether the right operand is a number, quoted literal, or SQL function. Move +constants to `joinWhere()` / `onValue()` and reviewed expressions to `onRaw()`. ### Limit / offset / pagination @@ -745,4 +764,3 @@ method relocation. (`Connection::getPrefix()`). The `$prefix` property intentionally defaults to `null` (not `''`) to preserve bare-table behaviour for plugins that rely on it. See [Schema builder reference](schema.md). - diff --git a/src/Model.php b/src/Model.php index d409439..eb8e9a5 100644 --- a/src/Model.php +++ b/src/Model.php @@ -56,10 +56,15 @@ * @method static QueryBuilder having(...$params) * @method static QueryBuilder orHaving(...$params) * @method static QueryBuilder join($table, $first_column, $operator = null, $second_column = null, $type = 'INNER') + * @method static QueryBuilder joinWhere($table, $first_column, $operator, $value, $type = 'INNER') * @method static QueryBuilder leftJoin($table, $first_column, $operator = null, $second_column = null) * @method static QueryBuilder rightJoin($table, $first_column, $operator = null, $second_column = null) * @method static QueryBuilder fullJoin($table, $first_column, $operator = null, $second_column = null) * @method static QueryBuilder crossJoin($table, $first_column, $operator = null, $second_column = null) + * @method static QueryBuilder onValue($first_column, $operator, $value, $bool = 'AND') + * @method static QueryBuilder orOnValue($first_column, $operator, $value) + * @method static QueryBuilder onRaw($sql, array $bindings = [], $bool = 'AND') + * @method static QueryBuilder orOnRaw($sql, array $bindings = []) * @method static QueryBuilder orderBy($column) * @method static QueryBuilder orderByRaw($query, $bindings = []) * @method static QueryBuilder asc() diff --git a/src/Query/Grammar.php b/src/Query/Grammar.php index c5b2479..f2d0be9 100644 --- a/src/Query/Grammar.php +++ b/src/Query/Grammar.php @@ -33,7 +33,7 @@ public function compileSelect(QueryBuilder $query): string }, $query->select); $sql = 'SELECT ' . ($query->isDistinct() ? 'DISTINCT ' : '') . implode(',', $columns); $sql .= $this->prepareRawSelect($query); - $sql .= ' FROM ' . $query->getTable(); + $sql .= ' FROM ' . Identifier::quoteQualified($query->getTable()); $sql .= $this->getFrom($query); $sql .= $this->getJoin($query); $sql .= $this->getWhere($query); @@ -75,8 +75,12 @@ public function getJoin(QueryBuilder $query) } foreach ($joins as $join) { - $sql .= ' ' . $join['type'] . ' JOIN ' . $join['table'] - . ' ON ' . $this->processConditions($query, $join['on']); + $sql .= ' ' . JoinType::normalize($join['type']) . ' JOIN ' + . Identifier::quoteQualified($join['table']); + if ($join['userAlias'] !== null) { + $sql .= ' AS ' . Identifier::quoteAlias($join['userAlias']); + } + $sql .= ' ON ' . $this->processConditions($query, $join['on']); } return $sql; @@ -205,7 +209,7 @@ private function getFrom(QueryBuilder $query) { $alias = $query->getFromAlias(); - return isset($alias) ? " {$alias}" : null; + return isset($alias) ? ' ' . Identifier::quoteAlias($alias) : null; } /** @@ -282,13 +286,13 @@ private function prepareOperatorForWhere($clause) } if (isset($clause['operator'])) { - $sql .= ' ' . $clause['operator']; + $sql .= ' ' . SqlOperator::normalize($clause['operator']); } elseif (\is_array($clause['value'])) { - $sql .= ' IN '; + $sql .= ' ' . SqlOperator::normalize('IN') . ' '; } elseif (\is_null($clause['value'])) { - $sql = ' IS NULL'; + $sql = ' ' . SqlOperator::normalize('IS NULL'); } else { - $sql .= ' = '; + $sql .= ' ' . SqlOperator::normalize('=') . ' '; } return $sql; @@ -305,11 +309,14 @@ private function prepareValueForWhere(QueryBuilder $query, $clause) { $sql = ''; if (isset($clause['secondColumn'])) { - if (preg_match('/^[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)?$/', $clause['secondColumn'])) { - return ' ' . $query->renderIdentifier($clause['secondColumn']); - } + return ' ' . $query->renderIdentifier($clause['secondColumn']); + } - return ' ' . $clause['secondColumn']; + if (isset($clause['between'])) { + [$start, $end] = $clause['between']; + $query->addBindings([$start, $end]); + + return ' ' . $query->getValueType($start) . ' AND ' . $query->getValueType($end); } if (!isset($clause['value'])) { @@ -324,11 +331,6 @@ private function prepareValueForWhere(QueryBuilder $query, $clause) } $sql = rtrim($sql, ',') . ')'; - } elseif (isset($clause['operator']) && strpos($clause['operator'], 'IS') !== false) { - $sql .= ' ' . $clause['value']; - } elseif (isset($clause['operator']) && strtoupper($clause['operator']) === 'LIKE') { - $sql .= ' %s'; - $query->addBindings($clause['value']); } elseif (!\is_null($clause['value'])) { $sql .= ' ' . $query->getValueType($clause['value']); $query->addBindings($clause['value']); diff --git a/src/Query/JoinType.php b/src/Query/JoinType.php new file mode 100644 index 0000000..040cf2b --- /dev/null +++ b/src/Query/JoinType.php @@ -0,0 +1,34 @@ +', + '>', + '<', + '>=', + '<=', + 'LIKE', + 'NOT LIKE', + 'IN', + 'NOT IN', + 'IS NULL', + 'IS NOT NULL', + 'BETWEEN', + ]; + + public static function normalize($operator): string + { + if (!\is_string($operator)) { + throw new RuntimeException('Invalid SQL operator.'); + } + + $normalized = strtoupper($operator); + if (!\in_array($normalized, self::ALLOWED, true)) { + throw new RuntimeException('Invalid SQL operator.'); + } + + return $normalized; + } +} diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index cc127a7..22f3bc7 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -5,6 +5,8 @@ use BitApps\WPDatabase\Concerns\QueriesRelationships; use BitApps\WPDatabase\Query\Grammar; use BitApps\WPDatabase\Query\Identifier; +use BitApps\WPDatabase\Query\JoinType; +use BitApps\WPDatabase\Query\SqlOperator; use Closure; use DateTime; @@ -145,6 +147,11 @@ public function withCast(array $casts) */ public function from($_from) { + if (!\is_string($_from)) { + throw new RuntimeException('Invalid SQL table alias.'); + } + + Identifier::assertSimple($_from); $this->_from = $_from; return $this; @@ -362,6 +369,11 @@ public function all($columns = ['*']) return $this->get($columns); } + /** + * Renders a select/aggregate column in this query's table context. + * + * @return string + */ public function prepareColumnName(string $column) { return $this->renderIdentifier($column, true, true); @@ -615,6 +627,8 @@ public function orWhereRaw($sql, $bindings = []) */ public function whereIn($column, $value) { + $this->assertSafeIdentifier($column); + if (\is_array($value)) { if ($value === []) { return $this->whereRaw('0 = 1'); @@ -641,6 +655,8 @@ public function whereIn($column, $value) */ public function whereNull($column) { + $this->assertSafeIdentifier($column); + $this->where[] = [ 'column' => $column, 'operator' => 'IS NULL', @@ -658,6 +674,8 @@ public function whereNull($column) */ public function whereNotNull($column) { + $this->assertSafeIdentifier($column); + $this->where[] = [ 'column' => $column, 'operator' => 'IS NOT NULL', @@ -701,10 +719,12 @@ public function onlyTrashed() */ public function whereBetween($column, $start, $end) { + $this->assertSafeIdentifier($column); + $this->where[] = [ - 'raw' => ' (' . $column . ' BETWEEN ' . $this->getValueType($start) - . ' AND ' . $this->getValueType($end) . ')', - 'bindings' => [$start, $end], + 'column' => $column, + 'operator' => SqlOperator::normalize('BETWEEN'), + 'between' => [$start, $end], ]; return $this; @@ -721,10 +741,12 @@ public function whereBetween($column, $start, $end) */ public function orWhereBetween($column, $start, $end) { + $this->assertSafeIdentifier($column); + $this->where[] = [ - 'raw' => ' (' . $column . ' BETWEEN ' . $this->getValueType($start) - . ' AND ' . $this->getValueType($end) . ')', - 'bindings' => [$start, $end], + 'column' => $column, + 'operator' => SqlOperator::normalize('BETWEEN'), + 'between' => [$start, $end], 'bool' => 'OR', ]; @@ -820,19 +842,77 @@ public function orHaving(...$params) */ public function join($table, $firstColumn, $operator = null, $secondColumn = null, $type = 'INNER') { - $parts = preg_split('/\s+as\s+/i', trim($table), 2); - $rawTable = $parts[0]; - $alias = isset($parts[1]) ? $parts[1] : null; + [$rawTable, $alias, $prefixedTable, $reference] = $this->parseJoinTable($table); + + return $this->storeJoin( + $rawTable, + $alias, + $prefixedTable, + $reference, + $this->prepareOnColumn($reference, $firstColumn, $operator, $secondColumn), + $type + ); + } + + /** + * Joins a table using a bound scalar value as the right-hand operand. + * + * @return $this + */ + public function joinWhere($table, $firstColumn, $operator, $value, $type = 'INNER') + { + [$rawTable, $alias, $prefixedTable, $reference] = $this->parseJoinTable($table); + + return $this->storeJoin( + $rawTable, + $alias, + $prefixedTable, + $reference, + $this->prepareOnValue($firstColumn, $operator, $value), + $type + ); + } + + /** + * Parses a structured join table declaration with an optional explicit + * `AS` alias. Both identifiers remain logical state until compilation. + * + * @return array{string, null|string, string, string} + */ + private function parseJoinTable($table) + { + if (!\is_string($table) + || !preg_match('/^([A-Za-z_][A-Za-z0-9_]*)(?:\s+AS\s+([A-Za-z_][A-Za-z0-9_]*))?$/i', $table, $matches) + ) { + throw new RuntimeException('Invalid SQL join table declaration.'); + } + + $rawTable = $matches[1]; + $alias = isset($matches[2]) ? $matches[2] : null; $prefixedTable = $this->_model->getTablePrefix() . $rawTable; $reference = $alias !== null ? $alias : $prefixedTable; - $tableSql = $alias !== null ? $prefixedTable . ' as ' . $alias : $prefixedTable; - $on[] = $this->prepareOn($reference, $firstColumn, $operator, $secondColumn, 'AND'); + Identifier::assertSimple($rawTable); + Identifier::assertSimple($prefixedTable); + if ($alias !== null) { + Identifier::assertSimple($alias); + } + + return [$rawTable, $alias, $prefixedTable, $reference]; + } + + /** + * Stores one validated join and its first ON condition. + * + * @return $this + */ + private function storeJoin($rawTable, $alias, $prefixedTable, $reference, array $on, $type) + { $this->joins[] = [ - 'table' => $tableSql, + 'table' => $prefixedTable, 'alias' => $reference, - 'on' => $on, - 'type' => $type, + 'on' => [$on], + 'type' => JoinType::normalize($type), 'raw' => $rawTable, 'prefixed' => $prefixedTable, 'userAlias' => $alias, @@ -1044,11 +1124,11 @@ public function on($firstColumn, $operator = null, $secondColumn = null, $bool = { $joinIndex = (\count($this->joins) - 1); if ($joinIndex < 0) { - $joinIndex = 0; + throw new RuntimeException('Cannot add an ON clause before a join.'); } $table = $this->joins[$joinIndex]['alias']; - $this->joins[$joinIndex]['on'][] = $this->prepareOn($table, $firstColumn, $operator, $secondColumn, $bool); + $this->joins[$joinIndex]['on'][] = $this->prepareOnColumn($table, $firstColumn, $operator, $secondColumn, $bool); return $this; } @@ -1067,6 +1147,69 @@ public function orOn($firstColumn, $operator = null, $secondColumn = null) return $this->on($firstColumn, $operator, $secondColumn, 'OR'); } + /** + * Adds an ON condition whose right-hand operand is a bound scalar value. + * + * @return $this + */ + public function onValue($firstColumn, $operator, $value, $bool = 'AND') + { + $joinIndex = (\count($this->joins) - 1); + if ($joinIndex < 0) { + throw new RuntimeException('Cannot add an ON clause before a join.'); + } + + $this->joins[$joinIndex]['on'][] = $this->prepareOnValue($firstColumn, $operator, $value, $bool); + + return $this; + } + + /** + * Adds an OR ON condition whose right-hand operand is a bound scalar value. + * + * @return $this + */ + public function orOnValue($firstColumn, $operator, $value) + { + return $this->onValue($firstColumn, $operator, $value, 'OR'); + } + + /** + * Adds an explicitly raw ON condition. The SQL must be developer-authored; + * dynamic values belong in placeholders and $bindings. + * + * @return $this + */ + public function onRaw($sql, array $bindings = [], $bool = 'AND') + { + $joinIndex = (\count($this->joins) - 1); + if ($joinIndex < 0) { + throw new RuntimeException('Cannot add an ON clause before a join.'); + } + + if (!\is_string($sql) || $sql === '') { + throw new RuntimeException('Invalid raw ON expression.'); + } + + $this->joins[$joinIndex]['on'][] = [ + 'raw' => $sql, + 'bindings' => $bindings, + 'bool' => $this->normalizeBoolean($bool), + ]; + + return $this; + } + + /** + * Adds an explicitly raw OR ON condition. + * + * @return $this + */ + public function orOnRaw($sql, array $bindings = []) + { + return $this->onRaw($sql, $bindings, 'OR'); + } + /** * Sets order by * @@ -1642,6 +1785,8 @@ protected function prepareConditional($params, $bool = 'AND', $type = 'where') return $conditions; } + $bool = $this->normalizeBoolean($bool); + if (\func_num_args() == 1 && \is_array($params[0])) { foreach ($params[0] as $clause) { if ($type === 'where') { @@ -1658,7 +1803,7 @@ protected function prepareConditional($params, $bool = 'AND', $type = 'where') \call_user_func($params[0], $nestedQuery); $conditions['query'] = $nestedQuery; if (isset($params[1])) { - $conditions['bool'] = $params[1]; + $conditions['bool'] = $this->normalizeBoolean($params[1]); } } elseif ($noOfParams == 2) { $conditions['column'] = $params[0]; @@ -1671,7 +1816,7 @@ protected function prepareConditional($params, $bool = 'AND', $type = 'where') $conditions['column'] = $params[0]; $conditions['operator'] = $params[1]; $conditions[$type === 'where' ? 'value' : 'secondColumn'] = $params[2]; - $conditions['bool'] = $params[3]; + $conditions['bool'] = $this->normalizeBoolean($params[3]); } return $this->normalizeConditions($conditions, $type); @@ -1723,23 +1868,48 @@ protected function prepareHaving($params, $bool = 'AND') * * @return $this */ - protected function prepareOn($table, $column, $operator, $secondColumn, $bool = 'AND') + protected function prepareOnColumn($table, $column, $operator, $secondColumn, $bool = 'AND') { if (\is_null($operator) && \is_null($secondColumn)) { $secondColumn = $column; $operator = '='; } - // Qualify only a bare column identifier; leave constants, quoted values - // and function calls (10, 'active', NOW()) untouched, and dotted names - // that are already qualified. - if (!\is_null($secondColumn) && preg_match('/^[A-Za-z_][A-Za-z0-9_]*$/', $secondColumn)) { + $this->assertSafeIdentifier($column); + $this->assertSafeIdentifier($secondColumn); + $operator = SqlOperator::normalize($operator); + $bool = $this->normalizeBoolean($bool); + + if (strpos($secondColumn, '.') === false) { $secondColumn = $table . '.' . $secondColumn; } return compact('column', 'operator', 'secondColumn', 'bool'); } + /** + * Prepares a join condition with a bound right-hand value. + * + * @return array + */ + protected function prepareOnValue($column, $operator, $value, $bool = 'AND') + { + $this->assertSafeIdentifier($column); + if (\is_array($value) || \is_object($value) || \is_resource($value)) { + throw new RuntimeException('Join values must be scalar or null.'); + } + + $operator = SqlOperator::normalize($operator); + $bool = $this->normalizeBoolean($bool); + if (\is_null($value)) { + $operator = $this->nullOperator($operator); + + return compact('column', 'operator', 'bool'); + } + + return compact('column', 'operator', 'value', 'bool'); + } + /** * Helper function, to get current timestamp * @@ -1840,6 +2010,18 @@ private function isListOfRows($attributes) */ private function normalizeConditions(array $conditions, $type) { + if (isset($conditions['bool'])) { + $conditions['bool'] = $this->normalizeBoolean($conditions['bool']); + } + + if (isset($conditions['column'])) { + $this->assertSafeIdentifier($conditions['column']); + } + + if (isset($conditions['operator'])) { + $conditions['operator'] = SqlOperator::normalize($conditions['operator']); + } + if ($type !== 'where' || !\array_key_exists('value', $conditions)) { return $conditions; } @@ -1882,16 +2064,35 @@ private function normalizeConditions(array $conditions, $type) */ private function nullOperator($operator) { - $negations = ['!=', '<>', 'NOT', 'IS NOT', 'IS NOT NULL']; + $negations = ['!=', '<>', 'NOT LIKE', 'NOT IN', 'IS NOT NULL']; return \in_array($operator, $negations, true) ? 'IS NOT NULL' : 'IS NULL'; } /** - * Guards an ORDER BY / GROUP BY column against injection: only a plain, - * qualified (table.column) or back-ticked identifier is accepted. Raw - * expressions must go through orderByRaw(). Valid identifiers are not - * re-rendered, so the emitted SQL stays byte-identical. + * Normalizes a structured boolean connector without accepting whitespace + * or comments around it. + * + * @return string + */ + private function normalizeBoolean($bool) + { + if (!\is_string($bool)) { + throw new RuntimeException('Invalid SQL boolean connector.'); + } + + $normalized = strtoupper($bool); + if (!\in_array($normalized, ['AND', 'OR'], true)) { + throw new RuntimeException('Invalid SQL boolean connector.'); + } + + return $normalized; + } + + /** + * Guards a structured column boundary. Only plain or qualified logical + * identifiers are accepted; raw expressions belong in an explicitly raw + * API and contextual qualification remains deferred until compilation. * * @param mixed $column * diff --git a/tests/BelongsToManyPivotTest.php b/tests/BelongsToManyPivotTest.php index 5e089c0..22f38c6 100644 --- a/tests/BelongsToManyPivotTest.php +++ b/tests/BelongsToManyPivotTest.php @@ -64,18 +64,18 @@ public function testEagerSqlShape(): void $this->assertStringContainsString('SELECT `wp_roles`.*', $sql); $this->assertStringContainsString('wp_role_user.member_id as `pivot_member_id`', $sql); - $this->assertStringContainsString('INNER JOIN wp_role_user', $sql); + $this->assertStringContainsString('INNER JOIN `wp_role_user`', $sql); $this->assertStringContainsString('`wp_role_user`.`role_id` = `wp_roles`.`id`', $sql); // Inner fragment without a leading `WHERE ` boundary: the grammar emits `WHERE ` (double space). $this->assertStringContainsString( - 'wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM wp_members) AS subquery )', + 'wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM `wp_members`) AS subquery )', $sql ); // Exact pin (absorbs the double-space WHERE/ON grammar artifacts). $expected = 'SELECT `wp_roles`.*, wp_role_user.member_id as `pivot_member_id`' - . ' FROM wp_roles INNER JOIN wp_role_user ON `wp_role_user`.`role_id` = `wp_roles`.`id`' - . ' WHERE wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM wp_members) AS subquery )'; + . ' FROM `wp_roles` INNER JOIN `wp_role_user` ON `wp_role_user`.`role_id` = `wp_roles`.`id`' + . ' WHERE wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM `wp_members`) AS subquery )'; $this->assertSame($expected, $sql); } @@ -93,7 +93,7 @@ public function testLazyAccessEmitsSingleValuePredicate(): void // Exact pin: single-value predicate, double-space WHERE/ON/`=` grammar artifacts. $expected = 'SELECT `wp_roles`.*, wp_role_user.member_id as `pivot_member_id`' - . ' FROM wp_roles INNER JOIN wp_role_user ON `wp_role_user`.`role_id` = `wp_roles`.`id`' + . ' FROM `wp_roles` INNER JOIN `wp_role_user` ON `wp_role_user`.`role_id` = `wp_roles`.`id`' . ' WHERE `wp_role_user`.`member_id` = 1'; $this->assertSame($expected, $sql); } diff --git a/tests/BuilderEntryTest.php b/tests/BuilderEntryTest.php index e7af10d..707194e 100644 --- a/tests/BuilderEntryTest.php +++ b/tests/BuilderEntryTest.php @@ -27,7 +27,7 @@ public function testQueryIsChainable(): void { $sql = User::query()->where('id', 1)->toSql(); - $this->assertStringContainsString('FROM wp_users', $sql); + $this->assertStringContainsString('FROM `wp_users`', $sql); $this->assertStringContainsString('WHERE', $sql); } diff --git a/tests/EagerLoadIntegrationTest.php b/tests/EagerLoadIntegrationTest.php index ac99069..e3e316d 100644 --- a/tests/EagerLoadIntegrationTest.php +++ b/tests/EagerLoadIntegrationTest.php @@ -96,7 +96,7 @@ public function testEagerLoadKeySubqueryIgnoresParentSelectRaw(): void $this->assertStringNotContainsString('CONCAT', $postsSql); $this->assertStringContainsString( - 'IN ( SELECT * FROM (SELECT `wp_users`.`id` FROM wp_users', + 'IN ( SELECT * FROM (SELECT `wp_users`.`id` FROM `wp_users`', $postsSql ); } diff --git a/tests/IdentifierQualificationRegressionTest.php b/tests/IdentifierQualificationRegressionTest.php index a04328b..d0f6645 100644 --- a/tests/IdentifierQualificationRegressionTest.php +++ b/tests/IdentifierQualificationRegressionTest.php @@ -16,7 +16,7 @@ final class IdentifierQualificationRegressionTest extends TestCase public function testBareColumnUsesPhysicalModelTable(): void { $this->assertSame( - 'SELECT `wp_users`.`id` FROM wp_users', + 'SELECT `wp_users`.`id` FROM `wp_users`', (new User())->select('id')->toSql() ); } @@ -24,7 +24,7 @@ public function testBareColumnUsesPhysicalModelTable(): void public function testLogicalModelQualifierMapsToPhysicalTable(): void { $this->assertSame( - 'SELECT `wp_users`.`id` FROM wp_users', + 'SELECT `wp_users`.`id` FROM `wp_users`', (new User())->select('users.id')->toSql() ); } @@ -32,7 +32,7 @@ public function testLogicalModelQualifierMapsToPhysicalTable(): void public function testPhysicalModelQualifierIsRecognizedAndQuoted(): void { $this->assertSame( - 'SELECT `wp_users`.`id` FROM wp_users', + 'SELECT `wp_users`.`id` FROM `wp_users`', (new User())->select('wp_users.id')->toSql() ); } @@ -40,7 +40,7 @@ public function testPhysicalModelQualifierIsRecognizedAndQuoted(): void public function testJoinAliasQualifierIsPreservedAndQuoted(): void { $this->assertSame( - 'SELECT `p`.`id` FROM wp_users INNER JOIN wp_posts as p' + 'SELECT `p`.`id` FROM `wp_users` INNER JOIN `wp_posts` AS `p`' . ' ON `wp_users`.`user_id` = `p`.`id`', (new User())->join('posts as p', 'user_id', '=', 'id')->select('p.id')->toSql() ); @@ -49,7 +49,7 @@ public function testJoinAliasQualifierIsPreservedAndQuoted(): void public function testLogicalJoinedTableMapsWhenSelectPrecedesJoin(): void { $this->assertSame( - 'SELECT `wp_posts`.`title` FROM wp_users INNER JOIN wp_posts' + 'SELECT `wp_posts`.`title` FROM `wp_users` INNER JOIN `wp_posts`' . ' ON `wp_users`.`user_id` = `wp_posts`.`id`', (new User())->select('posts.title')->join('posts', 'user_id', '=', 'id')->toSql() ); @@ -58,7 +58,7 @@ public function testLogicalJoinedTableMapsWhenSelectPrecedesJoin(): void public function testQualifiedWildcardResolvesOnlyAtSelectBoundary(): void { $this->assertSame( - 'SELECT `wp_users`.* FROM wp_users', + 'SELECT `wp_users`.* FROM `wp_users`', (new User())->select('users.*')->toSql() ); } @@ -66,7 +66,7 @@ public function testQualifiedWildcardResolvesOnlyAtSelectBoundary(): void public function testExplicitColumnAliasValidatesAndQuotesBothSides(): void { $this->assertSame( - 'SELECT `wp_users`.`id` AS `user_id` FROM wp_users', + 'SELECT `wp_users`.`id` AS `user_id` FROM `wp_users`', (new User())->select('id AS user_id')->toSql() ); } @@ -74,7 +74,7 @@ public function testExplicitColumnAliasValidatesAndQuotesBothSides(): void public function testFromAliasQualifierIsRecognizedAndQuoted(): void { $this->assertSame( - 'SELECT `u`.`id` FROM wp_users u', + 'SELECT `u`.`id` FROM `wp_users` `u`', (new User())->from('u')->select('u.id')->toSql() ); } diff --git a/tests/Query/GrammarTest.php b/tests/Query/GrammarTest.php index bd00378..609d4bf 100644 --- a/tests/Query/GrammarTest.php +++ b/tests/Query/GrammarTest.php @@ -17,7 +17,7 @@ final class GrammarTest extends TestCase public function testSelectAllCompilesToQualifiedStar(): void { $this->assertSame( - 'SELECT `wp_users`.* FROM wp_users', + 'SELECT `wp_users`.* FROM `wp_users`', User::select('*')->toSql() ); } @@ -25,7 +25,7 @@ public function testSelectAllCompilesToQualifiedStar(): void public function testSelectSpecificColumns(): void { $this->assertSame( - 'SELECT `wp_users`.`id`,`wp_users`.`user_login` FROM wp_users', + 'SELECT `wp_users`.`id`,`wp_users`.`user_login` FROM `wp_users`', (new User())->select('id', 'user_login')->toSql() ); } @@ -33,7 +33,7 @@ public function testSelectSpecificColumns(): void public function testWhereEquals(): void { $this->assertSame( - 'SELECT FROM wp_users WHERE `wp_users`.`id` = %d', + 'SELECT FROM `wp_users` WHERE `wp_users`.`id` = %d', (new User())->where('id', 5)->toSql() ); } @@ -41,7 +41,7 @@ public function testWhereEquals(): void public function testWhereWithOperator(): void { $this->assertSame( - 'SELECT FROM wp_users WHERE `wp_users`.`id` > %d', + 'SELECT FROM `wp_users` WHERE `wp_users`.`id` > %d', (new User())->where('id', '>', 0)->toSql() ); } @@ -49,7 +49,7 @@ public function testWhereWithOperator(): void public function testOrWhere(): void { $this->assertSame( - 'SELECT FROM wp_users WHERE `wp_users`.`a` = %d OR `wp_users`.`b` = %d', + 'SELECT FROM `wp_users` WHERE `wp_users`.`a` = %d OR `wp_users`.`b` = %d', (new User())->where('a', 1)->orWhere('b', 2)->toSql() ); } @@ -61,7 +61,7 @@ public function testNestedClosureWhereProducesParenthesizedGroup(): void })->toSql(); $this->assertSame( - 'SELECT FROM wp_users WHERE `wp_users`.`a` = %d AND ' + 'SELECT FROM `wp_users` WHERE `wp_users`.`a` = %d AND ' . '( `wp_users`.`b` = %d OR `wp_users`.`c` = %d)', $sql ); @@ -72,7 +72,7 @@ public function testNestedClosureWhereProducesParenthesizedGroup(): void public function testJoin(): void { $this->assertSame( - 'SELECT FROM wp_users INNER JOIN wp_posts ON `wp_users`.`user_id` = `wp_posts`.`id`', + 'SELECT FROM `wp_users` INNER JOIN `wp_posts` ON `wp_users`.`user_id` = `wp_posts`.`id`', (new User())->join('posts', 'user_id', '=', 'id')->toSql() ); } @@ -80,7 +80,7 @@ public function testJoin(): void public function testJoinWithAlias(): void { $sql = (new User())->join('posts as p', 'user_id', '=', 'id')->toSql(); - $this->assertStringContainsString('INNER JOIN wp_posts as p ON', $sql); + $this->assertStringContainsString('INNER JOIN `wp_posts` AS `p` ON', $sql); $this->assertStringContainsString('= `p`.`id`', $sql); } @@ -94,28 +94,30 @@ public function testJoinWithDottedColumnsResolvesKnownTables(): void public function testJoinAliasSplitToleratesExtraWhitespace(): void { $sql = (new User())->join('posts as p', 'user_id', '=', 'id')->toSql(); - $this->assertStringContainsString('INNER JOIN wp_posts as p ON', $sql); + $this->assertStringContainsString('INNER JOIN `wp_posts` AS `p` ON', $sql); $this->assertStringContainsString('= `p`.`id`', $sql); } - public function testJoinOnConstantSecondOperandNotPrefixed(): void + public function testJoinWhereBindsConstantSecondOperand(): void { - $sql = (new User())->join('posts', 'posts.owner_id', '=', '5')->toSql(); - $this->assertStringContainsString('= 5', $sql); - $this->assertStringNotContainsString('.5', $sql); + $query = (new User())->joinWhere('posts', 'posts.owner_id', '=', 5); + + $this->assertStringContainsString('= %d', $query->toSql()); + $this->assertSame([5], $query->getBindings()); } - public function testJoinOnFunctionSecondOperandNotPrefixed(): void + public function testJoinOnFunctionUsesExplicitRawPath(): void { - $sql = (new User())->join('posts', 'posts.created', '<', 'NOW()')->toSql(); - $this->assertStringContainsString('< NOW()', $sql); - $this->assertStringNotContainsString('.NOW', $sql); + $sql = (new User())->join('posts', 'posts.user_id', '=', 'users.id') + ->onRaw('`wp_posts`.`created` < NOW()')->toSql(); + + $this->assertStringContainsString('AND `wp_posts`.`created` < NOW()', $sql); } public function testGroupBy(): void { $this->assertSame( - 'SELECT FROM wp_users GROUP BY `wp_users`.`status`', + 'SELECT FROM `wp_users` GROUP BY `wp_users`.`status`', (new User())->groupBy('status')->toSql() ); } @@ -123,7 +125,7 @@ public function testGroupBy(): void public function testHaving(): void { $this->assertSame( - 'SELECT FROM wp_users GROUP BY `wp_users`.`status` HAVING `wp_users`.`id` > %d', + 'SELECT FROM `wp_users` GROUP BY `wp_users`.`status` HAVING `wp_users`.`id` > %d', (new User())->groupBy('status')->having('id', '>', 1)->toSql() ); } @@ -131,7 +133,7 @@ public function testHaving(): void public function testOrderByDesc(): void { $this->assertSame( - 'SELECT FROM wp_users ORDER BY `wp_users`.`id` DESC', + 'SELECT FROM `wp_users` ORDER BY `wp_users`.`id` DESC', (new User())->orderBy('id')->desc()->toSql() ); } @@ -139,7 +141,7 @@ public function testOrderByDesc(): void public function testLimitAndOffset(): void { $this->assertSame( - 'SELECT FROM wp_users LIMIT 10 OFFSET 20', + 'SELECT FROM `wp_users` LIMIT 10 OFFSET 20', (new User())->take(10)->skip(20)->toSql() ); } @@ -147,7 +149,7 @@ public function testLimitAndOffset(): void public function testLimitWithoutOffsetOmitsOffsetClause(): void { $this->assertSame( - 'SELECT FROM wp_users LIMIT 5', + 'SELECT FROM `wp_users` LIMIT 5', (new User())->take(5)->toSql() ); } @@ -163,7 +165,7 @@ public function testCompileSelectReturnsString(): void public function testDistinctEmitsSelectDistinct(): void { $this->assertSame( - 'SELECT DISTINCT `wp_users`.`id` FROM wp_users', + 'SELECT DISTINCT `wp_users`.`id` FROM `wp_users`', (new User())->select('id')->distinct()->toSql() ); } @@ -171,7 +173,7 @@ public function testDistinctEmitsSelectDistinct(): void public function testWithoutDistinctSelectIsUnchanged(): void { $this->assertSame( - 'SELECT `wp_users`.`id` FROM wp_users', + 'SELECT `wp_users`.`id` FROM `wp_users`', (new User())->select('id')->toSql() ); } diff --git a/tests/QueryFeaturesTest.php b/tests/QueryFeaturesTest.php index 509a5dd..cc444f6 100644 --- a/tests/QueryFeaturesTest.php +++ b/tests/QueryFeaturesTest.php @@ -79,7 +79,7 @@ public function testWhereHasEmitsCorrelatedExistsSubquery(): void $sql = User::whereHas('posts')->toSql(); $this->assertStringContainsString('exists(', $sql); - $this->assertStringContainsString('FROM wp_posts', $sql); + $this->assertStringContainsString('FROM `wp_posts`', $sql); $this->assertStringContainsString('`wp_users`.`id`=`wp_posts`.`user_id`', $sql); } diff --git a/tests/RelationDefinitionRegressionTest.php b/tests/RelationDefinitionRegressionTest.php index 56a91e4..1327796 100644 --- a/tests/RelationDefinitionRegressionTest.php +++ b/tests/RelationDefinitionRegressionTest.php @@ -29,7 +29,7 @@ public function testStaticBuilderChainStillWorks(): void { $sql = User::where('id', '>', 0)->toSql(); - $this->assertStringContainsString('FROM wp_users', $sql); + $this->assertStringContainsString('FROM `wp_users`', $sql); $this->assertStringContainsString('WHERE', $sql); } diff --git a/tests/RelationStaticAccessTest.php b/tests/RelationStaticAccessTest.php index b017182..af2d61e 100644 --- a/tests/RelationStaticAccessTest.php +++ b/tests/RelationStaticAccessTest.php @@ -73,24 +73,24 @@ public function testWithRegistersRelationOnModelStatically(): void public function testWithCountStaticSqlMatchesBaseline(): void { - $expected = 'SELECT `wp_users`.*, (SELECT count(*) FROM wp_posts WHERE ' - . '`wp_users`.`id`=`wp_posts`.`user_id`) as `posts_count` FROM wp_users'; + $expected = 'SELECT `wp_users`.*, (SELECT count(*) FROM `wp_posts` WHERE ' + . '`wp_users`.`id`=`wp_posts`.`user_id`) as `posts_count` FROM `wp_users`'; $this->assertSame($expected, User::withCount('posts')->toSql()); } public function testWhereHasStaticSqlMatchesBaseline(): void { - $expected = 'SELECT FROM wp_users WHERE exists(SELECT `wp_posts`.* FROM ' - . 'wp_posts WHERE `wp_users`.`id`=`wp_posts`.`user_id`)'; + $expected = 'SELECT FROM `wp_users` WHERE exists(SELECT `wp_posts`.* FROM ' + . '`wp_posts` WHERE `wp_users`.`id`=`wp_posts`.`user_id`)'; $this->assertSame($expected, User::whereHas('posts')->toSql()); } public function testWithExistsStaticSqlMatchesBaseline(): void { - $expected = 'SELECT `wp_users`.*, exists(SELECT `wp_posts`.* FROM wp_posts ' - . 'WHERE `wp_users`.`id`=`wp_posts`.`user_id`) as `posts_exists` FROM wp_users'; + $expected = 'SELECT `wp_users`.*, exists(SELECT `wp_posts`.* FROM `wp_posts` ' + . 'WHERE `wp_users`.`id`=`wp_posts`.`user_id`) as `posts_exists` FROM `wp_users`'; $this->assertSame($expected, User::withExists('posts')->toSql()); } diff --git a/tests/SelectJoinAggregateEdgeTest.php b/tests/SelectJoinAggregateEdgeTest.php index 584862c..afc222e 100644 --- a/tests/SelectJoinAggregateEdgeTest.php +++ b/tests/SelectJoinAggregateEdgeTest.php @@ -66,7 +66,7 @@ public function testSelectThenSelectRawJoinedByComma(): void public function testEmptySelectEmitsQualifiedNothing(): void { - $this->assertStringContainsString('SELECT FROM wp_users', (new User())->toSql()); + $this->assertStringContainsString('SELECT FROM `wp_users`', (new User())->toSql()); } public function testPrepareColumnNameStarStaysQualifiedStar(): void @@ -78,9 +78,9 @@ public function testPrepareColumnNameStarStaysQualifiedStar(): void public function testRightFullCrossJoinKeywords(): void { - $this->assertStringContainsString('RIGHT JOIN wp_posts', (new User())->rightJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); - $this->assertStringContainsString('FULL JOIN wp_posts', (new User())->fullJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); - $this->assertStringContainsString('CROSS JOIN wp_posts', (new User())->crossJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); + $this->assertStringContainsString('RIGHT JOIN `wp_posts`', (new User())->rightJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); + $this->assertStringContainsString('FULL JOIN `wp_posts`', (new User())->fullJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); + $this->assertStringContainsString('CROSS JOIN `wp_posts`', (new User())->crossJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); } public function testOnAndOrOnAppendToSameJoin(): void @@ -97,7 +97,7 @@ public function testJoinOnCustomPrefixModelQualifiesBaseColumnWithFullPrefix(): { $sql = (new PrefixedModel())->join('gadgets', 'gid', '=', 'wid')->toSql(); - $this->assertStringContainsString('INNER JOIN wp_crm_gadgets', $sql); + $this->assertStringContainsString('INNER JOIN `wp_crm_gadgets`', $sql); $this->assertStringContainsString('`wp_crm_widgets`.`gid` = `wp_crm_gadgets`.`wid`', $sql); } diff --git a/tests/StructuredClauseSafetyTest.php b/tests/StructuredClauseSafetyTest.php new file mode 100644 index 0000000..60e4900 --- /dev/null +++ b/tests/StructuredClauseSafetyTest.php @@ -0,0 +1,291 @@ + ['=', '='], + 'not equal bang' => ['!=', '!='], + 'not equal angle' => ['<>', '<>'], + 'greater than' => ['>', '>'], + 'less than' => ['<', '<'], + 'greater or equal' => ['>=', '>='], + 'less or equal' => ['<=', '<='], + 'like' => ['like', 'LIKE'], + 'not like' => ['not like', 'NOT LIKE'], + 'in' => ['in', 'IN'], + 'not in' => ['not in', 'NOT IN'], + 'is null' => ['is null', 'IS NULL'], + 'is not null' => ['is not null', 'IS NOT NULL'], + 'between' => ['between', 'BETWEEN'], + ]; + } + + #[DataProvider('operatorProvider')] + public function testNormalizesFiniteOperatorSet(string $input, string $expected): void + { + $this->assertSame($expected, SqlOperator::normalize($input)); + } + + public static function invalidOperatorProvider(): array + { + return [ + 'empty' => [''], + 'arbitrary keyword' => ['REGEXP'], + 'comment suffix' => ['= --'], + 'block comment' => ['LIKE/**/'], + 'doubled inner space' => ['NOT LIKE'], + 'leading whitespace' => [' LIKE'], + 'trailing whitespace' => ['LIKE '], + 'newline smuggling' => [">\n="], + 'boolean payload' => ['= OR 1=1'], + ]; + } + + #[DataProvider('invalidOperatorProvider')] + public function testRejectsOperatorsOutsideFiniteSet(string $operator): void + { + $this->expectException(RuntimeException::class); + + SqlOperator::normalize($operator); + } + + public static function joinTypeProvider(): array + { + return [ + 'inner' => ['inner', 'INNER'], + 'left' => ['left', 'LEFT'], + 'right' => ['right', 'RIGHT'], + 'full' => ['full', 'FULL'], + 'cross' => ['cross', 'CROSS'], + ]; + } + + #[DataProvider('joinTypeProvider')] + public function testNormalizesFiniteJoinTypeSet(string $input, string $expected): void + { + $this->assertSame($expected, JoinType::normalize($input)); + } + + public static function invalidJoinTypeProvider(): array + { + return [ + 'empty' => [''], + 'unsupported' => ['NATURAL'], + 'outer smuggling' => ['LEFT OUTER'], + 'comment' => ['LEFT/**/'], + 'leading whitespace' => [' LEFT'], + 'trailing whitespace' => ['LEFT '], + 'payload' => ['LEFT JOIN secrets; --'], + ]; + } + + #[DataProvider('invalidJoinTypeProvider')] + public function testRejectsJoinTypesOutsideFiniteSet(string $type): void + { + $this->expectException(RuntimeException::class); + + JoinType::normalize($type); + } + + public static function hostileStructuredPathProvider(): array + { + return [ + 'from alias' => [static function (): void { + User::query()->from('u; DROP TABLE x')->get(); + }], + 'select column' => [static function (): void { + User::query()->select('id; DROP TABLE x')->get(); + }], + 'addSelect column' => [static function (): void { + User::query()->select('id')->addSelect('a); DROP')->get(); + }], + 'where column' => [static function (): void { + User::query()->where('id; DROP TABLE x', 1)->get(); + }], + 'where operator' => [static function (): void { + User::query()->where('id', '= OR 1=1 --', 1)->get(); + }], + 'orWhere column' => [static function (): void { + User::query()->where('id', 1)->orWhere('a); DROP', 2)->get(); + }], + 'orWhere operator' => [static function (): void { + User::query()->where('id', 1)->orWhere('name', 'LIKE/**/OR', '%x%')->get(); + }], + 'where boolean connector' => [static function (): void { + User::query()->where('id', '=', 1, 'OR 1=1 --')->get(); + }], + 'whereIn column' => [static function (): void { + User::query()->whereIn('id) OR 1=1 --', [1])->get(); + }], + 'whereNull column' => [static function (): void { + User::query()->whereNull('id; DROP TABLE x')->get(); + }], + 'whereNotNull column' => [static function (): void { + User::query()->whereNotNull('a); DROP')->get(); + }], + 'whereBetween column' => [static function (): void { + User::query()->whereBetween('age) OR 1=1 --', 1, 2)->get(); + }], + 'orWhereBetween column' => [static function (): void { + User::query()->orWhereBetween('age; DROP TABLE x', 1, 2)->get(); + }], + 'having column' => [static function (): void { + User::query()->having('count) OR 1=1 --', '>', 1)->get(); + }], + 'having operator' => [static function (): void { + User::query()->having('count', '> OR 1=1', 1)->get(); + }], + 'orHaving column' => [static function (): void { + User::query()->orHaving('count; DROP TABLE x', '>', 1)->get(); + }], + 'orHaving operator' => [static function (): void { + User::query()->orHaving('count', 'NOT LIKE', 'x')->get(); + }], + 'group column reported payload' => [static function (): void { + User::query()->groupBy('a); DROP')->get(); + }], + 'order column reported payload' => [static function (): void { + User::query()->orderBy('id; DROP TABLE x')->get(); + }], + 'aggregate column' => [static function (): void { + User::query()->max('score); DROP TABLE x')->get(); + }], + 'aggregate function' => [static function (): void { + User::query()->aggregate('COUNT(*); DROP TABLE x; --', 'id'); + }], + 'join table' => [static function (): void { + User::query()->join('posts; DROP TABLE x', 'posts.user_id', '=', 'users.id')->get(); + }], + 'join alias' => [static function (): void { + User::query()->join('posts AS p; DROP', 'p.user_id', '=', 'users.id')->get(); + }], + 'join first column' => [static function (): void { + User::query()->join('posts', 'posts.user_id OR 1=1', '=', 'users.id')->get(); + }], + 'join second column' => [static function (): void { + User::query()->join('posts', 'posts.user_id', '=', 'users.id OR 1=1')->get(); + }], + 'join operator' => [static function (): void { + User::query()->join('posts', 'posts.user_id', '= OR 1=1', 'users.id')->get(); + }], + 'join type' => [static function (): void { + User::query()->join('posts', 'posts.user_id', '=', 'users.id', 'LEFT; DROP')->get(); + }], + 'chained on column' => [static function (): void { + User::query()->join('posts', 'posts.user_id', '=', 'users.id') + ->on('posts.state OR 1=1', '=', 'users.state')->get(); + }], + 'chained on operator' => [static function (): void { + User::query()->join('posts', 'posts.user_id', '=', 'users.id') + ->on('posts.state', '= --', 'users.state')->get(); + }], + 'chained on boolean connector' => [static function (): void { + User::query()->join('posts', 'posts.user_id', '=', 'users.id') + ->on('posts.state', '=', 'users.state', 'OR 1=1 --')->get(); + }], + 'chained orOn column' => [static function (): void { + User::query()->join('posts', 'posts.user_id', '=', 'users.id') + ->orOn('posts.state', '=', 'users.state; DROP')->get(); + }], + 'chained orOn operator' => [static function (): void { + User::query()->join('posts', 'posts.user_id', '=', 'users.id') + ->orOn('posts.state', '!= OR 1=1', 'users.state')->get(); + }], + ]; + } + + #[DataProvider('hostileStructuredPathProvider')] + public function testStructuredPathsRejectHostileTokensBeforeExecution(callable $attempt): void + { + try { + $attempt(); + $this->fail('Expected the structured query path to reject hostile SQL structure.'); + } catch (RuntimeException $exception) { + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + } + + public function testStructuredBetweenClausesRenderIdentifiersAndBindBounds(): void + { + $query = User::query()->whereBetween('age', 18, 65)->orWhereBetween('score', 1.5, 9.5); + + $this->assertStringContainsString('`wp_users`.`age` BETWEEN %d AND %d', $query->toSql()); + $this->assertStringContainsString('OR `wp_users`.`score` BETWEEN %f AND %f', $query->toSql()); + $this->assertSame([18, 65, 1.5, 9.5], $query->getBindings()); + } + + public function testJoinValuePathsBindConstants(): void + { + $query = User::query() + ->joinWhere('posts', 'posts.status', '=', 'active') + ->onValue('posts.score', '>=', 10) + ->orOnValue('posts.kind', '=', 'featured'); + + $sql = $query->toSql(); + + $this->assertStringContainsString('`wp_posts`.`status` = %s', $sql); + $this->assertStringContainsString('AND `wp_posts`.`score` >= %d', $sql); + $this->assertStringContainsString('OR `wp_posts`.`kind` = %s', $sql); + $this->assertSame(['active', 10, 'featured'], $query->getBindings()); + } + + public function testExplicitRawJoinPathsDoNotGuessExpressions(): void + { + $query = User::query() + ->join('posts', 'posts.user_id', '=', 'users.id') + ->onRaw('`wp_posts`.`created_at` < NOW()') + ->orOnRaw('`wp_posts`.`updated_at` > DATE_SUB(NOW(), INTERVAL %d DAY)', [7]); + $sql = $query->toSql(); + + $this->assertStringContainsString('AND `wp_posts`.`created_at` < NOW()', $sql); + $this->assertStringContainsString('OR `wp_posts`.`updated_at` > DATE_SUB(NOW(), INTERVAL %d DAY)', $sql); + $this->assertSame([7], $query->getBindings()); + } + + public static function guessedJoinOperandProvider(): array + { + return [ + 'numeric constant' => ['5'], + 'quoted constant' => ["'active'"], + 'function call' => ['NOW()'], + ]; + } + + #[DataProvider('guessedJoinOperandProvider')] + public function testOrdinaryJoinRejectsNonColumnSecondOperands(string $operand): void + { + $this->expectException(RuntimeException::class); + + User::query()->join('posts', 'posts.value', '=', $operand); + } + + public function testStructuredTablesAndAliasesAreQuoted(): void + { + $this->assertSame( + 'SELECT `u`.`id` FROM `wp_users` `u` INNER JOIN `wp_posts` AS `p`' + . ' ON `p`.`user_id` = `u`.`id`', + User::query()->from('u')->join('posts AS p', 'p.user_id', '=', 'u.id')->select('u.id')->toSql() + ); + } +} diff --git a/tests/TablePrefixTest.php b/tests/TablePrefixTest.php index 40dfb8f..cd3d573 100644 --- a/tests/TablePrefixTest.php +++ b/tests/TablePrefixTest.php @@ -34,6 +34,6 @@ public function testJoinOnCustomPrefixModelKeepsWpPrefix(): void ->join('gadgets', 'gadgets.widget_id', '=', 'widgets.id') ->toSql(); - $this->assertStringContainsString('JOIN wp_crm_gadgets', $sql); + $this->assertStringContainsString('JOIN `wp_crm_gadgets`', $sql); } } diff --git a/tests/WhereClauseEdgeTest.php b/tests/WhereClauseEdgeTest.php index 9a7cdab..24e8451 100644 --- a/tests/WhereClauseEdgeTest.php +++ b/tests/WhereClauseEdgeTest.php @@ -104,11 +104,11 @@ public function testLikeUppercaseAndNotLike(): void public function testWhereBetweenAndOrWhereBetween(): void { $qb = (new User())->whereBetween('age', 18, 65); - $this->assertStringContainsString('(age BETWEEN %d AND %d)', $qb->toSql()); + $this->assertStringContainsString('`wp_users`.`age` BETWEEN %d AND %d', $qb->toSql()); $this->assertSame([18, 65], $qb->getBindings()); $qb2 = (new User())->where('id', 1)->orWhereBetween('age', 18, 65); - $this->assertStringContainsString('OR (age BETWEEN %d AND %d)', $qb2->toSql()); + $this->assertStringContainsString('OR `wp_users`.`age` BETWEEN %d AND %d', $qb2->toSql()); $this->assertSame([1, 18, 65], $qb2->getBindings()); } From 296cdb614d1bfdf0245a4fd728f7184160dcfa64 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 01:38:29 +0600 Subject: [PATCH 03/11] fix(query): enforce SQL clause contexts Compile UPDATE bindings in placeholder order, constrain operators by clause shape, reject unsupported FULL joins, and qualify bare columns through active base aliases. --- docs/breaking-changes.md | 2 + docs/usage.md | 5 +- src/Model.php | 1 - src/Query/Grammar.php | 20 ++-- src/Query/JoinType.php | 1 - src/Query/SqlOperator.php | 37 +++++-- src/QueryBuilder.php | 31 ++++-- tests/SelectJoinAggregateEdgeTest.php | 3 +- tests/StructuredClauseSafetyTest.php | 144 ++++++++++++++++++++++++-- 9 files changed, 206 insertions(+), 38 deletions(-) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index c06d546..9503ae5 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -381,6 +381,8 @@ Migrate constants to the bound-value APIs and expressions to the explicit raw AP `whereBetween()` and `orWhereBetween()` now retain structured column state and bind both bounds; hostile column text can no longer enter their SQL fragment. +`FULL JOIN` is rejected because MySQL does not support it; use supported +`INNER`, `LEFT`, `RIGHT`, or `CROSS` joins with a valid ON condition. --- diff --git a/docs/usage.md b/docs/usage.md index 011669b..3aa33ba 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -272,7 +272,7 @@ Contact::query() ->join('orders', 'orders.contact_id', '=', 'contacts.id') ->leftJoin('notes', 'notes.contact_id', '=', 'contacts.id') ->get(); -// also: rightJoin(), fullJoin(), crossJoin(), on(), orOn() +// also: rightJoin(), crossJoin(), on(), orOn() // Compare a join column with bound values (never interpolated as SQL). Contact::query() @@ -301,6 +301,9 @@ join alias and `from('alias')` for the base-table alias. `join()`, `on()`, and `orOn()` are column-to-column APIs. They no longer guess whether the right operand is a number, quoted literal, or SQL function. Move constants to `joinWhere()` / `onValue()` and reviewed expressions to `onRaw()`. +`crossJoin()` keeps this builder's four-operand conditional form and emits +`CROSS JOIN ... ON ...`, which MySQL accepts as an inner-join synonym; it does +not create an unqualified Cartesian product. ### Limit / offset / pagination diff --git a/src/Model.php b/src/Model.php index eb8e9a5..276fb91 100644 --- a/src/Model.php +++ b/src/Model.php @@ -59,7 +59,6 @@ * @method static QueryBuilder joinWhere($table, $first_column, $operator, $value, $type = 'INNER') * @method static QueryBuilder leftJoin($table, $first_column, $operator = null, $second_column = null) * @method static QueryBuilder rightJoin($table, $first_column, $operator = null, $second_column = null) - * @method static QueryBuilder fullJoin($table, $first_column, $operator = null, $second_column = null) * @method static QueryBuilder crossJoin($table, $first_column, $operator = null, $second_column = null) * @method static QueryBuilder onValue($first_column, $operator, $value, $bool = 'AND') * @method static QueryBuilder orOnValue($first_column, $operator, $value) diff --git a/src/Query/Grammar.php b/src/Query/Grammar.php index f2d0be9..ab5b942 100644 --- a/src/Query/Grammar.php +++ b/src/Query/Grammar.php @@ -285,14 +285,22 @@ private function prepareOperatorForWhere($clause) return $sql; } - if (isset($clause['operator'])) { - $sql .= ' ' . SqlOperator::normalize($clause['operator']); + if (isset($clause['between'])) { + $sql .= ' ' . SqlOperator::normalizeRange($clause['operator']); + } elseif (isset($clause['secondColumn'])) { + $sql .= ' ' . SqlOperator::normalizeBinary($clause['operator']); + } elseif (!\array_key_exists('value', $clause)) { + $sql .= ' ' . SqlOperator::normalizeUnary($clause['operator']); } elseif (\is_array($clause['value'])) { - $sql .= ' ' . SqlOperator::normalize('IN') . ' '; - } elseif (\is_null($clause['value'])) { - $sql = ' ' . SqlOperator::normalize('IS NULL'); + if (isset($clause['operator'])) { + $sql .= ' ' . SqlOperator::normalizeList($clause['operator']); + } else { + $sql .= ' ' . SqlOperator::normalizeList('IN') . ' '; + } + } elseif (isset($clause['operator'])) { + $sql .= ' ' . SqlOperator::normalizeBinary($clause['operator']); } else { - $sql .= ' ' . SqlOperator::normalize('=') . ' '; + $sql .= ' ' . SqlOperator::normalizeBinary('=') . ' '; } return $sql; diff --git a/src/Query/JoinType.php b/src/Query/JoinType.php index 040cf2b..396f7b7 100644 --- a/src/Query/JoinType.php +++ b/src/Query/JoinType.php @@ -14,7 +14,6 @@ final class JoinType 'INNER', 'LEFT', 'RIGHT', - 'FULL', 'CROSS', ]; diff --git a/src/Query/SqlOperator.php b/src/Query/SqlOperator.php index 64e53e0..0d8579a 100644 --- a/src/Query/SqlOperator.php +++ b/src/Query/SqlOperator.php @@ -10,7 +10,7 @@ final class SqlOperator { - private const ALLOWED = [ + private const BINARY = [ '=', '!=', '<>', @@ -20,21 +20,42 @@ final class SqlOperator '<=', 'LIKE', 'NOT LIKE', - 'IN', - 'NOT IN', - 'IS NULL', - 'IS NOT NULL', - 'BETWEEN', ]; - public static function normalize($operator): string + private const LIST = ['IN', 'NOT IN']; + + private const UNARY = ['IS NULL', 'IS NOT NULL']; + + private const RANGE = ['BETWEEN']; + + public static function normalizeBinary($operator): string + { + return self::normalize($operator, self::BINARY); + } + + public static function normalizeList($operator): string + { + return self::normalize($operator, self::LIST); + } + + public static function normalizeUnary($operator): string + { + return self::normalize($operator, self::UNARY); + } + + public static function normalizeRange($operator): string + { + return self::normalize($operator, self::RANGE); + } + + private static function normalize($operator, array $allowed): string { if (!\is_string($operator)) { throw new RuntimeException('Invalid SQL operator.'); } $normalized = strtoupper($operator); - if (!\in_array($normalized, self::ALLOWED, true)) { + if (!\in_array($normalized, $allowed, true)) { throw new RuntimeException('Invalid SQL operator.'); } diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 22f3bc7..4cfb1bd 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -723,7 +723,7 @@ public function whereBetween($column, $start, $end) $this->where[] = [ 'column' => $column, - 'operator' => SqlOperator::normalize('BETWEEN'), + 'operator' => SqlOperator::normalizeRange('BETWEEN'), 'between' => [$start, $end], ]; @@ -745,7 +745,7 @@ public function orWhereBetween($column, $start, $end) $this->where[] = [ 'column' => $column, - 'operator' => SqlOperator::normalize('BETWEEN'), + 'operator' => SqlOperator::normalizeRange('BETWEEN'), 'between' => [$start, $end], 'bool' => 'OR', ]; @@ -1012,7 +1012,9 @@ public function renderIdentifier(string $column, bool $allowWildcard = false, bo Identifier::quoteQualified($column, $allowWildcard); if (\count($segments) === 1) { - return Identifier::quoteQualified($this->table . '.' . $column, $allowWildcard); + $baseReference = $this->_from ?? $this->table; + + return Identifier::quoteQualified($baseReference . '.' . $column, $allowWildcard); } [$qualifier, $name] = $segments; @@ -1877,7 +1879,7 @@ protected function prepareOnColumn($table, $column, $operator, $secondColumn, $b $this->assertSafeIdentifier($column); $this->assertSafeIdentifier($secondColumn); - $operator = SqlOperator::normalize($operator); + $operator = SqlOperator::normalizeBinary($operator); $bool = $this->normalizeBoolean($bool); if (strpos($secondColumn, '.') === false) { @@ -1899,7 +1901,7 @@ protected function prepareOnValue($column, $operator, $value, $bool = 'AND') throw new RuntimeException('Join values must be scalar or null.'); } - $operator = SqlOperator::normalize($operator); + $operator = SqlOperator::normalizeBinary($operator); $bool = $this->normalizeBoolean($bool); if (\is_null($value)) { $operator = $this->nullOperator($operator); @@ -2019,7 +2021,7 @@ private function normalizeConditions(array $conditions, $type) } if (isset($conditions['operator'])) { - $conditions['operator'] = SqlOperator::normalize($conditions['operator']); + $conditions['operator'] = SqlOperator::normalizeBinary($conditions['operator']); } if ($type !== 'where' || !\array_key_exists('value', $conditions)) { @@ -2041,10 +2043,13 @@ private function normalizeConditions(array $conditions, $type) if (\is_null($value)) { if (isset($conditions['operator'])) { - unset($conditions['value']); $conditions['operator'] = $this->nullOperator($conditions['operator']); + } else { + $conditions['operator'] = 'IS NULL'; } + unset($conditions['value']); + return $conditions; } @@ -2101,7 +2106,7 @@ private function normalizeBoolean($bool) private function assertSafeIdentifier($column) { if (!\is_string($column)) { - throw new RuntimeException('Unsafe column passed to order/group by clause.'); + throw new RuntimeException('Invalid structured SQL identifier.'); } Identifier::quoteQualified($column); @@ -2327,16 +2332,20 @@ function ($value, $key) { */ private function prepareUpdate() { + $setBindings = $this->bindings; + $this->bindings = []; + $sql = 'UPDATE ' . $this->table; $sql .= $this->grammar()->getJoin($this); $sql .= ' SET '; $columnCount = \count($this->update); foreach ($this->update as $key => $column) { - if (\is_null($this->bindings[$key])) { + $value = $setBindings[$key]; + if (\is_null($value)) { $sql .= $column . ' = NULL'; - unset($this->bindings[$key]); } else { - $sql .= $column . ' = ' . $this->getValueType($this->bindings[$key]); + $sql .= $column . ' = ' . $this->getValueType($value); + $this->addBindings($value); } if ($key < $columnCount - 1) { diff --git a/tests/SelectJoinAggregateEdgeTest.php b/tests/SelectJoinAggregateEdgeTest.php index afc222e..bb1ee27 100644 --- a/tests/SelectJoinAggregateEdgeTest.php +++ b/tests/SelectJoinAggregateEdgeTest.php @@ -76,10 +76,9 @@ public function testPrepareColumnNameStarStaysQualifiedStar(): void // --- Joins --------------------------------------------------------------- - public function testRightFullCrossJoinKeywords(): void + public function testRightAndCrossJoinKeywords(): void { $this->assertStringContainsString('RIGHT JOIN `wp_posts`', (new User())->rightJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); - $this->assertStringContainsString('FULL JOIN `wp_posts`', (new User())->fullJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); $this->assertStringContainsString('CROSS JOIN `wp_posts`', (new User())->crossJoin('posts', 'posts.user_id', '=', 'users.id')->toSql()); } diff --git a/tests/StructuredClauseSafetyTest.php b/tests/StructuredClauseSafetyTest.php index 60e4900..8a33640 100644 --- a/tests/StructuredClauseSafetyTest.php +++ b/tests/StructuredClauseSafetyTest.php @@ -33,18 +33,30 @@ public static function operatorProvider(): array 'less or equal' => ['<=', '<='], 'like' => ['like', 'LIKE'], 'not like' => ['not like', 'NOT LIKE'], - 'in' => ['in', 'IN'], - 'not in' => ['not in', 'NOT IN'], - 'is null' => ['is null', 'IS NULL'], - 'is not null' => ['is not null', 'IS NOT NULL'], - 'between' => ['between', 'BETWEEN'], ]; } #[DataProvider('operatorProvider')] public function testNormalizesFiniteOperatorSet(string $input, string $expected): void { - $this->assertSame($expected, SqlOperator::normalize($input)); + $this->assertSame($expected, SqlOperator::normalizeBinary($input)); + } + + public static function specializedOperatorProvider(): array + { + return [ + 'list' => ['normalizeList', 'in', 'IN'], + 'negative list' => ['normalizeList', 'not in', 'NOT IN'], + 'unary' => ['normalizeUnary', 'is null', 'IS NULL'], + 'negative unary' => ['normalizeUnary', 'is not null', 'IS NOT NULL'], + 'range' => ['normalizeRange', 'between', 'BETWEEN'], + ]; + } + + #[DataProvider('specializedOperatorProvider')] + public function testNormalizesOperatorsOnlyWithinTheirShape(string $method, string $input, string $expected): void + { + $this->assertSame($expected, SqlOperator::{$method}($input)); } public static function invalidOperatorProvider(): array @@ -67,7 +79,7 @@ public function testRejectsOperatorsOutsideFiniteSet(string $operator): void { $this->expectException(RuntimeException::class); - SqlOperator::normalize($operator); + SqlOperator::normalizeBinary($operator); } public static function joinTypeProvider(): array @@ -76,7 +88,6 @@ public static function joinTypeProvider(): array 'inner' => ['inner', 'INNER'], 'left' => ['left', 'LEFT'], 'right' => ['right', 'RIGHT'], - 'full' => ['full', 'FULL'], 'cross' => ['cross', 'CROSS'], ]; } @@ -92,6 +103,7 @@ public static function invalidJoinTypeProvider(): array return [ 'empty' => [''], 'unsupported' => ['NATURAL'], + 'mysql unsupported' => ['FULL'], 'outer smuggling' => ['LEFT OUTER'], 'comment' => ['LEFT/**/'], 'leading whitespace' => [' LEFT'], @@ -288,4 +300,120 @@ public function testStructuredTablesAndAliasesAreQuoted(): void User::query()->from('u')->join('posts AS p', 'p.user_id', '=', 'u.id')->select('u.id')->toSql() ); } + + public static function incompatibleOperatorShapeProvider(): array + { + return [ + 'where unary with value' => [static function (): void { + User::query()->where('id', 'IS NULL', 1)->get(); + }], + 'where range with scalar' => [static function (): void { + User::query()->where('id', 'BETWEEN', 1)->get(); + }], + 'where list through binary API' => [static function (): void { + User::query()->where('id', 'IN', [1, 2])->get(); + }], + 'having unary with value' => [static function (): void { + User::query()->having('id', 'IS NOT NULL', 1)->get(); + }], + 'having range with scalar' => [static function (): void { + User::query()->having('id', 'BETWEEN', 1)->get(); + }], + 'join list with columns' => [static function (): void { + User::query()->join('posts', 'posts.user_id', 'IN', 'users.id')->get(); + }], + 'join unary with columns' => [static function (): void { + User::query()->join('posts', 'posts.user_id', 'IS NULL', 'users.id')->get(); + }], + 'on range with columns' => [static function (): void { + User::query()->join('posts', 'posts.user_id', '=', 'users.id') + ->on('posts.score', 'BETWEEN', 'users.score')->get(); + }], + ]; + } + + #[DataProvider('incompatibleOperatorShapeProvider')] + public function testBinaryApisRejectIncompatibleOperatorShapesBeforeExecution(callable $attempt): void + { + try { + $attempt(); + $this->fail('Expected the binary API to reject a non-binary operator.'); + } catch (RuntimeException $exception) { + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + } + + public function testFullJoinIsRejectedBecauseMysqlDoesNotSupportIt(): void + { + try { + User::query()->fullJoin('posts', 'posts.user_id', '=', 'users.id')->get(); + $this->fail('Expected FULL JOIN to be rejected.'); + } catch (RuntimeException $exception) { + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + } + + public function testBaseAliasQualifiesEveryStructuredBareColumnPath(): void + { + $sql = User::query()->from('u') + ->select('id') + ->where('status', 'active') + ->groupBy('role') + ->having('score', '>', 10) + ->orderBy('created_at') + ->toSql(); + + $this->assertStringContainsString('SELECT `u`.`id`', $sql); + $this->assertStringContainsString('WHERE `u`.`status`', $sql); + $this->assertStringContainsString('GROUP BY `u`.`role`', $sql); + $this->assertStringContainsString('HAVING `u`.`score`', $sql); + $this->assertStringContainsString('ORDER BY `u`.`created_at`', $sql); + $this->assertStringNotContainsString('`wp_users`.`', $sql); + } + + public function testBaseAliasQualifiesAggregateColumn(): void + { + $GLOBALS['wpdb']->resolver = static function () { + return [(object) ['MAX' => 20]]; + }; + + User::query()->from('u')->max('score'); + + $this->assertStringContainsString('MAX(`u`.`score`)', $GLOBALS['wpdb']->last_query); + } + + public function testUpdateJoinWhereRendersBindingsInPlaceholderOrder(): void + { + User::query()->joinWhere('posts', 'posts.status', '=', 'active') + ->where('users.id', 7) + ->update(['status' => 'archived']); + + $this->assertStringContainsString("`wp_posts`.`status` = 'active'", $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString("SET status = 'archived'", $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString('`wp_users`.`id` = 7', $GLOBALS['wpdb']->last_query); + } + + public function testUpdateOnValueRendersBindingsInPlaceholderOrder(): void + { + User::query()->join('posts', 'posts.user_id', '=', 'users.id') + ->onValue('posts.status', '=', 'active') + ->where('users.id', 7) + ->update(['status' => 'archived']); + + $this->assertStringContainsString("`wp_posts`.`status` = 'active'", $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString("SET status = 'archived'", $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString('`wp_users`.`id` = 7', $GLOBALS['wpdb']->last_query); + } + + public function testUpdateOnRawRendersBindingsInPlaceholderOrder(): void + { + User::query()->join('posts', 'posts.user_id', '=', 'users.id') + ->onRaw('`wp_posts`.`status` = %s', ['active']) + ->where('users.id', 7) + ->update(['status' => 'archived']); + + $this->assertStringContainsString("`wp_posts`.`status` = 'active'", $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString("SET status = 'archived'", $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString('`wp_users`.`id` = 7', $GLOBALS['wpdb']->last_query); + } } From 82c8309651ff58bb147ff2376604586fa6003fa4 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 01:44:01 +0600 Subject: [PATCH 04/11] fix(query): preserve list and write validity Normalize array-valued WHERE and HAVING operators through the list boundary, compile empty lists safely, and reject undeclared base aliases on UPDATE and DELETE. --- docs/breaking-changes.md | 2 + docs/usage.md | 4 ++ src/QueryBuilder.php | 37 +++++++++--- tests/StructuredClauseSafetyTest.php | 90 +++++++++++++++++++++++++++- 4 files changed, 123 insertions(+), 10 deletions(-) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 9503ae5..4a528b1 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -383,6 +383,8 @@ Migrate constants to the bound-value APIs and expressions to the explicit raw AP bind both bounds; hostile column text can no longer enter their SQL fragment. `FULL JOIN` is rejected because MySQL does not support it; use supported `INNER`, `LEFT`, `RIGHT`, or `CROSS` joins with a valid ON condition. +Base-table aliases created with `from()` are SELECT-only; UPDATE and DELETE +chains using them now throw before execution rather than emit an undeclared alias. --- diff --git a/docs/usage.md b/docs/usage.md index 3aa33ba..2bb0dc4 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -298,6 +298,10 @@ and `orderBy`. Already-prefixed names and registered aliases are recognized; unknown/schema-qualified names fail closed. Use `table AS alias` when declaring a join alias and `from('alias')` for the base-table alias. +`from()` aliases are supported for SELECT queries. UPDATE and DELETE reject a +base-table alias before execution because their current compilers do not declare +one; remove `from()` from write chains. + `join()`, `on()`, and `orOn()` are column-to-column APIs. They no longer guess whether the right operand is a number, quoted literal, or SQL function. Move constants to `joinWhere()` / `onValue()` and reviewed expressions to `onRaw()`. diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 4cfb1bd..4908baa 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -2003,7 +2003,8 @@ private function isListOfRows($attributes) * on prepare, independent of arity: an empty array becomes the false * constant `0 = 1`; a null value with an explicit operator becomes * IS [NOT] NULL; a non-empty array has its elements coerced to scalars; an - * object value is JSON-encoded. Having clauses are left untouched. + * object value is JSON-encoded. Explicit list operators are normalized by + * value shape for both where and having clauses. * * @param array $conditions * @param string $type @@ -2020,11 +2021,7 @@ private function normalizeConditions(array $conditions, $type) $this->assertSafeIdentifier($conditions['column']); } - if (isset($conditions['operator'])) { - $conditions['operator'] = SqlOperator::normalizeBinary($conditions['operator']); - } - - if ($type !== 'where' || !\array_key_exists('value', $conditions)) { + if (!\array_key_exists('value', $conditions)) { return $conditions; } @@ -2032,8 +2029,18 @@ private function normalizeConditions(array $conditions, $type) $bool = isset($conditions['bool']) ? $conditions['bool'] : 'AND'; if (\is_array($value)) { + $operator = 'IN'; + if (isset($conditions['operator'])) { + $operator = SqlOperator::normalizeList($conditions['operator']); + $conditions['operator'] = $operator; + } + if ($value === []) { - return ['bool' => $bool, 'raw' => '0 = 1', 'bindings' => []]; + return [ + 'bool' => $bool, + 'raw' => $operator === 'NOT IN' ? '1 = 1' : '0 = 1', + 'bindings' => [], + ]; } $conditions['value'] = $this->sanitizeInValues($value); @@ -2041,6 +2048,14 @@ private function normalizeConditions(array $conditions, $type) return $conditions; } + if (isset($conditions['operator'])) { + $conditions['operator'] = SqlOperator::normalizeBinary($conditions['operator']); + } + + if ($type !== 'where') { + return $conditions; + } + if (\is_null($value)) { if (isset($conditions['operator'])) { $conditions['operator'] = $this->nullOperator($conditions['operator']); @@ -2332,6 +2347,10 @@ function ($value, $key) { */ private function prepareUpdate() { + if (isset($this->_from)) { + throw new RuntimeException('Table aliases are not supported for write queries.'); + } + $setBindings = $this->bindings; $this->bindings = []; @@ -2365,6 +2384,10 @@ private function prepareUpdate() */ private function prepareDelete() { + if (isset($this->_from)) { + throw new RuntimeException('Table aliases are not supported for write queries.'); + } + $whereClause = $this->grammar()->getWhere($this); if (empty($whereClause)) { diff --git a/tests/StructuredClauseSafetyTest.php b/tests/StructuredClauseSafetyTest.php index 8a33640..251a6bb 100644 --- a/tests/StructuredClauseSafetyTest.php +++ b/tests/StructuredClauseSafetyTest.php @@ -310,9 +310,6 @@ public static function incompatibleOperatorShapeProvider(): array 'where range with scalar' => [static function (): void { User::query()->where('id', 'BETWEEN', 1)->get(); }], - 'where list through binary API' => [static function (): void { - User::query()->where('id', 'IN', [1, 2])->get(); - }], 'having unary with value' => [static function (): void { User::query()->having('id', 'IS NOT NULL', 1)->get(); }], @@ -416,4 +413,91 @@ public function testUpdateOnRawRendersBindingsInPlaceholderOrder(): void $this->assertStringContainsString("SET status = 'archived'", $GLOBALS['wpdb']->last_query); $this->assertStringContainsString('`wp_users`.`id` = 7', $GLOBALS['wpdb']->last_query); } + + public static function listConditionProvider(): array + { + return [ + 'where IN' => [static function () { + return User::query()->where('id', 'IN', [1, 2]); + }, 'WHERE `wp_users`.`id` IN (%d,%d)'], + 'where NOT IN' => [static function () { + return User::query()->where('id', 'NOT IN', [1, 2]); + }, 'WHERE `wp_users`.`id` NOT IN (%d,%d)'], + 'having IN' => [static function () { + return User::query()->having('id', 'IN', [1, 2]); + }, 'HAVING `wp_users`.`id` IN (%d,%d)'], + 'having NOT IN' => [static function () { + return User::query()->having('id', 'NOT IN', [1, 2]); + }, 'HAVING `wp_users`.`id` NOT IN (%d,%d)'], + ]; + } + + #[DataProvider('listConditionProvider')] + public function testWhereAndHavingPreserveNonEmptyListOperators(callable $build, string $expected): void + { + $query = $build(); + + $this->assertStringContainsString($expected, $query->toSql()); + $this->assertSame([1, 2], $query->getBindings()); + } + + public static function emptyListConditionProvider(): array + { + return [ + 'where empty IN is false' => [static function () { + return User::query()->where('id', 'IN', []); + }, 'WHERE 0 = 1'], + 'where empty NOT IN is true' => [static function () { + return User::query()->where('id', 'NOT IN', []); + }, 'WHERE 1 = 1'], + 'having empty IN is false' => [static function () { + return User::query()->having('id', 'IN', []); + }, 'HAVING 0 = 1'], + 'having empty NOT IN is true' => [static function () { + return User::query()->having('id', 'NOT IN', []); + }, 'HAVING 1 = 1'], + ]; + } + + #[DataProvider('emptyListConditionProvider')] + public function testWhereAndHavingCompileEmptyListsToBooleanConstants(callable $build, string $expected): void + { + $query = $build(); + + $this->assertStringContainsString($expected, $query->toSql()); + $this->assertSame([], $query->getBindings()); + } + + public function testHostileListOperatorIsRejectedBeforeExecution(): void + { + try { + User::query()->where('id', 'IN/**/OR', [1, 2])->get(); + $this->fail('Expected the hostile list operator to be rejected.'); + } catch (RuntimeException $exception) { + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + } + + public static function aliasedWriteProvider(): array + { + return [ + 'update' => [static function (): void { + User::query()->from('u')->where('id', 7)->update(['status' => 'archived']); + }], + 'delete' => [static function (): void { + User::query()->from('u')->where('id', 7)->delete(); + }], + ]; + } + + #[DataProvider('aliasedWriteProvider')] + public function testUnsupportedAliasedWritesAreRejectedBeforeExecution(callable $attempt): void + { + try { + $attempt(); + $this->fail('Expected an aliased write to be rejected.'); + } catch (RuntimeException $exception) { + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + } } From 10355ddd7c9207f04f1692e35396e5d8cc222065 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 01:59:14 +0600 Subject: [PATCH 05/11] fix(query): harden relationship SQL Keep generated relation aggregates, subqueries, and pivot projections in structured state so identifiers and aliases reach the shared renderer before execution. --- src/Concerns/QueriesRelationships.php | 15 +- src/Concerns/Relations.php | 101 +++++++-- src/Query/Grammar.php | 43 +++- src/QueryBuilder.php | 93 ++++++++- tests/BelongsToManyPivotTest.php | 14 +- tests/RelationshipIdentifierSafetyTest.php | 230 +++++++++++++++++++++ 6 files changed, 447 insertions(+), 49 deletions(-) create mode 100644 tests/RelationshipIdentifierSafetyTest.php diff --git a/src/Concerns/QueriesRelationships.php b/src/Concerns/QueriesRelationships.php index 35a5fe5..a50ea42 100644 --- a/src/Concerns/QueriesRelationships.php +++ b/src/Concerns/QueriesRelationships.php @@ -17,8 +17,8 @@ } /** - * Partial of {@see QueryBuilder} (relies on its $select/$selectRaw state, the - * bound $_model, and selectRaw()/whereRaw()/prepareColumnName()). + * Partial of {@see QueryBuilder} (relies on its structured select state, the + * bound $_model, and the identifier/raw-predicate boundaries). * * @mixin \BitApps\WPDatabase\QueryBuilder */ @@ -163,14 +163,11 @@ public function withAggregate($relation, $column, $function) $this->correlate($relationalQuery); if ($function === 'exists') { - $query = $relationalQuery->select($aggregateColumn)->prepare(); - $this->selectRaw("exists({$query}) as `{$alias}`")->withCast([$alias => 'bool']); + $relationalQuery->select($aggregateColumn); + $this->addSubquerySelect($relationalQuery, $alias, true)->withCast([$alias => 'bool']); } else { - if ($aggregateColumn !== '*') { - $aggregateColumn = $relationalQuery->prepareColumnName($aggregateColumn); - } - $query = $relationalQuery->selectRaw(sprintf('%s(%s)', $function, $aggregateColumn))->prepare(); - $this->selectRaw("({$query}) as `{$alias}`"); + $relationalQuery->addAggregateSelect($function, $aggregateColumn); + $this->addSubquerySelect($relationalQuery, $alias); } } diff --git a/src/Concerns/Relations.php b/src/Concerns/Relations.php index 305d6a2..12bb0c8 100644 --- a/src/Concerns/Relations.php +++ b/src/Concerns/Relations.php @@ -7,6 +7,7 @@ namespace BitApps\WPDatabase\Concerns; use BitApps\WPDatabase\Model; +use BitApps\WPDatabase\Query\Identifier; use BitApps\WPDatabase\QueryBuilder; use Closure; use RuntimeException; @@ -49,6 +50,13 @@ public function belongsTo($model, $foreignKey = null, $localKey = null) public function newBelongsTo($model, $foreignKey = null, $localKey = null) { + if ($foreignKey !== null) { + $this->assertSimpleRelationIdentifier($foreignKey); + } + if ($localKey !== null) { + $this->assertSimpleRelationIdentifier($localKey); + } + $model = new $model(); $model->setRelateAs('oneToOne'); $model->_relationKeys['oneToOne'] = [ @@ -75,6 +83,13 @@ public function hasMany($model, $foreignKey = null, $localKey = null) public function newHasMany($model, $foreignKey = null, $localKey = null) { + if ($foreignKey !== null) { + $this->assertSimpleRelationIdentifier($foreignKey); + } + if ($localKey !== null) { + $this->assertSimpleRelationIdentifier($localKey); + } + $model = new $model(); $model->setRelateAs('hasMany'); $model->_relationKeys['hasMany'] = [ @@ -111,6 +126,13 @@ public function belongsToMany( public function newBelongsToMany($model, $foreignKey = null, $localKey = null) { + if ($foreignKey !== null) { + $this->assertSimpleRelationIdentifier($foreignKey); + } + if ($localKey !== null) { + $this->assertSimpleRelationIdentifier($localKey); + } + $model = new $model(); $model->setRelateAs('belongsToMany'); $model->_relationKeys['belongsToMany'] = [ @@ -136,6 +158,10 @@ public function newBelongsToManyPivot( $parentKey = $parentKey ?: $this->getPrimaryKey(); $relatedKey = $relatedKey ?: $related->getPrimaryKey(); + foreach ([$pivotTable, $foreignPivotKey, $relatedPivotKey, $parentKey, $relatedKey] as $identifier) { + $this->assertSimpleRelationIdentifier($identifier); + } + $related->setRelateAs(Model::RELATE_AS_PIVOT); $related->_relationKeys[Model::RELATE_AS_PIVOT] = [ 'pivotTable' => $pivotTable, @@ -155,6 +181,10 @@ public function addPivotColumns(array $columns) throw new RuntimeException('withPivot() is only valid on a pivot belongsToMany relation.'); } + foreach ($columns as $column) { + $this->assertSimpleRelationIdentifier($column); + } + $this->_relationKeys[Model::RELATE_AS_PIVOT]['pivotColumns'] = array_merge( $this->_relationKeys[Model::RELATE_AS_PIVOT]['pivotColumns'], $columns @@ -210,18 +240,32 @@ public function prepareRelation(array $relations): array { $preparedRelation = []; foreach ($relations as $key => $value) { - if (\is_int($key) && \is_string($value) && ($method = explode(' ', $value)[0]) && method_exists($this, $method)) { - $preparedRelation[$value] = $this->resolveRelationQuery($method); - unset($relations[$key]); + if (!\is_int($key) || !\is_string($value)) { + continue; + } + + [$method] = $this->prepareRelationName($value); + if (!method_exists($this, $method)) { + continue; } + + $preparedRelation[$value] = $this->resolveRelationQuery($method); + unset($relations[$key]); } foreach ($relations as $key => $value) { - if (\is_string($key) && ($method = explode(' ', $key)[0]) && method_exists($this, $method)) { - $preparedRelation[$key] = $this->resolveRelationQuery($method); - if ($value instanceof Closure) { - $value($preparedRelation[$key]); - } + if (!\is_string($key)) { + continue; + } + + [$method] = $this->prepareRelationName($key); + if (!method_exists($this, $method)) { + continue; + } + + $preparedRelation[$key] = $this->resolveRelationQuery($method); + if ($value instanceof Closure) { + $value($preparedRelation[$key]); } } @@ -308,17 +352,38 @@ private function undefinedRelationMessage($method) public function prepareRelationName(string $relationName): array { - $name = $relationName; - $alias = null; - $nameChunk = explode(' ', $relationName); + $parts = preg_split('/\s+as\s+/i', $relationName); + if ($parts === false || \count($parts) > 2) { + throw new RuntimeException('Invalid relation name or alias.'); + } - if (\count($nameChunk) === 3 && strtolower($nameChunk[1]) === 'as') { - $alias = $nameChunk[2]; + $name = $parts[0]; + $alias = isset($parts[1]) ? $parts[1] : null; + $this->assertSimpleRelationIdentifier($name); + if ($alias !== null) { + $this->assertSimpleRelationIdentifier($alias); } return [$name, $alias]; } + /** + * Relationship metadata fields are single SQL identifier segments. Table + * qualification is added later by QueryBuilder's context-aware renderer. + * + * @param mixed $identifier + * + * @return void + */ + private function assertSimpleRelationIdentifier($identifier) + { + if (!\is_string($identifier)) { + throw new RuntimeException('Invalid relationship SQL identifier.'); + } + + Identifier::assertSimple($identifier); + } + private function getRelationKeys($foreignKey, $localKey) { if (!$foreignKey) { @@ -347,7 +412,7 @@ private function retrieveRelateData(QueryBuilder $query) $relationKey = $relationQuery->getModel()->getActiveRelationKey(); $relationQuery->whereRaw( - $relationKey['foreignKey'] + $relationQuery->prepareColumnName($relationKey['foreignKey']) . ' IN ( SELECT * FROM (' . $query->prepareKeySubquery($relationKey['localKey']) . ') AS subquery )' @@ -370,7 +435,7 @@ private function retrievePivotRelateData($relationName, QueryBuilder $relationQu [$pivot, $pivotRef, $bucketAlias] = $this->applyPivotSelectAndJoin($relationQuery); $relationQuery->whereRaw( - $pivotRef . '.' . $pivot['foreignPivotKey'] + $relationQuery->prepareColumnName($pivotRef . '.' . $pivot['foreignPivotKey']) . ' IN ( SELECT * FROM (' . $query->prepareKeySubquery($pivot['parentKey']) . ') AS subquery )' @@ -407,11 +472,11 @@ private function applyPivotSelectAndJoin(QueryBuilder $relationQuery) '=', $relationQuery->getTable() . '.' . $pivot['relatedKey'] ); - $relationQuery->selectRaw($pivotRef . '.' . $pivot['foreignPivotKey'] . ' as `' . $alias . '`'); + $relationQuery->addSelect($pivotRef . '.' . $pivot['foreignPivotKey'] . ' AS ' . $alias); foreach ($pivot['pivotColumns'] as $column) { - $relationQuery->selectRaw( - $pivotRef . '.' . $column . ' as `' . Model::PIVOT_ATTRIBUTE_PREFIX . $column . '`' + $relationQuery->addSelect( + $pivotRef . '.' . $column . ' AS ' . Model::PIVOT_ATTRIBUTE_PREFIX . $column ); } diff --git a/src/Query/Grammar.php b/src/Query/Grammar.php index ab5b942..32a8806 100644 --- a/src/Query/Grammar.php +++ b/src/Query/Grammar.php @@ -31,8 +31,14 @@ public function compileSelect(QueryBuilder $query): string $columns = array_map(static function ($column) use ($query) { return $query->renderIdentifier($column, true, true); }, $query->select); - $sql = 'SELECT ' . ($query->isDistinct() ? 'DISTINCT ' : '') . implode(',', $columns); - $sql .= $this->prepareRawSelect($query); + $structuredSql = implode(',', $columns); + foreach ($query->getSelectExpressions() as $expression) { + $structuredSql .= $structuredSql === '' ? '' : ', '; + $structuredSql .= $this->prepareSelectExpression($query, $expression); + } + + $sql = 'SELECT ' . ($query->isDistinct() ? 'DISTINCT ' : '') . $structuredSql; + $sql .= $this->prepareRawSelect($query, $structuredSql !== ''); $sql .= ' FROM ' . Identifier::quoteQualified($query->getTable()); $sql .= $this->getFrom($query); $sql .= $this->getJoin($query); @@ -242,11 +248,11 @@ private function getOffset(QueryBuilder $query) * * @return string */ - private function prepareRawSelect(QueryBuilder $query) + private function prepareRawSelect(QueryBuilder $query, $hasStructuredSelect = false) { $sql = ''; if (!empty($query->selectRaw['columns'])) { - $sql = \count($query->select) ? ', ' : ''; + $sql = $hasStructuredSelect ? ', ' : ''; $sql .= implode(', ', $query->selectRaw['columns']); } @@ -257,6 +263,35 @@ private function prepareRawSelect(QueryBuilder $query) return $sql; } + /** + * Compiles a framework-generated SELECT expression from typed state. + * + * @param array $expression + * + * @return string + */ + private function prepareSelectExpression(QueryBuilder $query, array $expression) + { + if ($expression['type'] === 'aggregate') { + $column = $expression['column'] === '*' + ? '*' + : $query->renderIdentifier($expression['column']); + $sql = $expression['function'] . '(' . $column . ')'; + if ($expression['alias'] !== null) { + $sql .= ' AS ' . Identifier::quoteAlias($expression['alias']); + } + + return $sql; + } + + $subquery = $expression['query']; + $sql = $subquery->toSql(); + $query->addBindings($subquery->getBindings()); + $sql = $expression['exists'] ? 'exists(' . $sql . ')' : '(' . $sql . ')'; + + return $sql . ' as ' . Identifier::quoteAlias($expression['alias']); + } + /** * Prepares the column part of a where clause. * diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 4908baa..2d89ab4 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -38,6 +38,15 @@ class QueryBuilder 'bindings' => [], ]; + /** + * Framework-generated SELECT expressions whose structural parts remain + * typed until Grammar compilation. Developer-authored expressions belong + * in selectRaw(). + * + * @var array + */ + protected $selectExpressions = []; + protected $table; protected $limit; @@ -411,10 +420,11 @@ public function select($columns = ['*']) */ public function prepareKeySubquery($keyColumn): string { - $clone = clone $this; - $clone->selectRaw = ['columns' => [], 'bindings' => []]; - $clone->groupBy = []; - $clone->having = []; + $clone = clone $this; + $clone->selectRaw = ['columns' => [], 'bindings' => []]; + $clone->selectExpressions = []; + $clone->groupBy = []; + $clone->having = []; if (!isset($clone->limit)) { $clone->orderBy = []; } @@ -443,6 +453,66 @@ public function addSelect($columns) return $this; } + /** + * Returns framework-generated structured SELECT expressions. + * + * @return array + */ + public function getSelectExpressions() + { + return $this->selectExpressions; + } + + /** + * Adds an aggregate SELECT expression without routing its identifier + * through the raw SQL channel. + * + * @param mixed $function + * @param mixed $column + * @param null|string $alias + * + * @return $this + */ + private function addAggregateSelect($function, $column, $alias = null) + { + $this->assertSafeAggregateFunction($function); + if ($column !== '*') { + $this->assertSafeIdentifier($column); + } + if ($alias !== null) { + Identifier::assertSimple($alias); + } + + $this->selectExpressions[] = [ + 'type' => 'aggregate', + 'function' => $function, + 'column' => $column, + 'alias' => $alias, + ]; + + return $this; + } + + /** + * Adds a relation subquery SELECT expression with a validated output alias. + * The nested builder remains structured until Grammar compilation. + * + * @return $this + */ + private function addSubquerySelect(QueryBuilder $query, string $alias, $exists = false) + { + Identifier::assertSimple($alias); + + $this->selectExpressions[] = [ + 'type' => 'subquery', + 'query' => $query, + 'alias' => $alias, + 'exists' => (bool) $exists, + ]; + + return $this; + } + /** * Selects raw query as column for query * @@ -1496,12 +1566,13 @@ public function aggregate($function, $column) { $this->assertSafeAggregateFunction($function); - $query = $this->clone(); - $query->select = []; - $query->selectRaw = ['columns' => [], 'bindings' => []]; - $query->distinct = false; - $preparedColumn = $column === '*' ? '*' : $query->prepareColumnName($column); - $result = $query->selectRaw($function . '(' . $preparedColumn . ') as ' . $function)->exec(); + $query = $this->clone(); + $query->select = []; + $query->selectRaw = ['columns' => [], 'bindings' => []]; + $query->selectExpressions = []; + $query->distinct = false; + $preparedColumn = $column === '*' ? '*' : $query->prepareColumnName($column); + $result = $query->selectRaw($function . '(' . $preparedColumn . ') as ' . $function)->exec(); return \is_array($result) && isset($result[0]->{$function}) ? $result[0]->{$function} : null; } @@ -1649,7 +1720,7 @@ public function toSql() break; } - } elseif (!empty($this->select) || !empty($this->selectRaw)) { + } elseif (!empty($this->select) || !empty($this->selectExpressions) || !empty($this->selectRaw)) { $this->_method = self::SELECT; $sql = $this->grammar()->compileSelect($this); } diff --git a/tests/BelongsToManyPivotTest.php b/tests/BelongsToManyPivotTest.php index 22f38c6..2c24df8 100644 --- a/tests/BelongsToManyPivotTest.php +++ b/tests/BelongsToManyPivotTest.php @@ -63,19 +63,19 @@ public function testEagerSqlShape(): void $sql = $GLOBALS['wpdb']->queries[1]; $this->assertStringContainsString('SELECT `wp_roles`.*', $sql); - $this->assertStringContainsString('wp_role_user.member_id as `pivot_member_id`', $sql); + $this->assertStringContainsString('`wp_role_user`.`member_id` AS `pivot_member_id`', $sql); $this->assertStringContainsString('INNER JOIN `wp_role_user`', $sql); $this->assertStringContainsString('`wp_role_user`.`role_id` = `wp_roles`.`id`', $sql); // Inner fragment without a leading `WHERE ` boundary: the grammar emits `WHERE ` (double space). $this->assertStringContainsString( - 'wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM `wp_members`) AS subquery )', + '`wp_role_user`.`member_id` IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM `wp_members`) AS subquery )', $sql ); // Exact pin (absorbs the double-space WHERE/ON grammar artifacts). - $expected = 'SELECT `wp_roles`.*, wp_role_user.member_id as `pivot_member_id`' + $expected = 'SELECT `wp_roles`.*,`wp_role_user`.`member_id` AS `pivot_member_id`' . ' FROM `wp_roles` INNER JOIN `wp_role_user` ON `wp_role_user`.`role_id` = `wp_roles`.`id`' - . ' WHERE wp_role_user.member_id IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM `wp_members`) AS subquery )'; + . ' WHERE `wp_role_user`.`member_id` IN ( SELECT * FROM (SELECT `wp_members`.`id` FROM `wp_members`) AS subquery )'; $this->assertSame($expected, $sql); } @@ -92,7 +92,7 @@ public function testLazyAccessEmitsSingleValuePredicate(): void $this->assertStringNotContainsString('subquery', $sql, 'lazy access must not use the IN ( SELECT ) subquery form'); // Exact pin: single-value predicate, double-space WHERE/ON/`=` grammar artifacts. - $expected = 'SELECT `wp_roles`.*, wp_role_user.member_id as `pivot_member_id`' + $expected = 'SELECT `wp_roles`.*,`wp_role_user`.`member_id` AS `pivot_member_id`' . ' FROM `wp_roles` INNER JOIN `wp_role_user` ON `wp_role_user`.`role_id` = `wp_roles`.`id`' . ' WHERE `wp_role_user`.`member_id` = 1'; $this->assertSame($expected, $sql); @@ -115,7 +115,7 @@ public function testDefaultKeyDerivationUsesForeignKeyConvention(): void $members = Member::with('rolesDefaultKeys')->get(); $sql = $GLOBALS['wpdb']->queries[1]; - $this->assertStringContainsString('wp_role_user.members_id as `pivot_members_id`', $sql); + $this->assertStringContainsString('`wp_role_user`.`members_id` AS `pivot_members_id`', $sql); $this->assertStringContainsString('`wp_role_user`.`roles_id` = `wp_roles`.`id`', $sql); $this->assertCount(2, $members[0]->rolesDefaultKeys); $this->assertCount(1, $members[1]->rolesDefaultKeys); @@ -128,7 +128,7 @@ public function testWithPivotSelectsAndExposesExtraColumn(): void $members = Member::with('rolesWithPivot')->get(); $sql = $GLOBALS['wpdb']->queries[1]; - $this->assertStringContainsString('wp_role_user.assigned_at as `pivot_assigned_at`', $sql); + $this->assertStringContainsString('`wp_role_user`.`assigned_at` AS `pivot_assigned_at`', $sql); $this->assertSame('2024-01-01', $members[0]->rolesWithPivot[0]->pivot_assigned_at); } diff --git a/tests/RelationshipIdentifierSafetyTest.php b/tests/RelationshipIdentifierSafetyTest.php new file mode 100644 index 0000000..9e7a303 --- /dev/null +++ b/tests/RelationshipIdentifierSafetyTest.php @@ -0,0 +1,230 @@ + ['posts as total`'], + 'comment' => ['posts as total --'], + 'expression' => ['posts as COUNT(id)'], + 'qualified segment' => ['posts as report.total'], + ]; + } + + #[DataProvider('hostileRelationAliases')] + public function testAggregateRelationAliasRejectsHostileIdentifierBeforeExecution(string $relation): void + { + $this->assertRejectedBeforeExecution(static function () use ($relation): void { + User::withCount($relation)->get(); + }); + } + + #[DataProvider('hostileRelationAliases')] + public function testWhereHasRelationAliasRejectsHostileIdentifierBeforeExecution(string $relation): void + { + $this->assertRejectedBeforeExecution(static function () use ($relation): void { + User::whereHas($relation)->get(); + }); + } + + public static function hostileHasManyKeys(): array + { + return [ + 'foreign key backtick' => ['foreignKey', 'parent_id` OR 1=1 --'], + 'foreign key comment' => ['foreignKey', 'parent_id/*x*/'], + 'local key expression' => ['localKey', 'COALESCE(id, 0)'], + 'local key dot' => ['localKey', 'parents.id'], + ]; + } + + #[DataProvider('hostileHasManyKeys')] + public function testHasManyKeyRejectsNonSimpleSegmentBeforeExecution(string $property, string $value): void + { + SafetyRelationParent::${$property} = $value; + + $this->assertRejectedBeforeExecution(static function (): void { + SafetyRelationParent::with('unsafeChildren')->get(); + }); + } + + public static function hostilePivotMetadata(): array + { + return [ + 'pivot table comment' => ['pivotTable', 'role_user/*x*/'], + 'pivot table alias' => ['pivotTable', 'role_user AS pivot_link'], + 'foreign pivot key backtick' => ['foreignPivotKey', 'member_id`'], + 'foreign pivot key dot' => ['foreignPivotKey', 'pivot.member_id'], + 'related pivot key comment' => ['relatedPivotKey', 'role_id--'], + 'parent key expression' => ['parentKey', 'COALESCE(id, 0)'], + 'related key dot' => ['relatedKey', 'roles.id'], + ]; + } + + #[DataProvider('hostilePivotMetadata')] + public function testPivotMetadataRejectsNonSimpleSegmentBeforeExecution(string $property, string $value): void + { + SafetyPivotParent::${$property} = $value; + + $this->assertRejectedBeforeExecution(static function (): void { + SafetyPivotParent::with('unsafePivots')->get(); + }); + } + + public static function hostilePivotColumns(): array + { + return [ + 'backtick' => ['assigned_at`'], + 'comment' => ['assigned_at/*x*/'], + 'expression' => ['MAX(assigned_at)'], + 'qualified segment' => ['role_user.assigned_at'], + ]; + } + + #[DataProvider('hostilePivotColumns')] + public function testPivotSelectedColumnAndDerivedAliasRejectHostileIdentifierBeforeExecution(string $column): void + { + SafetyPivotParent::$pivotColumns = [$column]; + + $this->assertRejectedBeforeExecution(static function (): void { + SafetyPivotParent::with('unsafePivots')->get(); + }); + } + + public function testAggregateColumnRejectsRawExpressionBeforeExecution(): void + { + $this->assertRejectedBeforeExecution(static function (): void { + User::withAggregate('posts', 'amount) AS injected --', 'sum')->get(); + }); + } + + public function testValidAggregateAliasIsQuotedAndUsesStructuredSubquery(): void + { + $query = User::withCount('posts as published_total'); + $sql = $query->toSql(); + + $this->assertStringContainsStringIgnoringCase('AS `published_total`', $sql); + $this->assertSame([], $query->selectRaw['columns']); + } + + public function testValidPivotIdentifiersKeepPrefixAndQuoteDerivedAliases(): void + { + $GLOBALS['wpdb']->resolver = static function ($sql) { + if (strpos($sql, 'wp_roles') !== false) { + return [(object) ['id' => 10, 'pivot_member_id' => 1, 'pivot_assigned_at' => '2024-01-01']]; + } + + return [(object) ['id' => 1]]; + }; + + Member::with('rolesWithPivot')->get(); + $sql = $GLOBALS['wpdb']->queries[1]; + + $this->assertStringContainsString('INNER JOIN `wp_role_user`', $sql); + $this->assertStringContainsString('`wp_role_user`.`member_id` AS `pivot_member_id`', $sql); + $this->assertStringContainsString('`wp_role_user`.`assigned_at` AS `pivot_assigned_at`', $sql); + } + + private function assertRejectedBeforeExecution(callable $operation): void + { + $exception = null; + + try { + $operation(); + } catch (RuntimeException $caught) { + $exception = $caught; + } + + $this->assertInstanceOf(RuntimeException::class, $exception); + $this->assertSame([], $GLOBALS['wpdb']->queries); + } +} + +final class SafetyRelationParent extends Model +{ + public static $foreignKey = 'parent_id'; + + public static $localKey = 'id'; + + public $timestamps = false; + + protected $table = 'safety_parents'; + + public function unsafeChildren() + { + return $this->hasMany(SafetyRelationChild::class, self::$foreignKey, self::$localKey); + } +} + +final class SafetyRelationChild extends Model +{ + public $timestamps = false; + + protected $table = 'safety_children'; +} + +final class SafetyPivotParent extends Model +{ + public static $pivotTable = 'role_user'; + + public static $foreignPivotKey = 'member_id'; + + public static $relatedPivotKey = 'role_id'; + + public static $parentKey = 'id'; + + public static $relatedKey = 'id'; + + public static $pivotColumns = []; + + public $timestamps = false; + + protected $table = 'safety_pivot_parents'; + + public function unsafePivots() + { + return $this->belongsToMany( + SafetyPivotRelated::class, + self::$pivotTable, + self::$foreignPivotKey, + self::$relatedPivotKey, + self::$parentKey, + self::$relatedKey + )->withPivot(self::$pivotColumns); + } +} + +final class SafetyPivotRelated extends Model +{ + public $timestamps = false; + + protected $table = 'safety_pivot_related'; +} From 3e91f5deb304cc4bf23bfb21ec3c0fc9684ce9d6 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 02:13:49 +0600 Subject: [PATCH 06/11] fix(query): harden write schemas --- src/QueryBuilder.php | 281 ++++++++++++++++++--------- tests/QueryFeaturesTest.php | 2 +- tests/RaggedBulkInsertFixTest.php | 6 +- tests/RelationDirtyFixTest.php | 4 +- tests/SoftDeleteRestoreTest.php | 2 +- tests/StructuredClauseSafetyTest.php | 6 +- tests/UpsertTest.php | 14 +- tests/WriteEdgeFixTest.php | 2 +- tests/WriteIdentifierSafetyTest.php | 176 +++++++++++++++++ 9 files changed, 379 insertions(+), 114 deletions(-) create mode 100644 tests/WriteIdentifierSafetyTest.php diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 2d89ab4..f5b16a8 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1435,6 +1435,7 @@ public function insert($attributes = []) return $this->bulkInsert($attributes); } + $this->normalizeWriteColumns(array_keys($attributes)); $this->_model->fill($attributes); if ($this->save()) { return $this->_model; @@ -1452,6 +1453,11 @@ public function insert($attributes = []) */ public function update($attributes = []) { + if (empty($attributes)) { + return false; + } + + $this->normalizeWriteColumns(array_keys($attributes)); $this->_model->fill($attributes); if ($this->_model->exists()) { return $this->save(); @@ -1484,6 +1490,10 @@ public function destroy($ids = []) */ public function save() { + $this->normalizeWriteColumns( + $this->withoutRelationColumns(array_keys($this->_model->getAttributes())) + ); + if ($this->_model->fireEvent('saving') === false) { return false; } @@ -1522,6 +1532,10 @@ public function save() return false; } + if (empty($columns)) { + return false; + } + $this->insert = $columns; if ($this->exec() === false) { return false; @@ -1743,79 +1757,57 @@ public function when($value = null, ?callable $callback = null, ?callable $defau public function upsert(array $values, ?array $update = null) { - if (!\is_array(reset($values))) { - $values = [$values]; + if (empty($values)) { + return false; } - if (empty($update)) { - $update = array_keys($values[0]); + if (!$this->isListOfRows($values)) { + $values = [$values]; } - $this->bindings = []; - $columns = array_keys($values[0]); - sort($columns); $manageTimestamps = property_exists($this->_model, 'timestamps') && $this->_model->timestamps; - $addCreatedAt = $manageTimestamps && !\in_array('created_at', $columns, true); - $addUpdatedAt = $manageTimestamps && !\in_array('updated_at', $columns, true); - if ($addCreatedAt) { - $columns[] = 'created_at'; + [$columns, $rows] = $this->normalizeWriteRows( + $values, + $manageTimestamps ? ['created_at', 'updated_at'] : [] + ); + if (empty($columns)) { + return false; } - if ($addUpdatedAt) { - $columns[] = 'updated_at'; + + if (empty($update)) { + $update = $columns; } - $sql = 'INSERT INTO ' . $this->table; - $sql .= ' (' . implode(', ', $columns) . ')'; + $update = $this->normalizeWriteColumns($update); - $sql .= ' VALUES '; - $insertAbleValues = []; - foreach ($values as $row) { - ksort($row); - if ($addCreatedAt || $addUpdatedAt) { - $now = $this->currentTimestamp(); - if ($addCreatedAt) { - $row['created_at'] = $now; - } - if ($addUpdatedAt) { - $row['updated_at'] = $now; - } + if ($manageTimestamps) { + // Never overwrite the original creation time on update; always bump updated_at. + $update = array_values(array_diff($update, ['created_at'])); + if (!\in_array('updated_at', $update, true)) { + $update[] = 'updated_at'; } + } - $rowValues = array_values($row); + $this->bindings = []; + $sql = 'INSERT INTO ' . $this->table; + $sql .= ' (' . implode(', ', $this->compileWriteColumns($columns)) . ')'; + + $sql .= ' VALUES '; + $insertAbleValues = []; + foreach ($rows as $row) { $insertAbleValues[] = ' (' . implode( ', ', - array_map( - function ($value) { - if (\is_null($value)) { - return 'NULL'; - } - - if (\is_array($value) || \is_object($value)) { - $value = wp_json_encode($value); - } - - $this->bindings[] = $value; - - return $this->getValueType($value); - }, - $rowValues - ) + array_map(fn ($value) => $this->compileWriteValue($value), $row) ) . ')'; } $sql .= empty($insertAbleValues) ? ' default values' : ' ' . implode(',', $insertAbleValues); $sql .= ' ON DUPLICATE KEY UPDATE '; - if ($manageTimestamps) { - // Never overwrite the original creation time on update; always bump updated_at. - $update = array_diff($update, ['created_at']); - if (!\in_array('updated_at', $update, true)) { - $update[] = 'updated_at'; - } - } - $update = array_map(function ($column) { + $sql .= implode(', ', array_map(function ($column) { + $column = $this->compileWriteColumn($column); + return $column . ' = VALUES(' . $column . ')'; - }, $update); - $sql .= implode(', ', $update); + }, $update)); $sql .= ';'; return $this->raw($sql, $this->bindings); @@ -2225,6 +2217,129 @@ private function autoScopeEnabled() return true; } + /** + * Validates logical write-column names while keeping them unquoted until + * compilation. Write schemas intentionally allow only simple identifiers: + * dot-qualified paths are meaningful in read clauses, not INSERT/UPDATE. + * + * @param array $columns + * + * @return array + */ + private function normalizeWriteColumns(array $columns) + { + $normalized = []; + + foreach ($columns as $column) { + if (!\is_string($column)) { + throw new RuntimeException('Invalid SQL identifier.'); + } + + Identifier::assertSimple($column); + if (!\in_array($column, $normalized, true)) { + $normalized[] = $column; + } + } + + return $normalized; + } + + /** + * Builds the canonical, first-seen union schema for a set of write rows + * and aligns every row to it. Missing input remains SQL NULL rather than + * borrowing a value by position from another row. + * + * @param array $rows + * @param array $managedTimestampColumns + * + * @return array{0: array, 1: array} + */ + private function normalizeWriteRows(array $rows, array $managedTimestampColumns = []) + { + $columns = []; + foreach ($rows as $row) { + if (!\is_array($row)) { + throw new RuntimeException('Invalid write row.'); + } + + foreach ($this->normalizeWriteColumns(array_keys($row)) as $column) { + if (!\in_array($column, $columns, true)) { + $columns[] = $column; + } + } + } + + $generatedTimestamps = []; + foreach ($managedTimestampColumns as $column) { + $this->normalizeWriteColumns([$column]); + if (!\in_array($column, $columns, true)) { + $columns[] = $column; + $generatedTimestamps[$column] = true; + } + } + + $normalizedRows = []; + foreach ($rows as $row) { + $normalizedRow = []; + foreach ($columns as $column) { + if (\array_key_exists($column, $row)) { + $normalizedRow[] = $row[$column]; + } elseif (isset($generatedTimestamps[$column])) { + $normalizedRow[] = $this->currentTimestamp(); + } else { + $normalizedRow[] = null; + } + } + $normalizedRows[] = $normalizedRow; + } + + return [$columns, $normalizedRows]; + } + + /** + * @param array $columns + * + * @return array + */ + private function compileWriteColumns(array $columns) + { + return array_map(fn ($column) => $this->compileWriteColumn($column), $columns); + } + + /** + * @param mixed $column + * + * @return string + */ + private function compileWriteColumn($column) + { + if (!\is_string($column)) { + throw new RuntimeException('Invalid SQL identifier.'); + } + + return Identifier::quoteQualified($column); + } + + /** + * @param mixed $value + * + * @return string + */ + private function compileWriteValue($value) + { + if (\is_null($value)) { + return 'NULL'; + } + + if (\is_array($value) || \is_object($value)) { + $value = wp_json_encode($value); + } + + $this->bindings[] = $value; + + return $this->getValueType($value); + } + /** * Run bulk insert query * @@ -2234,55 +2349,25 @@ private function autoScopeEnabled() */ private function bulkInsert($attributes) { - $firstRow = reset($attributes); - if (empty($firstRow)) { + [$columns, $rows] = $this->normalizeWriteRows( + $attributes, + property_exists($this->_model, 'timestamps') && $this->_model->timestamps ? ['created_at'] : [] + ); + if (empty($columns)) { return new Collection([]); } - ksort($firstRow); - $columns = array_keys($firstRow); - $createdAt = property_exists($this->_model, 'timestamps') && $this->_model->timestamps; - if ($createdAt) { - $columns[] = 'created_at'; - } - $this->bindings = []; $sql = 'INSERT INTO ' . $this->table; - $sql .= ' (' . implode(', ', $columns) . ')'; + $sql .= ' (' . implode(', ', $this->compileWriteColumns($columns)) . ')'; $sql .= ' VALUES '; $values = []; - foreach ($attributes as $row) { - if ($createdAt) { - $row['created_at'] = $this->currentTimestamp(); - } - - // Align each row to the header columns by key so rows with differing - // keys are not positionally misaligned (absent column => NULL). - $rowValues = []; - foreach ($columns as $column) { - $rowValues[] = isset($row[$column]) ? $row[$column] : null; - } - + foreach ($rows as $row) { $values[] = ' (' . implode( ', ', - array_map( - function ($value) { - if (\is_null($value)) { - return 'NULL'; - } - - if (\is_array($value) || \is_object($value)) { - $value = wp_json_encode($value); - } - - $this->bindings[] = $value; - - return $this->getValueType($value); - }, - $rowValues - ) + array_map(fn ($value) => $this->compileWriteValue($value), $row) ) . ')'; } @@ -2333,15 +2418,18 @@ private function prepareAttributeForSaveOrUpdate($isUpdate = false) } $columnsToPrepare = $this->withoutRelationColumns($columnsToPrepare); + $columnsToPrepare = $this->normalizeWriteColumns($columnsToPrepare); if (property_exists($this->_model, 'timestamps') && $this->_model->timestamps) { - if (!$isUpdate) { + if (!$isUpdate && !\in_array('created_at', $columnsToPrepare, true)) { $this->_model->setAttribute('created_at', $this->currentTimestamp()); $columnsToPrepare[] = 'created_at'; } $this->_model->setAttribute('updated_at', $this->currentTimestamp()); - $columnsToPrepare[] = 'updated_at'; + if (!\in_array('updated_at', $columnsToPrepare, true)) { + $columnsToPrepare[] = 'updated_at'; + } } $attributes = $this->_model->getAttributes(); @@ -2389,7 +2477,7 @@ private function withoutRelationColumns(array $columns) private function prepareInsert() { $sql = 'INSERT INTO ' . $this->table; - $sql .= ' (' . implode(', ', $this->insert) . ')'; + $sql .= ' (' . implode(', ', $this->compileWriteColumns($this->insert)) . ')'; $sql .= ' VALUES (' . implode( ', ', @@ -2430,7 +2518,8 @@ private function prepareUpdate() $sql .= ' SET '; $columnCount = \count($this->update); foreach ($this->update as $key => $column) { - $value = $setBindings[$key]; + $value = $setBindings[$key]; + $column = $this->compileWriteColumn($column); if (\is_null($value)) { $sql .= $column . ' = NULL'; } else { diff --git a/tests/QueryFeaturesTest.php b/tests/QueryFeaturesTest.php index cc444f6..97a88e9 100644 --- a/tests/QueryFeaturesTest.php +++ b/tests/QueryFeaturesTest.php @@ -155,7 +155,7 @@ public function testBulkUpdateExecutesUpdateStatement(): void $sql = $GLOBALS['wpdb']->last_query; $this->assertStringStartsWith('UPDATE wp_users', $sql); - $this->assertMatchesRegularExpression('/SET\s+status\s*=/i', $sql); + $this->assertMatchesRegularExpression('/SET\s+`status`\s*=/i', $sql); $this->assertStringContainsString('WHERE', $sql); } diff --git a/tests/RaggedBulkInsertFixTest.php b/tests/RaggedBulkInsertFixTest.php index 421f471..1cf76c7 100644 --- a/tests/RaggedBulkInsertFixTest.php +++ b/tests/RaggedBulkInsertFixTest.php @@ -35,9 +35,9 @@ public function testRaggedRowsAlignByColumnKey(): void $sql = $GLOBALS['wpdb']->last_query; - $this->assertStringContainsString('(a, b)', $sql); - $this->assertStringContainsString("('x', 'y')", $sql); - $this->assertStringContainsString("(NULL, 'z')", $sql); + $this->assertStringContainsString('(`a`, `b`, `c`)', $sql); + $this->assertStringContainsString("('x', 'y', NULL)", $sql); + $this->assertStringContainsString("(NULL, 'z', 'w')", $sql); $this->assertStringNotContainsString("('z'", $sql); } } diff --git a/tests/RelationDirtyFixTest.php b/tests/RelationDirtyFixTest.php index b34dbfe..ae208a1 100644 --- a/tests/RelationDirtyFixTest.php +++ b/tests/RelationDirtyFixTest.php @@ -49,7 +49,7 @@ public function testLazyRelationAccessIsNotPersistedOnSave(): void $sql = $GLOBALS['wpdb']->last_query; $this->assertStringContainsString('UPDATE wp_users', $sql); - $this->assertStringContainsString('name =', $sql); + $this->assertStringContainsString('`name` =', $sql); $this->assertStringNotContainsString('posts', $sql); } @@ -68,7 +68,7 @@ public function testRealArrayColumnIsStillWritten(): void $user->save(); $sql = $GLOBALS['wpdb']->last_query; - $this->assertStringContainsString('tags =', $sql); + $this->assertStringContainsString('`tags` =', $sql); $this->assertStringContainsString('["a","b"]', $sql); } } diff --git a/tests/SoftDeleteRestoreTest.php b/tests/SoftDeleteRestoreTest.php index 2277be8..ff2f591 100644 --- a/tests/SoftDeleteRestoreTest.php +++ b/tests/SoftDeleteRestoreTest.php @@ -39,7 +39,7 @@ public function testRestoreNullsDeletedAt(): void SoftPost::where('id', 1)->restore(); $sql = $GLOBALS['wpdb']->last_query; - $this->assertMatchesRegularExpression('/UPDATE\s+wp_soft_posts\s+SET\s+deleted_at\s*=\s*NULL/i', $sql); + $this->assertMatchesRegularExpression('/UPDATE\s+wp_soft_posts\s+SET\s+`deleted_at`\s*=\s*NULL/i', $sql); } public function testForceDeleteRejectsNonSoftDeleteModel(): void diff --git a/tests/StructuredClauseSafetyTest.php b/tests/StructuredClauseSafetyTest.php index 251a6bb..d7dfad1 100644 --- a/tests/StructuredClauseSafetyTest.php +++ b/tests/StructuredClauseSafetyTest.php @@ -386,7 +386,7 @@ public function testUpdateJoinWhereRendersBindingsInPlaceholderOrder(): void ->update(['status' => 'archived']); $this->assertStringContainsString("`wp_posts`.`status` = 'active'", $GLOBALS['wpdb']->last_query); - $this->assertStringContainsString("SET status = 'archived'", $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString("SET `status` = 'archived'", $GLOBALS['wpdb']->last_query); $this->assertStringContainsString('`wp_users`.`id` = 7', $GLOBALS['wpdb']->last_query); } @@ -398,7 +398,7 @@ public function testUpdateOnValueRendersBindingsInPlaceholderOrder(): void ->update(['status' => 'archived']); $this->assertStringContainsString("`wp_posts`.`status` = 'active'", $GLOBALS['wpdb']->last_query); - $this->assertStringContainsString("SET status = 'archived'", $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString("SET `status` = 'archived'", $GLOBALS['wpdb']->last_query); $this->assertStringContainsString('`wp_users`.`id` = 7', $GLOBALS['wpdb']->last_query); } @@ -410,7 +410,7 @@ public function testUpdateOnRawRendersBindingsInPlaceholderOrder(): void ->update(['status' => 'archived']); $this->assertStringContainsString("`wp_posts`.`status` = 'active'", $GLOBALS['wpdb']->last_query); - $this->assertStringContainsString("SET status = 'archived'", $GLOBALS['wpdb']->last_query); + $this->assertStringContainsString("SET `status` = 'archived'", $GLOBALS['wpdb']->last_query); $this->assertStringContainsString('`wp_users`.`id` = 7', $GLOBALS['wpdb']->last_query); } diff --git a/tests/UpsertTest.php b/tests/UpsertTest.php index c019a15..dfa01ec 100644 --- a/tests/UpsertTest.php +++ b/tests/UpsertTest.php @@ -24,7 +24,7 @@ protected function tearDown(): void } /** - * Column names are ordered to match the (alphabetically) sorted row values. + * Column names and values preserve first-seen input order. */ public function testUpsertAlignsColumnsWithValues(): void { @@ -32,8 +32,8 @@ public function testUpsertAlignsColumnsWithValues(): void $sql = $GLOBALS['wpdb']->last_query; - $this->assertStringContainsString('(email, first_name)', $sql); - $this->assertStringContainsString("('a@x.com', 'Ada')", $sql); + $this->assertStringContainsString('(`first_name`, `email`)', $sql); + $this->assertStringContainsString("('Ada', 'a@x.com')", $sql); } /** @@ -51,10 +51,10 @@ public function testUpsertManagesTimestampsOnDuplicate(): void $this->assertStringContainsString('created_at', $sql); $this->assertStringContainsString('updated_at', $sql); // updated_at is bumped from its own inserted value, not created_at - $this->assertStringContainsString('updated_at = VALUES(updated_at)', $sql); - $this->assertStringNotContainsString('VALUES(created_at)', $sql); + $this->assertStringContainsString('`updated_at` = VALUES(`updated_at`)', $sql); + $this->assertStringNotContainsString('VALUES(`created_at`)', $sql); // created_at is preserved on update (never in the update set) - $this->assertStringNotContainsString('created_at = VALUES(created_at)', $sql); + $this->assertStringNotContainsString('`created_at` = VALUES(`created_at`)', $sql); } /** @@ -69,7 +69,7 @@ public function testUpsertWithoutTimestampsHasNoMagic(): void $sql = $GLOBALS['wpdb']->last_query; - $this->assertStringContainsString('created_at = VALUES(created_at)', $sql); + $this->assertStringContainsString('`created_at` = VALUES(`created_at`)', $sql); $this->assertStringNotContainsString('updated_at', $sql); } diff --git a/tests/WriteEdgeFixTest.php b/tests/WriteEdgeFixTest.php index 0ecdc10..fb476a1 100644 --- a/tests/WriteEdgeFixTest.php +++ b/tests/WriteEdgeFixTest.php @@ -83,7 +83,7 @@ public function testUpsertEmptyUpdateListDefaultsToAllColumns(): void User::query()->upsert(['email' => 'a@x.com'], []); $sql = $GLOBALS['wpdb']->last_query; - $this->assertStringContainsString('email = VALUES(email)', $sql); + $this->assertStringContainsString('`email` = VALUES(`email`)', $sql); $this->assertStringNotContainsString('UPDATE ;', $sql); } diff --git a/tests/WriteIdentifierSafetyTest.php b/tests/WriteIdentifierSafetyTest.php new file mode 100644 index 0000000..57fbdef --- /dev/null +++ b/tests/WriteIdentifierSafetyTest.php @@ -0,0 +1,176 @@ +assertWriteIsRejected(static function (): void { + User::query()->insert(['name) VALUES (1); --' => 'Ada']); + }); + } + + public function testBulkInsertRejectsHostileLaterRowKeyWithoutQuery(): void + { + $this->assertWriteIsRejected(static function (): void { + User::query()->insert([ + ['name' => 'Ada'], + ['email) VALUES (1); --' => 'ada@example.test'], + ]); + }); + } + + public function testUpdateRejectsHostileKeyWithoutQuery(): void + { + $this->assertWriteIsRejected(static function (): void { + User::query()->update(['name = NULL; --' => 'Ada']); + }); + } + + public function testSaveRejectsHostileModelAttributeWithoutQuery(): void + { + $this->assertWriteIsRejected(static function (): void { + $user = new User(['name = NULL; --' => 'Ada']); + $user->setExists(false); + $user->save(); + }); + } + + public function testUpsertRejectsHostileLaterRowKeyWithoutQuery(): void + { + $this->assertWriteIsRejected(static function (): void { + User::query()->upsert([ + ['name' => 'Ada'], + ['email) VALUES (1); --' => 'ada@example.test'], + ]); + }); + } + + public function testUpsertRejectsHostileExplicitUpdateColumnWithoutQuery(): void + { + $this->assertWriteIsRejected(static function (): void { + User::query()->upsert(['email' => 'ada@example.test'], ['email = VALUES(email); --']); + }); + } + + public function testUpsertEmptyRowsReturnsFalseWithoutQuery(): void + { + $this->assertFalse(User::query()->upsert([])); + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + + public function testUpdateEmptyAttributesReturnsFalseWithoutQuery(): void + { + $this->assertFalse(User::query()->update([])); + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + + public function testSaveNewModelWithoutAttributesReturnsFalseWithoutQuery(): void + { + $user = new User(); + $user->setExists(false); + + $this->assertFalse($user->save()); + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + + public function testBulkInsertNormalizesFirstSeenUnionAndRaggedValues(): void + { + $GLOBALS['wpdb']->rows_affected = 0; + + User::query()->insert([ + ['name' => 'Ada', 'meta' => ['active' => true]], + ['custom_field_12' => null, 'name' => 'Grace'], + ['meta' => ['active' => false], 'email' => 'grace@example.test'], + ]); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('(`name`, `meta`, `custom_field_12`, `email`)', $sql); + $this->assertStringContainsString("('Ada', '{\"active\":true}', NULL, NULL)", $sql); + $this->assertStringContainsString("('Grace', NULL, NULL, NULL)", $sql); + $this->assertStringContainsString("(NULL, '{\"active\":false}', NULL, 'grace@example.test')", $sql); + } + + public function testUpsertNormalizesRowsAndRendersIdentifiers(): void + { + User::query()->upsert([ + ['email' => 'ada@example.test', 'meta' => ['active' => true]], + ['custom_field_12' => null, 'email' => 'grace@example.test'], + ]); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertStringContainsString('(`email`, `meta`, `custom_field_12`)', $sql); + $this->assertStringContainsString("('ada@example.test', '{\"active\":true}', NULL)", $sql); + $this->assertStringContainsString("('grace@example.test', NULL, NULL)", $sql); + $this->assertStringContainsString('`email` = VALUES(`email`)', $sql); + $this->assertStringContainsString('`meta` = VALUES(`meta`)', $sql); + } + + public function testTimestampedUpsertAddsManagedColumnsOnlyOnce(): void + { + TimestampedRow::query()->upsert([ + ['email' => 'ada@example.test', 'created_at' => '2020-01-01 00:00:00'], + ['email' => 'grace@example.test', 'updated_at' => '2020-01-02 00:00:00'], + ]); + + $sql = $GLOBALS['wpdb']->last_query; + + $this->assertSame(1, substr_count($this->columnList($sql), '`created_at`')); + $this->assertSame(1, substr_count($this->columnList($sql), '`updated_at`')); + $this->assertStringContainsString("('ada@example.test', '2020-01-01 00:00:00', NULL)", $sql); + $this->assertStringContainsString("('grace@example.test', NULL, '2020-01-02 00:00:00')", $sql); + } + + public function testSingleWritesRenderQuotedIdentifiers(): void + { + $GLOBALS['wpdb']->insert_id = 1; + User::query()->insert(['custom_field_12' => null]); + $this->assertStringContainsString('(`custom_field_12`) VALUES (NULL)', $GLOBALS['wpdb']->last_query); + + User::query()->update(['custom_field_12' => null]); + $this->assertStringContainsString('SET `custom_field_12` = NULL', $GLOBALS['wpdb']->last_query); + + $user = new User(['custom_field_12' => null]); + $user->setExists(false); + $user->save(); + $this->assertStringContainsString('(`custom_field_12`) VALUES (NULL)', $GLOBALS['wpdb']->last_query); + } + + private function assertWriteIsRejected(callable $write): void + { + try { + $write(); + $this->fail('Expected hostile write schema to be rejected.'); + } catch (RuntimeException $exception) { + $this->assertSame('Invalid SQL identifier.', $exception->getMessage()); + } + + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + + private function columnList(string $sql): string + { + preg_match('/INSERT INTO [^(]+(\([^)]*\))/', $sql, $matches); + + return $matches[1] ?? ''; + } +} From 7c8b0acd8a9b624770e0606e457d2d78dc0dafe9 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 02:20:53 +0600 Subject: [PATCH 07/11] fix(query): reject empty prepared writes --- src/QueryBuilder.php | 8 ++++++++ tests/WriteIdentifierSafetyTest.php | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index f5b16a8..0ecfcf1 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1464,6 +1464,9 @@ public function update($attributes = []) } $this->update = $this->prepareAttributeForSaveOrUpdate(true); + if (empty($this->update)) { + return false; + } return $this->exec(); } @@ -2349,6 +2352,11 @@ private function compileWriteValue($value) */ private function bulkInsert($attributes) { + [$callerColumns] = $this->normalizeWriteRows($attributes); + if (empty($callerColumns)) { + return new Collection([]); + } + [$columns, $rows] = $this->normalizeWriteRows( $attributes, property_exists($this->_model, 'timestamps') && $this->_model->timestamps ? ['created_at'] : [] diff --git a/tests/WriteIdentifierSafetyTest.php b/tests/WriteIdentifierSafetyTest.php index 57fbdef..bca9ff9 100644 --- a/tests/WriteIdentifierSafetyTest.php +++ b/tests/WriteIdentifierSafetyTest.php @@ -2,6 +2,8 @@ namespace BitApps\WPDatabase\Tests; +use BitApps\WPDatabase\Collection; +use BitApps\WPDatabase\Tests\Fixtures\CreatingUser; use BitApps\WPDatabase\Tests\Fixtures\TimestampedRow; use BitApps\WPDatabase\Tests\Fixtures\User; use FakeWpdb; @@ -91,6 +93,24 @@ public function testSaveNewModelWithoutAttributesReturnsFalseWithoutQuery(): voi $this->assertSame([], $GLOBALS['wpdb']->queries); } + public function testUpdateWithOnlyRejectedFillableAttributesReturnsFalseWithoutQuery(): void + { + $query = CreatingUser::query(); + $query->getModel()->setExists(false); + + $this->assertFalse($query->update(['email' => 'ada@example.test'])); + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + + public function testTimestampedBulkInsertWithOnlyEmptyRowsReturnsEmptyCollectionWithoutQuery(): void + { + $result = TimestampedRow::query()->insert([[], []]); + + $this->assertInstanceOf(Collection::class, $result); + $this->assertCount(0, $result); + $this->assertSame([], $GLOBALS['wpdb']->queries); + } + public function testBulkInsertNormalizesFirstSeenUnionAndRaggedValues(): void { $GLOBALS['wpdb']->rows_affected = 0; From 6f4bd58850d6952cd15eca50ed7b1f69dd66f5a9 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 02:32:30 +0600 Subject: [PATCH 08/11] feat(query): add typed raw templates --- docs/breaking-changes.md | 63 +++++- docs/usage.md | 72 ++++++- src/Query/RawTemplate.php | 144 +++++++++++++ src/QueryBuilder.php | 54 ++++- tests/Query/RawTemplateTest.php | 358 ++++++++++++++++++++++++++++++++ 5 files changed, 682 insertions(+), 9 deletions(-) create mode 100644 src/Query/RawTemplate.php create mode 100644 tests/Query/RawTemplateTest.php diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 4a528b1..ba75576 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -32,6 +32,7 @@ API and runtime behavior change, it is a **major** version bump. | 13 | Model | soft-delete reads exclude trashed by default (opt out with `$soft_delete_scope = false`) | Medium | | 14 | Composer | minimum PHP raised to **8.0** (was 7.4) — package no longer installs on PHP < 8.0 | High | | 15 | QueryBuilder | structured identifiers/operators/joins now fail closed | High | +| 16 | QueryBuilder | typed `rawPrepared()` added; legacy `raw()` deprecated | High | --- @@ -386,6 +387,56 @@ bind both bounds; hostile column text can no longer enter their SQL fragment. Base-table aliases created with `from()` are SELECT-only; UPDATE and DELETE chains using them now throw before execution rather than emit an undeclared alias. +Relationship aliases and pivot table/key/column metadata now use the same strict +identifier grammar. Pivot metadata accepts simple segments only; dotted keys and +raw `AS` fragments fail closed. Insert, update, save, bulk-insert, and upsert +attribute keys are validated too. Multi-row writes use a first-seen union schema +and represent missing later-row values as `NULL` instead of shifting or dropping +data. + +### 2.16 Typed raw templates and an explicit unsafe escape hatch + +`rawPrepared()` is the constrained route for complex, developer-authored SQL. +Its template is static application code; supplying request-controlled SQL as the +template is unsupported regardless of bindings. + +```php +$query->rawPrepared( + 'SELECT {{identifier:column}} FROM {{identifier:table}}' + . ' WHERE {{identifier:status}} = %s' + . ' ORDER BY {{identifier:column}} {{direction:sort}}', + ['active'], + [ + 'column' => 'wp_contacts.created_at', + 'table' => 'wp_contacts', + 'status' => 'wp_contacts.status', + ], + ['sort' => 'DESC'] +); +``` + +Only `{{identifier:key}}` and `{{direction:key}}` markers are supported; keys +match `[A-Za-z_][A-Za-z0-9_]*`. Marker maps are consumed exactly, though repeated +markers may reuse one entry. Identifier values are strictly validated and quoted; +directions normalize to `ASC` or `DESC`. + +Templates reject single/double quotes, backticks, comment tokens (`#`, `--`, +`/*`, `*/`), malformed braces, and all semicolons except one optional trailing +terminator. Use unnumbered `%s`, `%d`, `%f`, or `%F` for values and `%%` for a +literal percent. Placeholder and binding counts must match exactly before wpdb is +called. Numbered/flagged placeholders and `%i` are not accepted. + +SQL requiring quoted/comment-like static syntax must use reviewed `unsafeRaw()`. +`raw()` remains functional but is deprecated and delegates to that legacy unsafe +implementation. Neither accepts request interpolation safely. + +```text +structured query -> structured builder APIs +complex static SQL with dynamic structure -> rawPrepared typed markers +fully developer-controlled legacy SQL -> unsafeRaw +request interpolation into raw/unsafeRaw -> unsupported and vulnerable +``` + --- ## 3. Behavioral changes @@ -556,8 +607,9 @@ User::query()->with('posts')->where('active', 1)->get(); ### 4.5 Other additions - **QueryBuilder:** `addSelect()`, `selectRaw()`, `orderByRaw()`, `upsert()`, - `when()`, `toSql()`, `clone()`, `aggregate()`, `prepareColumnName()`, - `withCast()` (chainable), and `__call()` forwarding to the bound model. + `rawPrepared()`, `unsafeRaw()`, `when()`, `toSql()`, `clone()`, `aggregate()`, + `prepareColumnName()`, `withCast()` (chainable), and `__call()` forwarding to + the bound model. - **Model:** `query()` (canonical static builder entry), `toArray()`, `getPrefix()`, `getTablePrefix()` (full table prefix — `wp_` + plugin prefix — for join/pivot table names), `withCast(array $casts)`, `bool`/`boolean` cast. @@ -607,6 +659,7 @@ Out of scope (read-only): `attach`/`detach`/`sync`, and | `QueryBuilder::startTransaction()` | `Connection::startTransaction()` | | `QueryBuilder::commit()` | `Connection::commit()` | | `QueryBuilder::rollback()` | `Connection::rollback()` | +| `QueryBuilder::raw()` | `QueryBuilder::rawPrepared()` or reviewed `QueryBuilder::unsafeRaw()` | Still functional, but migrate — they may be removed in a future release. @@ -625,6 +678,12 @@ Still functional, but migrate — they may be removed in a future release. with raw SQL (§2.5). - [ ] Null-check / coalesce values from nullable cast columns (§2.6). - [ ] Review non-SELECT `raw()` return handling (§2.7). +- [ ] Replace legacy `raw()` calls with structured APIs, `rawPrepared()`, or + reviewed `unsafeRaw()`; remove every request interpolation (§2.16). +- [ ] Remove pre-backticks, expressions, and unknown/schema qualifiers from + structured identifiers; validate relation/pivot and write keys (§2.15). +- [ ] Move join constants to `joinWhere()` / `onValue()` and reviewed join + expressions to `onRaw()` (§2.15). - [ ] Replace the old single-arg `with(Closure)` form and the old `withCount()` no-arg call (§2.8, §2.9). - [ ] Replace direct `addRelation(string)` calls with `with()` (§2.10). diff --git a/docs/usage.md b/docs/usage.md index 2bb0dc4..bca0780 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -193,6 +193,13 @@ Contact::where('is_active', 1)->get(); // filtered → Collection Every static call on a model opens a builder; chain freely. Use `toSql()` to inspect the SQL without executing. +Structured table, column, key, and alias inputs use simple identifier segments +(`A-Z`, `a-z`, `_`, then letters, digits, or `_`). Qualified columns may join +segments with dots where the API permits them. Do not pre-quote identifiers with +backticks or pass expressions through structured arguments. Unknown table/schema +qualifiers fail closed; register the table through the builder or use an explicit, +reviewed raw API. + ### Select ```php @@ -391,6 +398,10 @@ Contact::where('is_active', 0)->update(['status' => 'archived']); > `update()` on a builder executes immediately (it is not chainable). Set conditions > **before** calling it. When updating a model instance through `save()`, the return > value is the `Model` on success or `false` on failure. +> +> Insert, update, save, bulk-insert, and upsert attribute keys must be simple +> identifiers. Bulk operations build a stable union of row keys in first-seen +> order and bind `NULL` for a key missing from a later row. --- @@ -679,14 +690,65 @@ try { ## Raw queries ```php -// SELECT → returns result rows -Contact::query()->raw('SELECT * FROM wp_contacts WHERE id = %d', [1]); +// Complex developer-authored SQL with typed dynamic structure and bound values. +Contact::query()->rawPrepared( + 'SELECT {{identifier:column}} FROM {{identifier:table}}' + . ' WHERE {{identifier:status}} = %s' + . ' ORDER BY {{identifier:column}} {{direction:sort}}', + ['active'], + [ + 'column' => 'wp_contacts.created_at', + 'table' => 'wp_contacts', + 'status' => 'wp_contacts.status', + ], + ['sort' => 'DESC'] +); + +// Escape a literal percent as %%, including when no values are bound. +Contact::query()->rawPrepared('SELECT 100 %% 7 AS remainder'); + +// Reviewed, fully developer-controlled legacy SQL. +Contact::query()->unsafeRaw( + "SELECT `id` FROM `wp_contacts` WHERE `status` = %s AND 'fixed' = 'fixed'", + ['active'] +); +``` + +`rawPrepared()` is a static-template boundary: the template must be written by a +developer and must never come from request data, even when a bindings array is +provided. It accepts only these structural markers: + +- `{{identifier:key}}`, compiled from the matching identifier-map entry; +- `{{direction:key}}`, compiled from the matching direction-map entry as `ASC` + or `DESC`. + +Marker keys match `[A-Za-z_][A-Za-z0-9_]*`. Every map entry must be used and +every marker must have a matching entry; duplicate markers reuse one map value. +Identifiers are quoted by the compiler, so the template itself must not contain +backticks. + +Value placeholders are limited to unnumbered `%s`, `%d`, `%f`, and `%F`; use +`%%` for a literal percent. The binding count must match the real placeholder +count exactly. Numbered, flagged, `%i`, dangling, and unknown placeholders are +rejected before `$wpdb->prepare()` or query execution. + +To keep this API conservative rather than pretending to be a SQL lexer, templates +reject single quotes, double quotes, backticks, `#`, `--`, `/*`, `*/`, unresolved +braces, and every semicolon except one optional trailing terminator. SQL that +genuinely requires such static syntax belongs in reviewed `unsafeRaw()` code. + +Choose the narrowest boundary: -// Non-SELECT → returns the wpdb query result -Contact::query()->raw('UPDATE wp_contacts SET is_active = 1 WHERE id = %d', [1]); +```text +structured query -> structured builder APIs +complex static SQL with dynamic structure -> rawPrepared typed markers +fully developer-controlled legacy SQL -> unsafeRaw +request interpolation into raw/unsafeRaw -> unsupported and vulnerable ``` -Placeholders use `$wpdb` conventions (`%d`, `%s`, `%f`) with the bindings array. +`raw()` is deprecated and delegates to `unsafeRaw()` for compatibility. Both +legacy entry points preserve `$wpdb` placeholder behavior, but neither makes an +interpolated or request-provided SQL string safe. --- diff --git a/src/Query/RawTemplate.php b/src/Query/RawTemplate.php new file mode 100644 index 0000000..44bfec0 --- /dev/null +++ b/src/Query/RawTemplate.php @@ -0,0 +1,144 @@ += $length) { + throw new RuntimeException('Raw template contains a dangling percent sign.'); + } + + $specifier = $template[++$index]; + if ($specifier === '%') { + continue; + } + + if (!\in_array($specifier, ['s', 'd', 'f', 'F'], true)) { + throw new RuntimeException('Raw template contains an unsupported placeholder.'); + } + + $placeholderCount++; + } + + if ($placeholderCount !== $bindingCount) { + throw new RuntimeException('Raw-template placeholder and binding counts must match exactly.'); + } + } +} diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 0ecfcf1..f100fb3 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -6,6 +6,7 @@ use BitApps\WPDatabase\Query\Grammar; use BitApps\WPDatabase\Query\Identifier; use BitApps\WPDatabase\Query\JoinType; +use BitApps\WPDatabase\Query\RawTemplate; use BitApps\WPDatabase\Query\SqlOperator; use Closure; @@ -1360,14 +1361,47 @@ public function desc() } /** - * Runs raw query + * Runs a constrained, developer-authored SQL template. + * + * Dynamic values must use wpdb placeholders. Dynamic identifiers and sort + * directions must use typed markers and exactly matching maps. Request data + * must never supply the template itself. + * + * @return mixed + */ + public function rawPrepared( + string $template, + array $bindings = [], + array $identifiers = [], + array $directions = [] + ) { + $sql = RawTemplate::compile($template, $bindings, $identifiers, $directions); + + if ($bindings === []) { + $sql = str_replace('%%', '%', $sql); + } else { + $prepared = Connection::prepare($sql, $bindings); + if (!\is_string($prepared) || $prepared === '') { + throw new RuntimeException('SQL query preparation failed.'); + } + $sql = $prepared; + } + + return $this->unsafeRaw($sql); + } + + /** + * Runs reviewed, fully developer-controlled SQL through the legacy path. + * + * Bind every dynamic value. Request data must never be interpolated into + * the SQL string. * * @param string $sql * @param array $bindings * * @return mixed */ - public function raw($sql, $bindings = []) + public function unsafeRaw(string $sql, array $bindings = []) { $this->_method = self::RAW; $this->raw = $sql; @@ -1385,6 +1419,22 @@ public function raw($sql, $bindings = []) return $result; } + /** + * Runs raw SQL through the legacy unsafe implementation. + * + * @deprecated Use rawPrepared() for typed templates or unsafeRaw() for + * reviewed, fully developer-controlled legacy SQL. + * + * @param string $sql + * @param array $bindings + * + * @return mixed + */ + public function raw($sql, $bindings = []) + { + return $this->unsafeRaw($sql, $bindings); + } + public function prepareRaw() { return $this->raw; diff --git a/tests/Query/RawTemplateTest.php b/tests/Query/RawTemplateTest.php new file mode 100644 index 0000000..e2bd3cf --- /dev/null +++ b/tests/Query/RawTemplateTest.php @@ -0,0 +1,358 @@ + 'users.id', 'table' => 'users'], + ['sort' => 'desc'] + ); + + $this->assertSame( + 'SELECT `users`.`id` FROM `users` ORDER BY `users`.`id` DESC', + $sql + ); + } + + public static function invalidMarkerProvider(): array + { + return [ + 'missing identifier map entry' => [ + 'SELECT {{identifier:column}}', + [], + [], + ], + 'missing direction map entry' => [ + 'SELECT id {{direction:sort}}', + [], + [], + ], + 'unused identifier map entry' => [ + 'SELECT id', + ['column' => 'id'], + [], + ], + 'unused direction map entry' => [ + 'SELECT id', + [], + ['sort' => 'ASC'], + ], + 'unknown marker kind' => [ + 'SELECT {{column:name}}', + ['name' => 'id'], + [], + ], + 'invalid marker key' => [ + 'SELECT {{identifier:1name}}', + ['1name' => 'id'], + [], + ], + 'unclosed marker' => [ + 'SELECT {{identifier:name}', + ['name' => 'id'], + [], + ], + 'stray opening brace' => [ + 'SELECT {id', + [], + [], + ], + 'stray closing brace' => [ + 'SELECT id}', + [], + [], + ], + ]; + } + + #[DataProvider('invalidMarkerProvider')] + public function testRejectsMalformedOrInexactMarkerMaps( + string $template, + array $identifiers, + array $directions + ): void { + $this->expectException(RuntimeException::class); + + RawTemplate::compile($template, [], $identifiers, $directions); + } + + public static function invalidIdentifierProvider(): array + { + return [ + 'empty' => [''], + 'leading digit' => ['1column'], + 'empty segment' => ['users..id'], + 'wildcard' => ['users.*'], + 'implicit alias' => ['id alias'], + 'expression' => ['LOWER(id)'], + ]; + } + + #[DataProvider('invalidIdentifierProvider')] + public function testRejectsInvalidIdentifierMarkerValues(string $identifier): void + { + $this->expectException(RuntimeException::class); + + RawTemplate::compile( + 'SELECT {{identifier:column}}', + [], + ['column' => $identifier] + ); + } + + public static function invalidDirectionProvider(): array + { + return [ + 'empty' => [''], + 'nulls last' => ['DESC NULLS LAST'], + 'comment' => ['ASC--'], + 'expression' => ['RAND()'], + 'non-string' => [1], + ]; + } + + #[DataProvider('invalidDirectionProvider')] + public function testRejectsInvalidDirectionMarkerValues($direction): void + { + $this->expectException(RuntimeException::class); + + RawTemplate::compile( + 'SELECT id ORDER BY id {{direction:sort}}', + [], + [], + ['sort' => $direction] + ); + } + + public static function forbiddenTemplateTokenProvider(): array + { + return [ + 'single quote' => ["SELECT 'literal'"], + 'double quote' => ['SELECT "literal"'], + 'backtick' => ['SELECT `id`'], + 'hash comment' => ['SELECT id # comment'], + 'dash comment' => ['SELECT id -- comment'], + 'block comment open' => ['SELECT /* comment'], + 'block comment close' => ['SELECT comment */ id'], + ]; + } + + #[DataProvider('forbiddenTemplateTokenProvider')] + public function testRejectsEveryForbiddenTemplateToken(string $template): void + { + $this->expectException(RuntimeException::class); + + RawTemplate::compile($template); + } + + public function testAcceptsOnlySupportedUnnumberedValuePlaceholdersAndEscapedPercent(): void + { + $this->assertSame( + 'SELECT %s, %d, %f, %F, %%', + RawTemplate::compile('SELECT %s, %d, %f, %F, %%', ['x', 1, 1.5, 2.5]) + ); + } + + public static function invalidPlaceholderProvider(): array + { + return [ + 'dangling percent' => ['SELECT %', []], + 'numbered placeholder' => ['SELECT %1$s', ['x']], + 'flagged placeholder' => ['SELECT %05d', [1]], + 'identifier placeholder' => ['SELECT %i', ['id']], + 'unknown placeholder' => ['SELECT %q', ['x']], + 'binding without marker' => ['SELECT 1', [1]], + 'missing binding' => ['SELECT %s', []], + 'extra binding' => ['SELECT %s', ['x', 'y']], + ]; + } + + #[DataProvider('invalidPlaceholderProvider')] + public function testRejectsInvalidPlaceholderGrammarOrBindingCount(string $template, array $bindings): void + { + $this->expectException(RuntimeException::class); + + RawTemplate::compile($template, $bindings); + } + + public function testAllowsZeroPlaceholdersWithZeroBindings(): void + { + $this->assertSame('SELECT 1', RawTemplate::compile('SELECT 1')); + } + + public function testAllowsOneOptionalTrailingSemicolonAfterTrimmingOuterWhitespace(): void + { + $this->assertSame('SELECT %d;', RawTemplate::compile(" \nSELECT %d;\t", [1])); + } + + public static function invalidSemicolonProvider(): array + { + return [ + 'embedded terminator' => ['SELECT 1; SELECT 2'], + 'two terminators' => ['SELECT 1;;'], + 'semicolon before suffix' => ['SELECT 1; -- suffix'], + 'quoted literal' => ["SELECT 'x;y'"], + ]; + } + + #[DataProvider('invalidSemicolonProvider')] + public function testRejectsEveryNonTrailingOrRepeatedSemicolon(string $template): void + { + $this->expectException(RuntimeException::class); + + RawTemplate::compile($template); + } + + public function testDoesNotBlacklistSqlKeywords(): void + { + $this->assertSame( + 'SELECT SLEEP(%d) UNION SELECT %d', + RawTemplate::compile('SELECT SLEEP(%d) UNION SELECT %d', [0, 1]) + ); + } + + public function testRawPreparedCompilesThenPreparesAndExecutes(): void + { + $wpdb = new RawTemplateTrackingWpdb(); + $GLOBALS['wpdb'] = $wpdb; + $rows = [(object) ['id' => 7]]; + $wpdb->queueResult($rows); + + $result = User::query()->rawPrepared( + 'SELECT {{identifier:column}} FROM {{identifier:table}}' + . ' WHERE {{identifier:column}} = %d ORDER BY {{identifier:column}} {{direction:sort}}', + [7], + ['column' => 'wp_users.id', 'table' => 'wp_users'], + ['sort' => 'desc'] + ); + + $this->assertSame($rows, $result); + $this->assertSame(1, $wpdb->prepareCalls); + $this->assertSame( + 'SELECT `wp_users`.`id` FROM `wp_users` WHERE `wp_users`.`id` = 7' + . ' ORDER BY `wp_users`.`id` DESC', + $wpdb->last_query + ); + } + + public function testRawPreparedBypassesPrepareWhenThereAreNoPlaceholdersOrBindings(): void + { + $wpdb = new RawTemplateTrackingWpdb(); + $GLOBALS['wpdb'] = $wpdb; + + User::query()->rawPrepared('SELECT 100 %% 7'); + + $this->assertSame(0, $wpdb->prepareCalls); + $this->assertSame(['SELECT 100 % 7'], $wpdb->queries); + } + + public function testRawPreparedRejectsBeforePrepareOrExecution(): void + { + $wpdb = new RawTemplateTrackingWpdb(); + $GLOBALS['wpdb'] = $wpdb; + + try { + User::query()->rawPrepared('SELECT %s'); + $this->fail('A missing binding must be rejected.'); + } catch (RuntimeException $exception) { + $this->assertStringContainsString('placeholder', strtolower($exception->getMessage())); + $this->assertSame(0, $wpdb->prepareCalls); + $this->assertSame([], $wpdb->queries); + } + } + + public function testRawPreparedDoesNotExecuteWhenWpdbPreparationFails(): void + { + $wpdb = new RawTemplateTrackingWpdb(); + $wpdb->prepareFailure = true; + $GLOBALS['wpdb'] = $wpdb; + + try { + User::query()->rawPrepared('SELECT %s', ['value']); + $this->fail('A failed wpdb preparation must be rejected.'); + } catch (RuntimeException $exception) { + $this->assertStringContainsString('preparation failed', strtolower($exception->getMessage())); + $this->assertSame(1, $wpdb->prepareCalls); + $this->assertSame([], $wpdb->queries); + } + } + + public function testRawPreparedPreservesNonSelectResultSemantics(): void + { + $wpdb = new RawTemplateTrackingWpdb(); + $wpdb->rows_affected = 3; + $GLOBALS['wpdb'] = $wpdb; + + $result = User::query()->rawPrepared('UPDATE contacts SET active = %d', [1]); + + $this->assertSame(3, $result); + } + + public function testUnsafeRawPreservesTheReviewedLegacyEscapeHatch(): void + { + $wpdb = new RawTemplateTrackingWpdb(); + $GLOBALS['wpdb'] = $wpdb; + + User::query()->unsafeRaw( + "SELECT `id` FROM `wp_users` WHERE `name` = %s AND 'static' = 'static' # reviewed", + ['Ada'] + ); + + $this->assertSame(1, $wpdb->prepareCalls); + $this->assertSame( + "SELECT `id` FROM `wp_users` WHERE `name` = 'Ada' AND 'static' = 'static' # reviewed", + $wpdb->last_query + ); + } + + public function testDeprecatedRawRetainsLegacyUnsafeBehavior(): void + { + $wpdb = new RawTemplateTrackingWpdb(); + $GLOBALS['wpdb'] = $wpdb; + + User::query()->raw("SELECT 'legacy'"); + + $this->assertSame("SELECT 'legacy'", $wpdb->last_query); + } +} + +final class RawTemplateTrackingWpdb extends FakeWpdb +{ + public $prepareCalls = 0; + + public $prepareFailure = false; + + public function prepare($query, ...$args) + { + $this->prepareCalls++; + + if ($this->prepareFailure) { + return false; + } + + return parent::prepare($query, ...$args); + } +} From f0dace4113c6bf9f3bd8344da2f0e4e869f3ca80 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 02:45:35 +0600 Subject: [PATCH 09/11] chore(release): prepare hardened package line --- composer.json | 2 +- docs/breaking-changes.md | 6 + docs/usage.md | 6 + src/Concerns/Relations.php | 45 ++--- src/Connection.php | 4 +- src/Model.php | 4 +- src/Query/Grammar.php | 2 + src/QueryBuilder.php | 321 ++++++++++++++++++-------------- tests/Query/RawTemplateTest.php | 11 ++ tests/bootstrap.php | 4 +- 10 files changed, 235 insertions(+), 170 deletions(-) diff --git a/composer.json b/composer.json index 06d3e35..2a2c6aa 100644 --- a/composer.json +++ b/composer.json @@ -50,7 +50,7 @@ }, "extra": { "branch-alias": { - "dev-main": "1.x-dev" + "dev-main": "2.x-dev" } }, "config": { diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index ba75576..33bf8a2 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -162,6 +162,12 @@ column and keeps the alias separate, so `->select(['id', 'title AS t'])` emits ->selectRaw('SUM(amount) as amt', $bindings) ``` +Projection order is channel-based rather than call-based: structured +`select()` / `addSelect()` columns compile first, generated relation +aggregate/existence columns compile next, and `selectRaw()` expressions compile +last. Calling these methods in another order does not interleave the resulting +columns. + --- ### 2.5 `delete()` with no `WHERE` clause no longer wipes the table diff --git a/docs/usage.md b/docs/usage.md index bca0780..2f92d19 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -215,6 +215,12 @@ Contact::selectRaw('SUM(amount) as amt', [])->get(); > `select()` handles plain columns and `column AS alias`, back-tick-quoting them > as identifiers. Pass raw SQL expressions or function calls (`COUNT(*)`, …) to > `selectRaw()` instead. +> +> Projection channels compile in a stable order regardless of call order: +> structured `select()` / `addSelect()` columns first, framework-generated +> relation aggregate/existence columns next, and `selectRaw()` expressions last. +> Do not rely on method-call order to position a raw expression between generated +> relation columns. ### Where diff --git a/src/Concerns/Relations.php b/src/Concerns/Relations.php index 12bb0c8..49355b2 100644 --- a/src/Concerns/Relations.php +++ b/src/Concerns/Relations.php @@ -10,6 +10,7 @@ use BitApps\WPDatabase\Query\Identifier; use BitApps\WPDatabase\QueryBuilder; use Closure; +use ReflectionMethod; use RuntimeException; if (!\defined('ABSPATH')) { @@ -24,7 +25,9 @@ trait Relations private $_relationKeys = []; - /** Memoized framework-vs-consumer verdict per "class::method". */ + /** + * Memoized framework-vs-consumer verdict per "class::method". + */ private static $relationMethodCache = []; /** @@ -272,6 +275,23 @@ public function prepareRelation(array $relations): array return $preparedRelation; } + public function prepareRelationName(string $relationName): array + { + $parts = preg_split('/\s+as\s+/i', $relationName); + if ($parts === false || \count($parts) > 2) { + throw new RuntimeException('Invalid relation name or alias.'); + } + + $name = $parts[0]; + $alias = isset($parts[1]) ? $parts[1] : null; + $this->assertSimpleRelationIdentifier($name); + if ($alias !== null) { + $this->assertSimpleRelationIdentifier($alias); + } + + return [$name, $alias]; + } + /** * Resolves an existing method name to its relation query, or fails loudly. * @@ -312,12 +332,12 @@ private function resolveRelationQuery($method) */ private function isFrameworkModelMethod($method) { - $cacheKey = \get_class($this) . '::' . $method; + $cacheKey = static::class . '::' . $method; if (isset(self::$relationMethodCache[$cacheKey])) { return self::$relationMethodCache[$cacheKey]; } - $declaringClass = (new \ReflectionMethod($this, $method))->getDeclaringClass()->getName(); + $declaringClass = (new ReflectionMethod($this, $method))->getDeclaringClass()->getName(); return self::$relationMethodCache[$cacheKey] = $declaringClass === Model::class; } @@ -347,24 +367,7 @@ private function isRelationQuery($result) private function undefinedRelationMessage($method) { - return 'Relation [' . $method . '] is not defined on [' . \get_class($this) . '].'; - } - - public function prepareRelationName(string $relationName): array - { - $parts = preg_split('/\s+as\s+/i', $relationName); - if ($parts === false || \count($parts) > 2) { - throw new RuntimeException('Invalid relation name or alias.'); - } - - $name = $parts[0]; - $alias = isset($parts[1]) ? $parts[1] : null; - $this->assertSimpleRelationIdentifier($name); - if ($alias !== null) { - $this->assertSimpleRelationIdentifier($alias); - } - - return [$name, $alias]; + return 'Relation [' . $method . '] is not defined on [' . static::class . '].'; } /** diff --git a/src/Connection.php b/src/Connection.php index b248cae..84d8b28 100644 --- a/src/Connection.php +++ b/src/Connection.php @@ -134,7 +134,7 @@ public static function prop($var) /** * Starts transaction - * + * * @return bool */ public static function startTransaction() @@ -154,7 +154,7 @@ public static function commit() /** * Rollback previously execute query - * + * * @return void */ public static function rollback() diff --git a/src/Model.php b/src/Model.php index 276fb91..3386d74 100644 --- a/src/Model.php +++ b/src/Model.php @@ -307,7 +307,9 @@ public function getTable() return $this->table; } - /** Query/schema default prefix; NOT the full table prefix — use getTablePrefix() for join/pivot table names (it keeps wp_ for custom $prefix). */ + /** + * Query/schema default prefix; NOT the full table prefix — use getTablePrefix() for join/pivot table names (it keeps wp_ for custom $prefix). + */ public function getPrefix() { return $this->prefix === '' ? Connection::getPrefix() : $this->prefix; diff --git a/src/Query/Grammar.php b/src/Query/Grammar.php index 32a8806..bfedce8 100644 --- a/src/Query/Grammar.php +++ b/src/Query/Grammar.php @@ -246,6 +246,8 @@ private function getOffset(QueryBuilder $query) /** * Compiles the raw select columns and merges their bindings. * + * @param mixed $hasStructuredSelect + * * @return string */ private function prepareRawSelect(QueryBuilder $query, $hasStructuredSelect = false) diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index f100fb3..d770d0f 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -464,56 +464,6 @@ public function getSelectExpressions() return $this->selectExpressions; } - /** - * Adds an aggregate SELECT expression without routing its identifier - * through the raw SQL channel. - * - * @param mixed $function - * @param mixed $column - * @param null|string $alias - * - * @return $this - */ - private function addAggregateSelect($function, $column, $alias = null) - { - $this->assertSafeAggregateFunction($function); - if ($column !== '*') { - $this->assertSafeIdentifier($column); - } - if ($alias !== null) { - Identifier::assertSimple($alias); - } - - $this->selectExpressions[] = [ - 'type' => 'aggregate', - 'function' => $function, - 'column' => $column, - 'alias' => $alias, - ]; - - return $this; - } - - /** - * Adds a relation subquery SELECT expression with a validated output alias. - * The nested builder remains structured until Grammar compilation. - * - * @return $this - */ - private function addSubquerySelect(QueryBuilder $query, string $alias, $exists = false) - { - Identifier::assertSimple($alias); - - $this->selectExpressions[] = [ - 'type' => 'subquery', - 'query' => $query, - 'alias' => $alias, - 'exists' => (bool) $exists, - ]; - - return $this; - } - /** * Selects raw query as column for query * @@ -928,6 +878,12 @@ public function join($table, $firstColumn, $operator = null, $secondColumn = nul /** * Joins a table using a bound scalar value as the right-hand operand. * + * @param mixed $table + * @param mixed $firstColumn + * @param mixed $operator + * @param mixed $value + * @param mixed $type + * * @return $this */ public function joinWhere($table, $firstColumn, $operator, $value, $type = 'INNER') @@ -944,54 +900,6 @@ public function joinWhere($table, $firstColumn, $operator, $value, $type = 'INNE ); } - /** - * Parses a structured join table declaration with an optional explicit - * `AS` alias. Both identifiers remain logical state until compilation. - * - * @return array{string, null|string, string, string} - */ - private function parseJoinTable($table) - { - if (!\is_string($table) - || !preg_match('/^([A-Za-z_][A-Za-z0-9_]*)(?:\s+AS\s+([A-Za-z_][A-Za-z0-9_]*))?$/i', $table, $matches) - ) { - throw new RuntimeException('Invalid SQL join table declaration.'); - } - - $rawTable = $matches[1]; - $alias = isset($matches[2]) ? $matches[2] : null; - $prefixedTable = $this->_model->getTablePrefix() . $rawTable; - $reference = $alias !== null ? $alias : $prefixedTable; - - Identifier::assertSimple($rawTable); - Identifier::assertSimple($prefixedTable); - if ($alias !== null) { - Identifier::assertSimple($alias); - } - - return [$rawTable, $alias, $prefixedTable, $reference]; - } - - /** - * Stores one validated join and its first ON condition. - * - * @return $this - */ - private function storeJoin($rawTable, $alias, $prefixedTable, $reference, array $on, $type) - { - $this->joins[] = [ - 'table' => $prefixedTable, - 'alias' => $reference, - 'on' => [$on], - 'type' => JoinType::normalize($type), - 'raw' => $rawTable, - 'prefixed' => $prefixedTable, - 'userAlias' => $alias, - ]; - - return $this; - } - /** * Maps each unprefixed table name this query knows about (the model's own * table plus every non-aliased join) to its physical, prefixed name. @@ -1105,24 +1013,6 @@ public function renderIdentifier(string $column, bool $allowWildcard = false, bo return Identifier::quoteQualified($resolved . '.' . $name, $allowWildcard); } - /** - * Creates a nested builder that compiles its conditions inside this query's - * table context: it shares the model (via newQuery()) plus this query's - * joins and from() alias, so resolveQualifier() inside a nested where group - * resolves joined-table qualifiers exactly as at top level. The joins stay - * inert — a nested builder compiles only its conditions, never JOIN SQL. - * - * @return QueryBuilder - */ - private function newNestedQuery() - { - $query = $this->newQuery(); - $query->joins = $this->joins; - $query->_from = $this->_from; - - return $query; - } - /** * Sets left join * @@ -1223,6 +1113,11 @@ public function orOn($firstColumn, $operator = null, $secondColumn = null) /** * Adds an ON condition whose right-hand operand is a bound scalar value. * + * @param mixed $firstColumn + * @param mixed $operator + * @param mixed $value + * @param mixed $bool + * * @return $this */ public function onValue($firstColumn, $operator, $value, $bool = 'AND') @@ -1240,6 +1135,10 @@ public function onValue($firstColumn, $operator, $value, $bool = 'AND') /** * Adds an OR ON condition whose right-hand operand is a bound scalar value. * + * @param mixed $firstColumn + * @param mixed $operator + * @param mixed $value + * * @return $this */ public function orOnValue($firstColumn, $operator, $value) @@ -1251,6 +1150,9 @@ public function orOnValue($firstColumn, $operator, $value) * Adds an explicitly raw ON condition. The SQL must be developer-authored; * dynamic values belong in placeholders and $bindings. * + * @param mixed $sql + * @param mixed $bool + * * @return $this */ public function onRaw($sql, array $bindings = [], $bool = 'AND') @@ -1276,6 +1178,8 @@ public function onRaw($sql, array $bindings = [], $bool = 'AND') /** * Adds an explicitly raw OR ON condition. * + * @param mixed $sql + * * @return $this */ public function orOnRaw($sql, array $bindings = []) @@ -1644,23 +1548,6 @@ public function aggregate($function, $column) return \is_array($result) && isset($result[0]->{$function}) ? $result[0]->{$function} : null; } - /** - * Guards an aggregate function name that is interpolated straight into SQL: - * only a bare identifier is allowed, so parens/spaces/semicolons cannot - * smuggle in a payload. Case is preserved (SQL function names are - * case-insensitive). - * - * @param mixed $function - * - * @return void - */ - private function assertSafeAggregateFunction($function) - { - if (!\is_string($function) || !preg_match('/^[A-Za-z_][A-Za-z0-9_]*$/', $function)) { - throw new RuntimeException('Invalid aggregate function name.'); - } - } - public function delete() { $this->_method = self::DELETE; @@ -2008,6 +1895,11 @@ protected function prepareOnColumn($table, $column, $operator, $secondColumn, $b /** * Prepares a join condition with a bound right-hand value. * + * @param mixed $column + * @param mixed $operator + * @param mixed $value + * @param mixed $bool + * * @return array */ protected function prepareOnValue($column, $operator, $value, $bool = 'AND') @@ -2057,12 +1949,155 @@ protected function getTimeZone() $absHour = abs($hours); $absMins = abs($minutes * 60); - $timezoneString = sprintf('%s%02d:%02d', $sign, $absHour, $absMins); + $timezoneString = \sprintf('%s%02d:%02d', $sign, $absHour, $absMins); } return $timezoneString; } + /** + * Adds an aggregate SELECT expression without routing its identifier + * through the raw SQL channel. + * + * @param mixed $function + * @param mixed $column + * @param null|string $alias + * + * @return $this + */ + private function addAggregateSelect($function, $column, $alias = null) + { + $this->assertSafeAggregateFunction($function); + if ($column !== '*') { + $this->assertSafeIdentifier($column); + } + if ($alias !== null) { + Identifier::assertSimple($alias); + } + + $this->selectExpressions[] = [ + 'type' => 'aggregate', + 'function' => $function, + 'column' => $column, + 'alias' => $alias, + ]; + + return $this; + } + + /** + * Adds a relation subquery SELECT expression with a validated output alias. + * The nested builder remains structured until Grammar compilation. + * + * @param mixed $exists + * + * @return $this + */ + private function addSubquerySelect(QueryBuilder $query, string $alias, $exists = false) + { + Identifier::assertSimple($alias); + + $this->selectExpressions[] = [ + 'type' => 'subquery', + 'query' => $query, + 'alias' => $alias, + 'exists' => (bool) $exists, + ]; + + return $this; + } + + /** + * Parses a structured join table declaration with an optional explicit + * `AS` alias. Both identifiers remain logical state until compilation. + * + * @param mixed $table + * + * @return array{string, null|string, string, string} + */ + private function parseJoinTable($table) + { + if (!\is_string($table) + || !preg_match('/^([A-Za-z_][A-Za-z0-9_]*)(?:\s+AS\s+([A-Za-z_][A-Za-z0-9_]*))?$/i', $table, $matches) + ) { + throw new RuntimeException('Invalid SQL join table declaration.'); + } + + $rawTable = $matches[1]; + $alias = isset($matches[2]) ? $matches[2] : null; + $prefixedTable = $this->_model->getTablePrefix() . $rawTable; + $reference = $alias !== null ? $alias : $prefixedTable; + + Identifier::assertSimple($rawTable); + Identifier::assertSimple($prefixedTable); + if ($alias !== null) { + Identifier::assertSimple($alias); + } + + return [$rawTable, $alias, $prefixedTable, $reference]; + } + + /** + * Stores one validated join and its first ON condition. + * + * @param mixed $rawTable + * @param mixed $alias + * @param mixed $prefixedTable + * @param mixed $reference + * @param mixed $type + * + * @return $this + */ + private function storeJoin($rawTable, $alias, $prefixedTable, $reference, array $on, $type) + { + $this->joins[] = [ + 'table' => $prefixedTable, + 'alias' => $reference, + 'on' => [$on], + 'type' => JoinType::normalize($type), + 'raw' => $rawTable, + 'prefixed' => $prefixedTable, + 'userAlias' => $alias, + ]; + + return $this; + } + + /** + * Creates a nested builder that compiles its conditions inside this query's + * table context: it shares the model (via newQuery()) plus this query's + * joins and from() alias, so resolveQualifier() inside a nested where group + * resolves joined-table qualifiers exactly as at top level. The joins stay + * inert — a nested builder compiles only its conditions, never JOIN SQL. + * + * @return QueryBuilder + */ + private function newNestedQuery() + { + $query = $this->newQuery(); + $query->joins = $this->joins; + $query->_from = $this->_from; + + return $query; + } + + /** + * Guards an aggregate function name that is interpolated straight into SQL: + * only a bare identifier is allowed, so parens/spaces/semicolons cannot + * smuggle in a payload. Case is preserved (SQL function names are + * case-insensitive). + * + * @param mixed $function + * + * @return void + */ + private function assertSafeAggregateFunction($function) + { + if (!\is_string($function) || !preg_match('/^[A-Za-z_][A-Za-z0-9_]*$/', $function)) { + throw new RuntimeException('Invalid aggregate function name.'); + } + } + /** * Coerces each IN-list element to a scalar so it maps to exactly one * placeholder and one binding (a nested array/object is JSON-encoded, never @@ -2147,7 +2182,7 @@ private function normalizeConditions(array $conditions, $type) if (\is_array($value)) { $operator = 'IN'; if (isset($conditions['operator'])) { - $operator = SqlOperator::normalizeList($conditions['operator']); + $operator = SqlOperator::normalizeList($conditions['operator']); $conditions['operator'] = $operator; } @@ -2209,6 +2244,8 @@ private function nullOperator($operator) * Normalizes a structured boolean connector without accepting whitespace * or comments around it. * + * @param mixed $bool + * * @return string */ private function normalizeBoolean($bool) @@ -2305,7 +2342,7 @@ private function normalizeWriteColumns(array $columns) * @param array $rows * @param array $managedTimestampColumns * - * @return array{0: array, 1: array} + * @return array{0: array, 1: array, 2: array} */ private function normalizeWriteRows(array $rows, array $managedTimestampColumns = []) { @@ -2322,6 +2359,7 @@ private function normalizeWriteRows(array $rows, array $managedTimestampColumns } } + $callerColumns = $columns; $generatedTimestamps = []; foreach ($managedTimestampColumns as $column) { $this->normalizeWriteColumns([$column]); @@ -2346,7 +2384,7 @@ private function normalizeWriteRows(array $rows, array $managedTimestampColumns $normalizedRows[] = $normalizedRow; } - return [$columns, $normalizedRows]; + return [$columns, $normalizedRows, $callerColumns]; } /** @@ -2402,16 +2440,11 @@ private function compileWriteValue($value) */ private function bulkInsert($attributes) { - [$callerColumns] = $this->normalizeWriteRows($attributes); - if (empty($callerColumns)) { - return new Collection([]); - } - - [$columns, $rows] = $this->normalizeWriteRows( + [$columns, $rows, $callerColumns] = $this->normalizeWriteRows( $attributes, property_exists($this->_model, 'timestamps') && $this->_model->timestamps ? ['created_at'] : [] ); - if (empty($columns)) { + if (empty($callerColumns)) { return new Collection([]); } diff --git a/tests/Query/RawTemplateTest.php b/tests/Query/RawTemplateTest.php index e2bd3cf..805150a 100644 --- a/tests/Query/RawTemplateTest.php +++ b/tests/Query/RawTemplateTest.php @@ -269,6 +269,17 @@ public function testRawPreparedBypassesPrepareWhenThereAreNoPlaceholdersOrBindin $this->assertSame(['SELECT 100 % 7'], $wpdb->queries); } + public function testRawPreparedBindsAValueAndUnescapesALiteralPercent(): void + { + $wpdb = new RawTemplateTrackingWpdb(); + $GLOBALS['wpdb'] = $wpdb; + + User::query()->rawPrepared('SELECT %d, 100 %% 7', [7]); + + $this->assertSame(1, $wpdb->prepareCalls); + $this->assertSame(['SELECT 7, 100 % 7'], $wpdb->queries); + } + public function testRawPreparedRejectsBeforePrepareOrExecution(): void { $wpdb = new RawTemplateTrackingWpdb(); diff --git a/tests/bootstrap.php b/tests/bootstrap.php index a84c415..aa80570 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -97,7 +97,7 @@ public function prepare($query, ...$args) $index = 0; - return preg_replace_callback( + $prepared = preg_replace_callback( '/%[dsfF]/', function ($match) use (&$index, $args) { $value = $args[$index] ?? ''; @@ -107,6 +107,8 @@ function ($match) use (&$index, $args) { }, $query ); + + return str_replace('%%', '%', $prepared); } public function get_results($query) From c0b5fc7bf063a626dd58cbc6e506b0c3a9c26e7e Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 03:35:53 +0600 Subject: [PATCH 10/11] fix(query): resolve qualifiers to from alias Map logical and physical base-table qualifiers to the active SELECT alias at the rendering boundary, and document the strict quoted identifier behavior. --- docs/breaking-changes.md | 11 +++++++---- src/QueryBuilder.php | 6 +++++- tests/QualifiedColumnPrefixTest.php | 28 ++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index 33bf8a2..d107a3b 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -501,10 +501,13 @@ Not signature breaks, but observable runtime differences. `insert()` row whose first value is an array no longer crashes. - **`take()` / `skip()` cast their argument to `int`** — blocks `LIMIT`/`OFFSET` injection; numeric input is byte-identical. -- **`orderBy()` / `groupBy()` validate the column** as a plain identifier - (`^[A-Za-z0-9_.`]+$`) and throw `RuntimeException` otherwise — blocks - `ORDER BY`/`GROUP BY` injection. Plain/qualified identifiers emit byte-identical - SQL; pass raw expressions through `orderByRaw()`. +- **`orderBy()` / `groupBy()` use the strict structured identifier renderer.** + Accepted bare and qualified columns are resolved against registered + base/from/join tables and aliases, then each segment is backtick-quoted. Unknown + or schema-qualified names, pre-quoted input, comments, and expressions fail + closed with `RuntimeException`; emitted SQL is therefore not byte-identical to + the legacy unquoted output. Pass reviewed ordering expressions through + `orderByRaw()`. - **Cast aliases `integer`/`float`/`double`/`json`/`datetime` now work** (map onto the existing casters) — they were previously silent no-ops returning the raw value. - **Bulk `insert()` aligns ragged rows by column** — a row whose keys differ from diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index d770d0f..01a0d58 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1000,7 +1000,11 @@ public function renderIdentifier(string $column, bool $allowWildcard = false, bo $aliases = $this->getTableAliases(); $map = $this->getTableMap(); - if (\in_array($qualifier, $aliases, true)) { + if ($this->_from !== null + && \in_array($qualifier, [$this->_model->getTableWithoutPrefix(), $this->table], true) + ) { + $resolved = $this->_from; + } elseif (\in_array($qualifier, $aliases, true)) { $resolved = $qualifier; } elseif (isset($map[$qualifier])) { $resolved = $map[$qualifier]; diff --git a/tests/QualifiedColumnPrefixTest.php b/tests/QualifiedColumnPrefixTest.php index 13ca193..505c5fe 100644 --- a/tests/QualifiedColumnPrefixTest.php +++ b/tests/QualifiedColumnPrefixTest.php @@ -4,6 +4,7 @@ use BitApps\WPDatabase\Tests\Fixtures\SoftPost; use BitApps\WPDatabase\Tests\Fixtures\User; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use RuntimeException; @@ -15,6 +16,33 @@ */ final class QualifiedColumnPrefixTest extends TestCase { + public static function baseQualifierProvider(): array + { + return [ + 'logical base table' => ['users'], + 'physical base table' => ['wp_users'], + ]; + } + + #[DataProvider('baseQualifierProvider')] + public function testFromAliasRewritesQualifiedBaseColumnsAcrossStructuredClauses($qualifier): void + { + $sql = (new User())->from('u') + ->select($qualifier . '.id') + ->join('posts AS p', $qualifier . '.id', '=', 'p.user_id') + ->where($qualifier . '.status', 'active') + ->groupBy($qualifier . '.id') + ->orderBy($qualifier . '.id') + ->toSql(); + + $this->assertStringContainsString('SELECT `u`.`id` FROM `wp_users` `u`', $sql); + $this->assertStringContainsString('ON `u`.`id` = `p`.`user_id`', $sql); + $this->assertStringContainsString('WHERE `u`.`status` = ', $sql); + $this->assertStringContainsString('GROUP BY `u`.`id`', $sql); + $this->assertStringContainsString('ORDER BY `u`.`id` ASC', $sql); + $this->assertStringNotContainsString('`wp_users`.`id`', $sql); + } + public function testWhereResolvesUnprefixedModelTable(): void { $sql = (new User())->where('users.status', 1)->toSql(); From 7d096971d978d17f470f9373bbfe8498d5230645 Mon Sep 17 00:00:00 2001 From: Abdul Kaioum Date: Thu, 6 Aug 2026 07:57:35 +0600 Subject: [PATCH 11/11] fix(query): preserve explicit join aliases Resolve registered aliases before remapping hidden base-table qualifiers to the active FROM alias, preventing colliding join aliases from collapsing onto the base. --- src/QueryBuilder.php | 6 ++--- tests/QualifiedColumnPrefixTest.php | 37 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/QueryBuilder.php b/src/QueryBuilder.php index 01a0d58..932e8a4 100644 --- a/src/QueryBuilder.php +++ b/src/QueryBuilder.php @@ -1000,12 +1000,12 @@ public function renderIdentifier(string $column, bool $allowWildcard = false, bo $aliases = $this->getTableAliases(); $map = $this->getTableMap(); - if ($this->_from !== null + if (\in_array($qualifier, $aliases, true)) { + $resolved = $qualifier; + } elseif ($this->_from !== null && \in_array($qualifier, [$this->_model->getTableWithoutPrefix(), $this->table], true) ) { $resolved = $this->_from; - } elseif (\in_array($qualifier, $aliases, true)) { - $resolved = $qualifier; } elseif (isset($map[$qualifier])) { $resolved = $map[$qualifier]; } elseif (\in_array($qualifier, array_values($map), true)) { diff --git a/tests/QualifiedColumnPrefixTest.php b/tests/QualifiedColumnPrefixTest.php index 505c5fe..3e277ca 100644 --- a/tests/QualifiedColumnPrefixTest.php +++ b/tests/QualifiedColumnPrefixTest.php @@ -24,6 +24,24 @@ public static function baseQualifierProvider(): array ]; } + public static function joinAliasCollisionProvider(): array + { + return [ + 'logical base name' => [ + 'users', + 'SELECT `u`.`id`,`users`.`id` FROM `wp_users` `u`', + 'INNER JOIN `wp_posts` AS `users` ON `users`.`user_id` = `u`.`id`', + 'WHERE `users`.`status` = ', + ], + 'physical base name' => [ + 'wp_users', + 'SELECT `u`.`id`,`wp_users`.`id` FROM `wp_users` `u`', + 'INNER JOIN `wp_posts` AS `wp_users` ON `wp_users`.`user_id` = `u`.`id`', + 'WHERE `wp_users`.`status` = ', + ], + ]; + } + #[DataProvider('baseQualifierProvider')] public function testFromAliasRewritesQualifiedBaseColumnsAcrossStructuredClauses($qualifier): void { @@ -43,6 +61,25 @@ public function testFromAliasRewritesQualifiedBaseColumnsAcrossStructuredClauses $this->assertStringNotContainsString('`wp_users`.`id`', $sql); } + #[DataProvider('joinAliasCollisionProvider')] + public function testExplicitJoinAliasWinsWhenItMatchesHiddenBaseName( + $alias, + $expectedSelect, + $expectedJoin, + $expectedWhere + ): void { + $sql = (new User())->from('u') + ->join('posts AS ' . $alias, $alias . '.user_id', '=', 'u.id') + ->select(['u.id', $alias . '.id']) + ->where($alias . '.status', 'published') + ->toSql(); + + $this->assertStringContainsString($expectedSelect, $sql); + $this->assertStringContainsString($expectedJoin, $sql); + $this->assertStringContainsString($expectedWhere, $sql); + $this->assertStringNotContainsString('ON `u`.`user_id` = `u`.`id`', $sql); + } + public function testWhereResolvesUnprefixedModelTable(): void { $sql = (new User())->where('users.status', 1)->toSql();