nette/database

Error while binding short string with slash suffix

Closed this issue · 14 comments

  • bug report? yes
  • feature request? nono
  • version: 2.4.3

Description

Get error syntax exception, when mixing short (with suffix ) and long string parameters binding to INSERT or UPDATE query (maybe there are probably more scenarios to get this error).

Steps To Reproduce

I have tested this with PostqreSQL. Let's create this simple table:

CREATE TABLE test
(
  col1 text,
  col2 text,
  col3 text
);

Then run this command:

/** @var \Nette\Database\Context $context */
$context->table('test')->insert([
	'col1' => 'some text \\', // shorter than 20
	'col2' => 'some text longer than 20 characters',
	'col3' => 'some text', // shorter than 20
]);

This exception is thrown:

Nette\Database\DriverException #42601

SQLSTATE[42601]: Syntax error: 7 ERROR:  syntax error at or near ","
LINE 1: ... ("col1", "col2", "col3") VALUES ('some text \', ?, 'some te...

This behavior is probably fixed in 3.0.0-alpha with commit 644b587. To fix this in 2.4 branch it is possible to change SqlPreprocessor->formatValue to bind all strings as variable, not only strings longer than 20 characters. Bud I don't why this logic is there, maybe this update could be BC?

dg commented

According to the PostqreSQL documentation, everything is ok: https://www.postgresql.org/docs/8.2/static/sql-syntax-lexical.html#SQL-SYNTAX-STRINGS

A string constant in SQL is an arbitrary sequence of characters bounded by single quotes ('), for example 'This is a string'. To include a single-quote character within a string constant, write two adjacent single quotes, e.g. 'Dianne''s horse'. Note that this is not the same as a double-quote character (").

There are a mention switches standard_conforming_strings and backslash_quote. Can you find out what value they has? If it is true, it looks like a serious security issue in PDO.

milo commented

This is some bug in PDO parameters binding. The $pdo->quote() respects standard_conforming_strings setting.

# SET standard_conforming_strings TO on
$pdo->quote('BACK \\ SLASH')  =  BACK \ SLASH

# SET standard_conforming_strings TO off
$pdo->quote('BACK \\ SLASH')  =  BACK \\ SLASH

Query INSERT INTO test ("col1", "col2", "col3") VALUES ('some text \', 'abc', 'def') is valid PostgreSQL query with standard_conforming_strings set to on (which means, "Handle backslash as ordinary character").

IMHO the bug is in internal PDO values binding code when they are looking for ? in prepared query. The ? is not found, parameter is not binded correctly and PDO executes SQL which contains ?. The whole exception message is:

PHP Fatal error:  Uncaught PDOException: SQLSTATE[42601]: Syntax error: 7 ERROR:  syntax error at or near ","
LINE 1: INSERT INTO test (col1, col2, col3) VALUES ('\', ?, 'x')
                                                          ^ in D:\Web\temp\pdo\test2.php:14

Focus on the ^ and where it points.

Code to reproduce:

$pdo = new PDO('pgsql:host=localhost;dbname=test', 'test', 'test');
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$pdo->query('SET standard_conforming_strings TO on');

$stmt = $pdo->prepare("INSERT INTO test (col1, col2, col3) VALUES ('\\', ?, '')");
$stmt->bindValue(1, 'x', PDO::PARAM_STR);
$stmt->execute();

If you don't, I'll report PHP bug in the evening.

milo commented

Workaround is SET standard_conforming_strings TO off if an application does not depends on it.
In that case, PDO internals code works.

dg commented

That would be great if you could report it, I do not know the PostgreSQL at all. (I only develop its drivers :-) )

dg commented

Can ? be replaced directly by the database instead of the PDO?

dg commented

We have solved it by workaround because sending all strings as parameters causes incompatibility in PostgreSQL:

SELECT 'string' vs SELECT ? + parameter 'string' leads to SQLSTATE[42P18]: Indeterminate datatype: 7 ERROR: could not determine data type of parameter $1.

On the other hand, this behavior relies on the 20-character limit, so maybe a BC break could be done in 2.4.

milo commented

Can ? be replaced directly by the database instead of the PDO?

There is a PREPARE statement, but I don't know syntax using ?.

sending all strings as parameters causes incompatibility

It is probably related to PHP #36652, workaround is mentioned in comment.

dg commented

IMHO just PDO::ATTR_EMULATE_PREPARES causes this issue…

milo commented

I don't think so. With ATTR_EMULATE_PREPARES = false it uses PostgreSQL PREPARE statement. It translates ? into $1.

$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, true);
# PG log
# LOG:  statement: INSERT INTO test (col1, col2, col3) VALUES ('a', 'x', ' ')

$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
# PG log
# LOG:  execute pdo_stmt_00000001: INSERT INTO test (col1, col2, col3) VALUES ('a', $1, ' ')
# DETAIL:  parameters: $1 = 'x'

But with buggy query from this issue, it fails in the same way in both cases:

ERROR:  syntax error at or near "," at character 51
STATEMENT:  INSERT INTO test (col1, col2, col3) VALUES ('\', ?, ' ')
milo commented

It seems to me that issue appears only when some string ends with \.

$stmt = $pdo->prepare("INSERT INTO test (col1, col2, col3) VALUES ('\\', ?, ' ')");  # BUGGY
$stmt = $pdo->prepare("INSERT INTO test (col1, col2, col3) VALUES ('\\ ', ?, ' ')"); # OK
$stmt = $pdo->prepare("INSERT INTO test (col1, col2) VALUES ('\\', ?)"); # OK
dg commented

What about $stmt = $pdo->prepare("INSERT INTO test (col1, col2, col3) VALUES ('\\'' ', ?, ' ')");, ie doesn't end with, but contains \ before '

milo commented

It's buggy too.

milo commented

Reported PHP #74996.

Hi, big thanks for really quick fix!