From 730e0a34fe6eb05fd7973273bf0a68ac96cf9532 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Tue, 13 Oct 2026 17:06:01 +1100 Subject: [PATCH] Quote predicate names as identifiers where fragment quoting renders broken SQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Predicate identifiers render through quoteIdentifierInFragment(), which quotes every piece split out on a non-word character other than a space, dot, * or AS. A name such as x-y, a"b, nâme or my col therefore renders as adjacent quoted pieces or a bare sequence of identifiers. Such names are now quoted whole as an identifier chain, split only on the qualifier dot. Every name whose fragment quoting renders valid SQL keeps its current rendering, judged against the platform's own fragment pattern. --- src/Sql/AbstractSql.php | 71 +++- test/unit/Sql/PredicateIdentifierNameTest.php | 302 ++++++++++++++++++ .../TestAsset/TrustingHyphenWordPlatform.php | 27 ++ 3 files changed, 398 insertions(+), 2 deletions(-) create mode 100644 test/unit/Sql/PredicateIdentifierNameTest.php create mode 100644 test/unit/TestAsset/TrustingHyphenWordPlatform.php diff --git a/src/Sql/AbstractSql.php b/src/Sql/AbstractSql.php index 5ea9aaad..05865d4d 100644 --- a/src/Sql/AbstractSql.php +++ b/src/Sql/AbstractSql.php @@ -18,16 +18,23 @@ use PhpDb\Sql\Platform\PlatformDecoratorInterface; use function array_key_first; +use function array_map; use function count; +use function explode; use function get_object_vars; use function implode; use function is_array; use function is_string; +use function preg_split; use function rtrim; use function str_replace; +use function strtolower; use function strtoupper; use function vsprintf; +use const PREG_SPLIT_DELIM_CAPTURE; +use const PREG_SPLIT_NO_EMPTY; + /** * @api * @@ -247,7 +254,7 @@ protected function processExpression( $parameterContainer, ) : $platform->quoteValue((string) $argument->getValue()), - $argument instanceof Identifier => $platform->quoteIdentifierInFragment($argument->getValue()), + $argument instanceof Identifier => $this->quoteIdentifierArgument($argument->getValue(), $platform), $argument instanceof Literal => $argument->getValue(), $argument instanceof Identifiers => $this->processIdentifiersArgument($argument, $platform), $argument instanceof SelectArgument => $this->processExpressionOrSelect( @@ -330,7 +337,7 @@ protected function processIdentifiersArgument( $processedIdentifiers = []; foreach ($identifiers as $identifier) { - $processedIdentifiers[] = $platform->quoteIdentifierInFragment($identifier); + $processedIdentifiers[] = $this->quoteIdentifierArgument($identifier, $platform); } return implode(', ', $processedIdentifiers); @@ -555,6 +562,66 @@ private function matchSpecification(array $specifications, int $parameterCount): throw Exception\RuntimeException::forUnsupportedParameterCount(); } + /** + * Quote an identifier argument, keeping fragment quoting wherever it renders valid SQL + * + * Fragment quoting leaves spaces, dots, `*` and `AS` bare and quotes every other piece the + * platform splits out, so a name holding any other delimiter renders as adjacent quoted + * pieces, and names separated only by spaces render as a bare sequence of identifiers. Both + * are invalid, so such a name is quoted whole as an identifier chain instead, split only on + * the qualifier dot with a `*` part left bare. + */ + private function quoteIdentifierArgument(string $identifier, PlatformInterface $platform): string + { + $isValidFragment = true; + $previousIsName = false; + $afterSpace = false; + + foreach ((array) preg_split( + '/([ .*])/', + $identifier, + -1, + PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY, + ) as $part) { + if (' ' === $part) { + $afterSpace = true; + continue; + } + + $isName = '.' !== $part && '*' !== $part && 'as' !== strtolower((string) $part); + if ( + $isName + && ( + ( + $previousIsName + && $afterSpace + ) + || $platform->quoteIdentifierInFragment((string) $part) !== $platform->quoteIdentifier( + (string) $part, + ) + ) + ) { + $isValidFragment = false; + break; + } + + $previousIsName = $isName; + $afterSpace = false; + } + + if ($isValidFragment) { + return $platform->quoteIdentifierInFragment($identifier); + } + + return implode( + $platform->getIdentifierSeparator(), + array_map( + static fn(string $part): string => '*' === $part ? $part : $platform->quoteIdentifier($part), + explode('.', $identifier), + ), + ); + } + private function quoteJoinTableIdentifier(TableIdentifier $identifier, PlatformInterface $platform): string { [$table, $schema] = $identifier->getTableAndSchema(); diff --git a/test/unit/Sql/PredicateIdentifierNameTest.php b/test/unit/Sql/PredicateIdentifierNameTest.php new file mode 100644 index 00000000..07a90870 --- /dev/null +++ b/test/unit/Sql/PredicateIdentifierNameTest.php @@ -0,0 +1,302 @@ + + */ + public static function brokenNameProvider(): array + { + return [ + 'hyphen' => ['x-y', '"x-y"'], + 'space' => ['my col', '"my col"'], + 'quote character' => ['a"b', '"a\'b"'], + 'non-ascii' => ['nâme', '"nâme"'], + 'tab' => ["a\tb", "\"a\tb\""], + 'dotted hyphen' => ['t-1.x-y', '"t-1"."x-y"'], + 'dotted space' => ['t.my col', '"t"."my col"'], + 'hyphen qualified star' => ['t-1.*', '"t-1".*'], + ]; + } + + /** + * @return iterable + */ + public static function brokenPredicateProvider(): iterable + { + foreach (self::predicateMethodProvider() as $method => [$apply, $template]) { + foreach (self::brokenNameProvider() as $case => [$name, $quoted]) { + yield "{$method} {$case}" => [$apply, $template, $name, $quoted]; + } + } + } + + /** + * @return array + */ + public static function hyphenWordPlatformProvider(): array + { + return [ + 'hyphen' => ['x-y', '`x-y`'], + 'hyphen arithmetic' => ['a-b * c', '`a-b` * `c`'], + 'dotted hyphen' => ['t-1.x-y', '`t-1`.`x-y`'], + 'space' => ['my col', '`my col`'], + 'quote character' => ['a`b', '`a``b`'], + 'plus' => ['a+b', '`a+b`'], + ]; + } + + /** + * @return array + */ + public static function predicateMethodProvider(): array + { + return [ + 'equalTo' => [static fn(Predicate $p, string $n): Predicate => $p->equalTo($n, 1), "%s = '1'"], + 'notEqualTo' => [static fn(Predicate $p, string $n): Predicate => $p->notEqualTo($n, 1), "%s != '1'"], + 'lessThan' => [static fn(Predicate $p, string $n): Predicate => $p->lessThan($n, 1), "%s < '1'"], + 'greaterThan' => [static fn(Predicate $p, string $n): Predicate => $p->greaterThan($n, 1), "%s > '1'"], + 'like' => [static fn(Predicate $p, string $n): Predicate => $p->like($n, 'a%'), "%s LIKE 'a%%'"], + 'in' => [static fn(Predicate $p, string $n): Predicate => $p->in($n, [1, 2]), "%s IN ('1', '2')"], + 'between' => [ + static fn(Predicate $p, string $n): Predicate => $p->between($n, 1, 2), + "%s BETWEEN '1' AND '2'", + ], + 'isNull' => [static fn(Predicate $p, string $n): Predicate => $p->isNull($n), '%s IS NULL'], + ]; + } + + /** + * Names whose fragment quoting renders valid SQL, and that rendering + * + * @return array + */ + public static function unchangedNameProvider(): array + { + return [ + 'plain' => ['col', '"col"'], + 'dotted' => ['t.col', '"t"."col"'], + 'qualified star' => ['t.*', '"t".*'], + 'arithmetic' => ['price*qty', '"price"*"qty"'], + 'spaced arithmetic' => ['a * b', '"a" * "b"'], + 'dotted arithmetic' => ['t.a * t.b', '"t"."a" * "t"."b"'], + 'inline alias' => ['x as y', '"x" as "y"'], + 'inline alias upper' => ['x AS y', '"x" AS "y"'], + 'comma' => ['a,b', '"a,b"'], + 'empty' => ['', ''], + ]; + } + + /** + * @return iterable + */ + public static function unchangedPredicateProvider(): iterable + { + foreach (self::predicateMethodProvider() as $method => [$apply, $template]) { + foreach (self::unchangedNameProvider() as $case => [$name, $quoted]) { + yield "{$method} {$case}" => [$apply, $template, $name, $quoted]; + } + } + } + + #[Test] + #[DataProvider('hyphenWordPlatformProvider')] + public function followsThePlatformFragmentPatternForWordCharacters(string $name, string $quoted): void + { + $select = new Select('foo'); + $select->where->equalTo($name, 1); + + self::assertSame( + "SELECT `foo`.* FROM `foo` WHERE {$quoted} = '1'", + $select->getSqlString(new TrustingHyphenWordPlatform()), + ); + } + + /** + * @param Closure(Predicate, string): Predicate $apply + */ + #[Test] + #[DataProvider('unchangedPredicateProvider')] + public function keepsValidWhereNameRenderingUnchanged( + Closure $apply, + string $template, + string $name, + string $quoted, + ): void { + $select = new Select('foo'); + $apply($select->where, $name); + + self::assertSame( + sprintf("SELECT \"foo\".* FROM \"foo\" WHERE {$template}", $quoted), + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function leavesExpressionWhereUnchanged(): void + { + $select = new Select('foo'); + $select->where(new Expression('x-y = ?', [1])); + + self::assertSame( + "SELECT \"foo\".* FROM \"foo\" WHERE x-y = '1'", + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function leavesLiteralWhereUnchanged(): void + { + $select = new Select('foo'); + $select->where(new Literal('x-y = 1')); + + self::assertSame( + 'SELECT "foo".* FROM "foo" WHERE x-y = 1', + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function leavesStringWhereUnchanged(): void + { + $select = new Select('foo'); + $select->where('x-y = 1'); + + self::assertSame( + 'SELECT "foo".* FROM "foo" WHERE x-y = 1', + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesDeleteWhereName(string $name, string $quoted): void + { + $delete = new Delete('foo'); + $delete->where->equalTo($name, 1); + + self::assertSame( + "DELETE FROM \"foo\" WHERE {$quoted} = '1'", + $delete->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + public function quotesEachNameOfAnIdentifiersArgument(): void + { + $select = new Select('foo'); + $select->where(new Expression('(?) IN (SELECT 1)', [Argument::identifiers(['x-y', 't.col', 'a * b'])])); + + self::assertSame( + 'SELECT "foo".* FROM "foo" WHERE ("x-y", "t"."col", "a" * "b") IN (SELECT 1)', + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesHavingName(string $name, string $quoted): void + { + $select = new Select('foo'); + $select->having->greaterThan($name, 1); + + self::assertSame( + "SELECT \"foo\".* FROM \"foo\" HAVING {$quoted} > '1'", + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + /** + * @param Closure(Predicate, string): Predicate $apply + */ + #[Test] + #[DataProvider('brokenPredicateProvider')] + public function quotesPreviouslyBrokenWhereNameAsOneIdentifier( + Closure $apply, + string $template, + string $name, + string $quoted, + ): void { + $select = new Select('foo'); + $apply($select->where, $name); + + self::assertSame( + sprintf("SELECT \"foo\".* FROM \"foo\" WHERE {$template}", $quoted), + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesUpdateWhereName(string $name, string $quoted): void + { + $update = new Update('foo'); + $update->set(['bar' => 2]); + $update->where->equalTo($name, 1); + + self::assertSame( + "UPDATE \"foo\" SET \"bar\" = '2' WHERE {$quoted} = '1'", + $update->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesWhereArrayKeyAsPredicateName(string $name, string $quoted): void + { + $select = new Select('foo'); + $select->where([$name => 1]); + + self::assertSame( + "SELECT \"foo\".* FROM \"foo\" WHERE {$quoted} = '1'", + $select->getSqlString(new TrustingSql92Platform()), + ); + } + + #[Test] + #[DataProvider('brokenNameProvider')] + #[DataProvider('unchangedNameProvider')] + public function quotesWhereArrayKeyWithNullAsIsNullName(string $name, string $quoted): void + { + $select = new Select('foo'); + $select->where([$name => null]); + + self::assertSame( + "SELECT \"foo\".* FROM \"foo\" WHERE {$quoted} IS NULL", + $select->getSqlString(new TrustingSql92Platform()), + ); + } +} diff --git a/test/unit/TestAsset/TrustingHyphenWordPlatform.php b/test/unit/TestAsset/TrustingHyphenWordPlatform.php new file mode 100644 index 00000000..1e65219e --- /dev/null +++ b/test/unit/TestAsset/TrustingHyphenWordPlatform.php @@ -0,0 +1,27 @@ +quoteTrustedValue($value); + } +}