From 186035e6c2b0f4b2d5c3fa4f98e9a25f2d9b27a3 Mon Sep 17 00:00:00 2001 From: TPHRyan Date: Wed, 27 Mar 2019 13:11:35 +1000 Subject: [PATCH 1/5] Fix quotes in comments breaking parameters An odd number of quotes in comments would cause getUnquotedStatementFragments to start returning the inside of strings, rather than the outside. Ignore comment lines to solve this problem. --- lib/Doctrine/DBAL/SQLParserUtils.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/DBAL/SQLParserUtils.php b/lib/Doctrine/DBAL/SQLParserUtils.php index 0ff0456a00e..c25a4af2ddd 100644 --- a/lib/Doctrine/DBAL/SQLParserUtils.php +++ b/lib/Doctrine/DBAL/SQLParserUtils.php @@ -4,6 +4,7 @@ use const PREG_OFFSET_CAPTURE; use function array_fill; +use function array_filter; use function array_key_exists; use function array_merge; use function array_slice; @@ -250,11 +251,12 @@ private static function getUnquotedStatementFragments($statement) self::ESCAPED_DOUBLE_QUOTED_TEXT . '|' . self::ESCAPED_BACKTICK_QUOTED_TEXT . '|' . self::ESCAPED_BRACKET_QUOTED_TEXT; - $expression = sprintf('/((.+(?i:ARRAY)\\[.+\\])|([^\'"`\\[]+))(?:%s)?/s', $literal); - + $expression = sprintf('/(?:[\s]*--.*?\n)|((.+(?i:ARRAY)\\[.+\\])|([^-\'"`\\[]+))(?:%s)?/s', $literal); preg_match_all($expression, $statement, $fragments, PREG_OFFSET_CAPTURE); - return $fragments[1]; + return array_filter($fragments[1], static function ($match) { + return $match !== ''; + }); } /** From eac80aba0fb94450ba9b6d363cc99449f176d2e9 Mon Sep 17 00:00:00 2001 From: TPHRyan Date: Wed, 27 Mar 2019 14:47:13 +1000 Subject: [PATCH 2/5] Implement regression test for quotes in comments Parameters would previously break when you have an odd number of single quotes in a comment and then sandwich parameters between strings. This test ensures this functionality is fixed. --- .../Tests/DBAL/SQLParserUtilsTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php b/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php index 414f909f870..8ed55caf84f 100644 --- a/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php +++ b/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php @@ -86,6 +86,25 @@ public function dataGetPlaceholderPositions() ['SELECT data.age AS age, data.id AS id, data.name AS name, data.id AS id FROM test_data data WHERE (data.description LIKE :condition_0 ESCAPE "\\\\") AND (data.description LIKE :condition_1 ESCAPE \'\\\\\') ORDER BY id ASC', false, [121 => 'condition_0', 174 => 'condition_1']], ['SELECT data.age AS age, data.id AS id, data.name AS name, data.id AS id FROM test_data data WHERE (data.description LIKE :condition_0 ESCAPE `\\\\`) AND (data.description LIKE :condition_1 ESCAPE `\\\\`) ORDER BY id ASC', false, [121 => 'condition_0', 174 => 'condition_1']], ['SELECT data.age AS age, data.id AS id, data.name AS name, data.id AS id FROM test_data data WHERE (data.description LIKE :condition_0 ESCAPE \'\\\\\') AND (data.description LIKE :condition_1 ESCAPE `\\\\`) ORDER BY id ASC', false, [121 => 'condition_0', 174 => 'condition_1']], + // Named parameter containing comment(s) with odd number of single quotes + [ + <<<'SQLDATA' +SELECT * FROM foo WHERE +bar=:a_param1 +-- Oops! Perfectly normal comment but it's got an odd number of apostrophes +OR bar=':not_a_param1' +OR bar=:a_param2 +OR bar=:a_param3 +OR bar=':not_a_param2' +SQLDATA + , + false, + [ + 29 => 'a_param1', + 145 => 'a_param2', + 162 => 'a_param3', + ], + ], ]; } From c6a40c49a2e5169a53fafddc812c8ecc1a185e0f Mon Sep 17 00:00:00 2001 From: TPHRyan Date: Thu, 28 Mar 2019 10:26:44 +1000 Subject: [PATCH 3/5] Add additional test for SQL comments in strings It is possible a comment '--' in a string might break the parser (reverse the parity of following quotes) so this case tests for that. --- .../Doctrine/Tests/DBAL/SQLParserUtilsTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php b/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php index 8ed55caf84f..e52db6d35ea 100644 --- a/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php +++ b/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php @@ -105,6 +105,23 @@ public function dataGetPlaceholderPositions() 162 => 'a_param3', ], ], + // Check comments in strings don't reverse it either + [ + <<<'SQLDATA' +INSERT INTO foo VALUES +(' +-- Not a comment +Hello, world! +', +:a_param1 +':not_a_param1') +SQLDATA + , + false, + [ + 61 => 'a_param1', + ], + ], ]; } From 37c6b8a37af46034b7af7b64b355f983873585bf Mon Sep 17 00:00:00 2001 From: TPHRyan Date: Fri, 29 Mar 2019 13:43:13 +1000 Subject: [PATCH 4/5] Add test for quote and string comment on same line --- tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php b/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php index e52db6d35ea..6c75f00ea4d 100644 --- a/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php +++ b/tests/Doctrine/Tests/DBAL/SQLParserUtilsTest.php @@ -118,9 +118,19 @@ public function dataGetPlaceholderPositions() SQLDATA , false, - [ - 61 => 'a_param1', - ], + [ 61 => 'a_param1' ], + ], + // And with comment on same line as quote + [ + <<<'SQLDATA' +INSERT INTO foo VALUES +(' +-- Not a comment Hello, world! ', :a_param1 +':not_a_param1') +SQLDATA + , + false, + [ 60 => 'a_param1' ], ], ]; From aeab41dbc37c5cf28627672054a686d9980a6572 Mon Sep 17 00:00:00 2001 From: TPHRyan Date: Fri, 29 Mar 2019 17:44:36 +1000 Subject: [PATCH 5/5] Use argument and return types for getUnquotedStatementFragments filter --- lib/Doctrine/DBAL/SQLParserUtils.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/DBAL/SQLParserUtils.php b/lib/Doctrine/DBAL/SQLParserUtils.php index c25a4af2ddd..1eed8549662 100644 --- a/lib/Doctrine/DBAL/SQLParserUtils.php +++ b/lib/Doctrine/DBAL/SQLParserUtils.php @@ -254,7 +254,7 @@ private static function getUnquotedStatementFragments($statement) $expression = sprintf('/(?:[\s]*--.*?\n)|((.+(?i:ARRAY)\\[.+\\])|([^-\'"`\\[]+))(?:%s)?/s', $literal); preg_match_all($expression, $statement, $fragments, PREG_OFFSET_CAPTURE); - return array_filter($fragments[1], static function ($match) { + return array_filter($fragments[1], static function (string $match) : bool { return $match !== ''; }); }