Skip to content

Fix FR #71885 (Allow escaping question mark placeholders) #4217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 56 additions & 19 deletions ext/pdo/pdo_sql_parser.re
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@
#define PDO_PARSER_TEXT 1
#define PDO_PARSER_BIND 2
#define PDO_PARSER_BIND_POS 3
#define PDO_PARSER_EOI 4
#define PDO_PARSER_ESCAPED_QUESTION 4
#define PDO_PARSER_EOI 5

#define PDO_PARSER_BINDNO_ESCAPED_CHAR -1

#define RET(i) {s->cur = cursor; return i; }
#define SKIP_ONE(i) {s->cur = s->tok + 1; return i; }
Expand All @@ -46,16 +49,18 @@ static int scan(Scanner *s)
/*!re2c
BINDCHR = [:][a-zA-Z0-9_]+;
QUESTION = [?];
ESCQUESTION = [?][?];
COMMENTS = ("/*"([^*]+|[*]+[^/*])*[*]*"*/"|"--"[^\r\n]*);
SPECIALS = [:?"'-/];
MULTICHAR = ([:]{2,}|[?]{2,});
MULTICHAR = [:]{2,};
ANYNOEOF = [\001-\377];
*/

/*!re2c
(["](([\\]ANYNOEOF)|ANYNOEOF\["\\])*["]) { RET(PDO_PARSER_TEXT); }
(['](([\\]ANYNOEOF)|ANYNOEOF\['\\])*[']) { RET(PDO_PARSER_TEXT); }
MULTICHAR { RET(PDO_PARSER_TEXT); }
ESCQUESTION { RET(PDO_PARSER_ESCAPED_QUESTION); }
BINDCHR { RET(PDO_PARSER_BIND); }
QUESTION { RET(PDO_PARSER_BIND_POS); }
SPECIALS { SKIP_ONE(PDO_PARSER_TEXT); }
Expand Down Expand Up @@ -85,7 +90,7 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, char *inquery, size_t inquery_len
char *ptr, *newbuffer;
ptrdiff_t t;
uint32_t bindno = 0;
int ret = 0;
int ret = 0, escapes = 0;
size_t newbuffer_len;
HashTable *params;
struct pdo_bound_param_data *param;
Expand All @@ -98,14 +103,19 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, char *inquery, size_t inquery_len

/* phase 1: look for args */
while((t = scan(&s)) != PDO_PARSER_EOI) {
if (t == PDO_PARSER_BIND || t == PDO_PARSER_BIND_POS) {
if (t == PDO_PARSER_BIND || t == PDO_PARSER_BIND_POS || t == PDO_PARSER_ESCAPED_QUESTION) {
if (t == PDO_PARSER_ESCAPED_QUESTION && stmt->supports_placeholders == PDO_PLACEHOLDER_POSITIONAL) {
/* escaped question marks unsupported, treat as text */
continue;
}

if (t == PDO_PARSER_BIND) {
ptrdiff_t len = s.cur - s.tok;
if ((inquery < (s.cur - len)) && isalnum(*(s.cur - len - 1))) {
continue;
}
query_type |= PDO_PLACEHOLDER_NAMED;
} else {
} else if (t == PDO_PARSER_BIND_POS) {
query_type |= PDO_PLACEHOLDER_POSITIONAL;
}

Expand All @@ -114,7 +124,16 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, char *inquery, size_t inquery_len
plc->next = NULL;
plc->pos = s.tok;
plc->len = s.cur - s.tok;
plc->bindno = bindno++;

if (t == PDO_PARSER_ESCAPED_QUESTION) {
plc->bindno = PDO_PARSER_BINDNO_ESCAPED_CHAR;
plc->quoted = "?";
plc->qlen = 1;
plc->freeq = 0;
escapes++;
} else {
plc->bindno = bindno++;
}

if (placetail) {
placetail->next = plc;
Expand All @@ -125,7 +144,7 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, char *inquery, size_t inquery_len
}
}

if (bindno == 0) {
if (!placeholders) {
/* nothing to do; good! */
return 0;
}
Expand All @@ -140,11 +159,16 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, char *inquery, size_t inquery_len

if (stmt->supports_placeholders == query_type && !stmt->named_rewrite_template) {
/* query matches native syntax */
if (escapes) {
newbuffer_len = inquery_len;
goto rewrite;
}

ret = 0;
goto clean_up;
}

if (stmt->named_rewrite_template) {
if (query_type == PDO_PLACEHOLDER_NAMED && stmt->named_rewrite_template) {
/* magic/hack.
* We we pretend that the query was positional even if
* it was named so that we fall into the
Expand All @@ -155,14 +179,7 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, char *inquery, size_t inquery_len

params = stmt->bound_params;

/* Do we have placeholders but no bound params */
if (bindno && !params && stmt->supports_placeholders == PDO_PLACEHOLDER_NONE) {
pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "no parameters were bound");
ret = -1;
goto clean_up;
}

if (params && bindno != zend_hash_num_elements(params) && stmt->supports_placeholders == PDO_PLACEHOLDER_NONE) {
if (bindno && stmt->supports_placeholders == PDO_PLACEHOLDER_NONE && params && bindno != zend_hash_num_elements(params)) {
/* extra bit of validation for instances when same params are bound more than once */
if (query_type != PDO_PLACEHOLDER_POSITIONAL && bindno > zend_hash_num_elements(params)) {
int ok = 1;
Expand All @@ -188,7 +205,16 @@ safe:
newbuffer_len = inquery_len;

/* let's quote all the values */
for (plc = placeholders; plc; plc = plc->next) {
for (plc = placeholders; plc && params; plc = plc->next) {
if (plc->bindno == PDO_PARSER_BINDNO_ESCAPED_CHAR) {
/* escaped character */
continue;
}

if (query_type == PDO_PLACEHOLDER_NONE) {
continue;
}

if (query_type == PDO_PLACEHOLDER_POSITIONAL) {
param = zend_hash_index_find_ptr(params, plc->bindno);
} else {
Expand Down Expand Up @@ -316,8 +342,13 @@ rewrite:
memcpy(newbuffer, ptr, t);
newbuffer += t;
}
memcpy(newbuffer, plc->quoted, plc->qlen);
newbuffer += plc->qlen;
if (plc->quoted) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will check ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without, it doesn't work properly. When running the new pdo_pgsql tests, there is a code path that sends a non-quoted "?". I'll try to figure out how and when this afternoon.

memcpy(newbuffer, plc->quoted, plc->qlen);
newbuffer += plc->qlen;
} else {
memcpy(newbuffer, plc->pos, plc->len);
newbuffer += plc->len;
}
ptr = plc->pos + plc->len;

plc = plc->next;
Expand Down Expand Up @@ -350,6 +381,11 @@ rewrite:
for (plc = placeholders; plc; plc = plc->next) {
int skip_map = 0;
char *p;

if (plc->bindno == PDO_PARSER_BINDNO_ESCAPED_CHAR) {
continue;
}

name = estrndup(plc->pos, plc->len);

/* check if bound parameter is already available */
Expand Down Expand Up @@ -395,6 +431,7 @@ rewrite:
efree(name);
plc->quoted = "?";
plc->qlen = 1;
newbuffer_len -= plc->len - 1;
}

goto rewrite;
Expand Down
54 changes: 54 additions & 0 deletions ext/pdo/tests/bug_71885.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
--TEST--
PDO Common: FR #71885 (Allow escaping question mark placeholders)
--SKIPIF--
<?php
if (!extension_loaded('pdo')) die('skip');
$dir = getenv('REDIR_TEST_DIR');
if (false == $dir) die('skip no driver');
if (!strncasecmp(getenv('PDOTEST_DSN'), 'pgsql', strlen('pgsql'))) die('skip not relevant for pgsql driver');
if (!strncasecmp(getenv('PDOTEST_DSN'), 'odbc', strlen('odbc'))) die('skip inconsistent error message with odbc');
require_once $dir . 'pdo_test.inc';
PDOTest::skip();
?>
--FILE--
<?php
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.dirname(__FILE__) . '/../../pdo/tests/');
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
$db = PDOTest::factory();
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$db->exec("CREATE TABLE test (a int)");

$sql = "SELECT * FROM test WHERE a ?? 1";

try {
$db->exec($sql);
} catch (PDOException $e) {
var_dump(strpos($e->getMessage(), "?") !== false);
}

try {
$stmt = $db->prepare($sql);
$stmt->execute();
} catch (PDOException $e) {
var_dump(strpos($e->getMessage(), "?") !== false);
}

if ($db->getAttribute(PDO::ATTR_DRIVER_NAME) == 'mysql') {
$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, 1);
}

try {
$stmt = $db->prepare($sql);
$stmt->execute();
} catch (PDOException $e) {
var_dump(strpos($e->getMessage(), "?") !== false);
}

?>
===DONE===
--EXPECT--
bool(true)
bool(true)
bool(true)
===DONE===
46 changes: 23 additions & 23 deletions ext/pdo_pgsql/pgsql_driver.c
Original file line number Diff line number Diff line change
Expand Up @@ -256,36 +256,36 @@ static int pgsql_handle_preparer(pdo_dbh_t *dbh, const char *sql, size_t sql_len
execute_only = H->disable_prepares;
}

if (!emulate && PQprotocolVersion(H->server) > 2) {
if (!emulate && PQprotocolVersion(H->server) <= 2) {
emulate = 1;
}

if (emulate) {
stmt->supports_placeholders = PDO_PLACEHOLDER_NONE;
} else {
stmt->supports_placeholders = PDO_PLACEHOLDER_NAMED;
stmt->named_rewrite_template = "$%d";
ret = pdo_parse_params(stmt, (char*)sql, sql_len, &nsql, &nsql_len);

if (ret == 1) {
/* query was re-written */
sql = nsql;
} else if (ret == -1) {
/* couldn't grok it */
strcpy(dbh->error_code, stmt->error_code);
return 0;
}
}

if (!execute_only) {
/* prepared query: set the query name and defer the
actual prepare until the first execute call */
spprintf(&S->stmt_name, 0, "pdo_stmt_%08x", ++H->stmt_counter);
}
ret = pdo_parse_params(stmt, (char*)sql, sql_len, &nsql, &nsql_len);

if (nsql) {
S->query = nsql;
} else {
S->query = estrdup(sql);
}
if (ret == -1) {
/* couldn't grok it */
strcpy(dbh->error_code, stmt->error_code);
return 0;
} else if (ret == 1) {
/* query was re-written */
S->query = nsql;
} else {
S->query = estrdup(sql);
}

return 1;
if (!emulate && !execute_only) {
/* prepared query: set the query name and defer the
actual prepare until the first execute call */
spprintf(&S->stmt_name, 0, "pdo_stmt_%08x", ++H->stmt_counter);
}

stmt->supports_placeholders = PDO_PLACEHOLDER_NONE;
return 1;
}

Expand Down
46 changes: 46 additions & 0 deletions ext/pdo_pgsql/tests/bug71885.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
--TEST--
Request #71855 (PDO placeholder escaping)
--SKIPIF--
<?php
if (!extension_loaded('pdo') || !extension_loaded('pdo_pgsql')) die('skip not loaded');
require_once dirname(__FILE__) . '/../../../ext/pdo/tests/pdo_test.inc';
require_once dirname(__FILE__) . '/config.inc';
PDOTest::skip();
?>
--FILE--
<?php
require_once dirname(__FILE__) . '/../../../ext/pdo/tests/pdo_test.inc';
require_once dirname(__FILE__) . '/config.inc';
$db = PDOTest::test_factory(dirname(__FILE__) . '/common.phpt');
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$db->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_NUM);

foreach ([false, true] as $emulate) {
$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, $emulate);

try {
$stmt = $db->prepare('select ?- lseg \'((-1,0),(1,0))\'');
$stmt->execute();
} catch (PDOException $e) {
var_dump('ERR');
}

$stmt = $db->prepare('select ??- lseg \'((-1,0),(1,0))\'');
$stmt->execute();

var_dump($stmt->fetch());
}

?>
==OK==
--EXPECT--
string(3) "ERR"
array(1) {
[0]=>
bool(true)
}
array(1) {
[0]=>
bool(true)
}
==OK==
57 changes: 57 additions & 0 deletions ext/pdo_pgsql/tests/bug71885_2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
--TEST--
Request #71855 (PDO placeholder escaping, part 2)
--SKIPIF--
<?php
if (!extension_loaded('pdo') || !extension_loaded('pdo_pgsql')) die('skip not loaded');
require_once dirname(__FILE__) . '/../../../ext/pdo/tests/pdo_test.inc';
require_once dirname(__FILE__) . '/config.inc';
PDOTest::skip();

$db = PDOTest::factory();
if (version_compare($db->getAttribute(PDO::ATTR_SERVER_VERSION), '9.4.0') < 0) {
die("skip Requires 9.4+");
}

?>
--FILE--
<?php
require_once dirname(__FILE__) . '/../../../ext/pdo/tests/pdo_test.inc';
require_once dirname(__FILE__) . '/config.inc';
$db = PDOTest::test_factory(dirname(__FILE__) . '/common.phpt');
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$db->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_NUM);

$jsonb = $db->quote(json_encode(['a' => 1]));

foreach ([false, true] as $emulate) {
$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, $emulate);

$stmt = $db->prepare("SELECT {$jsonb}::jsonb ?? ?");
$stmt->execute(['b']);
var_dump($stmt->fetch());

$stmt = $db->prepare("SELECT {$jsonb}::jsonb ???");
$stmt->execute(['a']);
var_dump($stmt->fetch());
}

?>
==OK==
--EXPECT--
array(1) {
[0]=>
bool(false)
}
array(1) {
[0]=>
bool(true)
}
array(1) {
[0]=>
bool(false)
}
array(1) {
[0]=>
bool(true)
}
==OK==