database icon indicating copy to clipboard operation
database copied to clipboard

Buggy placeholder expansion with table aliases containing `--`

Open smuuf opened this issue 9 months ago • 0 comments

nette/database version: 3.2.6 (current latest)

Bug Description

Expansion of SQL parameters in SqlPreprocessor is buggy when a SQL query with parameters and ? placeholders also contains table aliases quoted in backticks which contain double minus sign --, like for example:

... AS `alias--a` ...

The example down below fails with:

Nette\InvalidArgumentException: There are more parameters than placeholders.

in src/Database/SqlPreprocessor.php(108)
in src/Database/Connection.php(276) Nette\Database\SqlPreprocessor->process()
in src/Database/Connection.php(248) Nette\Database\Connection->preprocess()
in Cases/regression/issue_13150.phpt(32) Nette\Database\Connection->query()
in src/Framework/Assert.php(390) Tests\Cases\{closure}()
in src/Framework/Assert.php(414) self::error($function, []);
in Cases/regression/issue_13150.phpt(36) Tester\Assert::noError()

Steps To Reproduce

<?php

// Create some "whatever" connection, so we get to SqlPreprocessor.
// The issue was discovered with MariaDB, but is also reproducible with SQLite.
$tempDir = sys_get_temp_dir();
$connection = new \Nette\Database\Connection("sqlite:/{$tempDir}/temp.sql");

$params = [0, 6];
$result = $connection->query('
	CREATE TEMPORARY TABLE _table(num INTEGER);
	SELECT `alias--a`.num AS _c
	FROM _table(1, 5) AS `alias--a` HAVING _c > ? AND _c < ?
', ...$params);

[!NOTE] This is not some "random nitpicky discovery", but a MVE extracted from a much larger real-world SQL query.

Funny thing

If the HAVING ... clause is placed on a separate line, like so:

CREATE TEMPORARY TABLE _table(num INTEGER);
SELECT `alias--a`.num AS _c
FROM _table(1, 5) AS `alias--a`
HAVING _c > ? AND _c < ?

... then the behavior seems to be correct.

Expected Behavior

I expect SqlPreprocessor to correctly handle the final SQL as being:

CREATE TEMPORARY TABLE _table(num INTEGER);
SELECT `alias--a`.num AS _c
FROM _table(1, 5) AS `alias--a` HAVING _c > 0 AND _c < 6

Possible Cause

I suspect the buggy behavior comes from commit https://github.com/nette/database/commit/bd43117bd61bb7a7d4ef93e0862c069c81f8b2cd. The regex used in SqlPreprocessor::process() was updated with case for |--[^\n]* (for some for-me-undecipherable reason), which then causes the --a.num AS c part to appear here:

Image

... which, I assume, is wrong.

Possible Solution

The most naive solution I could come up with:


// In SqlPreprocessor::process()
...

$res[] = Nette\Utils\Strings::replace(
	$param,
	<<<'X'
		~
			'[^']*+'
			|"[^"]*+"
+			|`[^`]*+`
			|\?[a-z]*
			|^\s*+(?:\(?\s*SELECT|INSERT|UPDATE|DELETE|REPLACE|EXPLAIN)\b
			|\b(?:SET|WHERE|HAVING|ORDER\ BY|GROUP\ BY|KEY\ UPDATE)(?=\s*$|\s*\?)
			|\bIN\s+(?:\?|\(\?\))
			|/\*.*?\*/
			|--[^\n]*
		~Dsix
		X,
	$this->parsePart(...),
);

... but without deeper understanding of the logic I'm not sure if this is the correct solution - and whether it comes without negative impact in some other cases.

smuuf avatar Apr 23 '25 07:04 smuuf