Skip to content

Buggy placeholder expansion with table aliases containing -- #318

Open
@smuuf

Description

@smuuf

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 bd43117.
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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions