warehack.ing/informix-db
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

The placeholder rewriter could not see a string literal

Browse Source 
We advertise paramstyle="numeric" and the wire protocol takes ?, so :N
placeholders are rewritten on the way out. That was a bare
re.sub(r":(\d+)", "?", sql), which rewrote the inside of string literals:

    UPDATE jobs SET url = 'http://host:8080/x' WHERE id = ?
                      stored as 'http://host?/x'

Any HH:MM time, any URL with a port, any aspect ratio, any key:value
string. It wrote wrong data and said nothing about it.

None of it was opt-in. The rewrite fires whenever a statement has
parameters, whatever placeholder style the caller actually used, so
writing ? everywhere and never touching numeric style did not protect
you. It also changed the placeholder count while num_qmarks was still
computed from len(params), leaving driver and server disagreeing about
how many binds exist.

Replaced with a single pass that substitutes only outside quotes and
comments. The lexical rules are Informix's own, measured against all
three servers rather than assumed from standard SQL, and one of them
would have been got wrong by habit: a backslash escapes nothing, so
'a\'b' is an unterminated string and draws -282. A scanner written to
Postgres reflexes would desync there and corrupt everything after it.
Block comments do not nest, the first */ closes them. Braces are a
comment. :: is stepped over as a unit so a cast can never be read as the
start of a placeholder.

An unterminated quote or comment consumes the rest and substitutes
nothing further. Under-substituting leaves the server to reject SQL that
was already malformed; guessing would corrupt a literal.

This was the last place the driver inferred meaning from SQL text
instead of handling it properly. Eleven of the new tests fail against
the previous commit.
This commit is contained in:
Ryan Malloy 2026-09-03 12:45:58 -06:00
parent 7ffc148112
commit 088171325d
2 changed files with 341 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

101
src/informix_db/cursors.py
View File

@ -60,7 +60,6 @@ if TYPE_CHECKING:
_cursor_counter = itertools.count(1)
_NUMERIC_PLACEHOLDER_RE = __import__("re").compile(r":(\d+)")
# Phase 28: pre-built CLOSE and RELEASE PDU bytes for cursor finalizers.
@ -247,17 +246,103 @@ def _finalize_cursor(
conn._wire_lock.release()
_ASCII_DIGITS = frozenset("0123456789")
def _rewrite_numeric_to_qmark(sql: str) -> str:
"""Convert ``:1`` / ``:2`` placeholders (paramstyle="numeric") to ``?``.
Informix's wire protocol uses ``?`` natively. Since we expose
``paramstyle="numeric"`` in the public API (matches Informix
ESQL/C convention), we rewrite before sending. Trivial cases only
— strings and comments are NOT escaped, so SQL containing literal
``:1`` inside string literals will be wrongly substituted. Phase 5
can add a proper SQL tokenizer.
Informix's wire protocol uses ``?`` natively, and we advertise
``paramstyle="numeric"`` to match the ESQL/C convention, so the
placeholders are rewritten on the way out.
This used to be ``re.sub(r":(\\d+)", "?", sql)``, which cannot see a
string literal and so rewrote the contents of one. Any ``HH:MM``
time, any URL with a port, any aspect ratio::
UPDATE jobs SET url = 'http://host:8080/x' WHERE id = ?
stored as 'http://host?/x'
Nothing about that is opt-in. The rewrite runs whenever a statement
has parameters, whatever placeholder style the caller actually used,
so writing ``?`` everywhere and never touching numeric style did not
protect you. It also changed the placeholder *count* while
``num_qmarks`` was still computed from ``len(params)``, leaving the
driver and the server disagreeing about how many binds exist.
So: a single pass that substitutes only outside quotes and comments.
The lexical rules are Informix's own, measured against 12.10, 14.10
and 15 rather than assumed from standard SQL:
* ``''`` doubling escapes a quote inside ``'...'``. A backslash does
**not** escape anything; ``'a\'b'`` is an unterminated string and
the server answers ``-282``. A Postgres-style scanner that honours
``\'`` would desync here and corrupt everything after it.
* ``"..."`` is a delimited identifier or a string depending on
``DELIMIDENT``. Either way its contents are not ours to touch.
* ``--`` runs to end of line, ``/* */`` does **not** nest (the first
``*/`` closes it; nesting is a syntax error), and ``{ }`` is a
comment too.
* ``::`` is the cast operator and is stepped over as a unit, so it
can never be read as the start of a placeholder.
An unterminated quote or comment consumes the rest of the string and
substitutes nothing further. That is deliberate: under-substituting
leaves the server to reject SQL that was already malformed, while
guessing would corrupt a literal.
"""
return _NUMERIC_PLACEHOLDER_RE.sub("?", sql)
if ":" not in sql:
return sql
out: list[str] = []
i = 0
n = len(sql)
while i < n:
ch = sql[i]
if ch in ("'", '"'):
j = i + 1
while j < n:
if sql[j] == ch:
if j + 1 < n and sql[j + 1] == ch:
j += 2 # doubled quote, still inside
continue
j += 1
break
j += 1
out.append(sql[i:j])
i = j
elif ch == "-" and sql.startswith("--", i):
j = sql.find("\n", i)
j = n if j == -1 else j
out.append(sql[i:j])
i = j
elif ch == "/" and sql.startswith("/*", i):
j = sql.find("*/", i + 2)
j = n if j == -1 else j + 2
out.append(sql[i:j])
i = j
elif ch == "{":
j = sql.find("}", i)
j = n if j == -1 else j + 1
out.append(sql[i:j])
i = j
elif ch == ":":
if sql.startswith("::", i):
out.append("::")
i += 2
continue
j = i + 1
while j < n and sql[j] in _ASCII_DIGITS:
j += 1
if j > i + 1:
out.append("?")
i = j
else:
out.append(ch)
i += 1
else:
out.append(ch)
i += 1
return "".join(out)
def _generate_cursor_name() -> str:

248
tests/test_placeholder_rewrite.py Normal file
View File

@ -0,0 +1,248 @@
"""Rewriting :N placeholders without rewriting the SQL around them.
We advertise ``paramstyle="numeric"`` to match Informix's ESQL/C
convention, and the wire protocol takes ``?``, so placeholders get
rewritten on the way out. That was ``re.sub(r":(\\d+)", "?", sql)``,
which cannot see a string literal and therefore rewrote the inside of
one::
UPDATE jobs SET url = 'http://host:8080/x' WHERE id = ?
stored as 'http://host?/x'
Any ``HH:MM`` time, any URL with a port, any aspect ratio, any
``key:value`` string. It wrote wrong data and said nothing.
Nothing about it was opt-in either. The rewrite runs whenever a
statement has parameters, whatever placeholder style the caller actually
used, so writing ``?`` everywhere and never touching numeric style did
not protect you. And it changed the placeholder *count* while
``num_qmarks`` was still computed from ``len(params)``, leaving driver
and server disagreeing about how many binds exist.
The lexical rules below are Informix's own, measured against 12.10,
14.10 and 15 rather than assumed from standard SQL. The one that matters
most is the backslash: ``'a\\'b'`` is an *unterminated string* to
Informix (``-282``), not an escaped quote. A scanner written to
Postgres habits would desync on it and corrupt everything after.
"""
from __future__ import annotations
import contextlib
import pytest
import informix_db
from informix_db.cursors import _rewrite_numeric_to_qmark as rewrite
from tests.conftest import ConnParams
# ---------------------------------------------------------------------------
# Substitution happens where it should
# ---------------------------------------------------------------------------
@pytest.mark.parametrize(
("sql", "expected"),
[
("SELECT * FROM t WHERE a = :1", "SELECT * FROM t WHERE a = ?"),
(
"SELECT * FROM t WHERE a = :1 AND b = :2",
"SELECT * FROM t WHERE a = ? AND b = ?",
),
("SELECT * FROM t WHERE a = :10", "SELECT * FROM t WHERE a = ?"),
# Already-? SQL is returned untouched, and cheaply.
("SELECT * FROM t WHERE a = ?", "SELECT * FROM t WHERE a = ?"),
("", ""),
],
)
def test_placeholders_are_rewritten(sql: str, expected: str) -> None:
assert rewrite(sql) == expected
# ---------------------------------------------------------------------------
# ...and nowhere else
# ---------------------------------------------------------------------------
@pytest.mark.parametrize(
("label", "sql", "expected"),
[
(
"url-with-port",
"UPDATE jobs SET url = 'http://host:8080/x' WHERE id = :1",
"UPDATE jobs SET url = 'http://host:8080/x' WHERE id = ?",
),
(
"time-of-day",
"INSERT INTO log VALUES ('started at 09:15:30', :1)",
"INSERT INTO log VALUES ('started at 09:15:30', ?)",
),
(
"aspect-ratio",
"UPDATE t SET ratio = '16:9' WHERE id = :1",
"UPDATE t SET ratio = '16:9' WHERE id = ?",
),
(
"doubled-quote-escape",
"SELECT 'it''s 10:30' FROM t WHERE a = :1",
"SELECT 'it''s 10:30' FROM t WHERE a = ?",
),
(
"delimited-identifier",
'SELECT "col:1" FROM t WHERE a = :1',
'SELECT "col:1" FROM t WHERE a = ?',
),
(
"line-comment",
"-- ticket :99\nSELECT * FROM t WHERE a = :1",
"-- ticket :99\nSELECT * FROM t WHERE a = ?",
),
(
"block-comment",
"SELECT /* not :99 */ * FROM t WHERE a = :1",
"SELECT /* not :99 */ * FROM t WHERE a = ?",
),
(
"brace-comment",
"SELECT { not :99 } * FROM t WHERE a = :1",
"SELECT { not :99 } * FROM t WHERE a = ?",
),
(
"cast-operator",
"SELECT a::INT FROM t WHERE a = :1",
"SELECT a::INT FROM t WHERE a = ?",
),
(
"colon-not-a-placeholder",
"SELECT a FROM t WHERE b = ':x' AND c = :1",
"SELECT a FROM t WHERE b = ':x' AND c = ?",
),
(
"literal-after-placeholder",
"SELECT * FROM t WHERE a = :1 AND b = '10:30'",
"SELECT * FROM t WHERE a = ? AND b = '10:30'",
),
],
)
def test_quotes_and_comments_are_left_alone(
label: str, sql: str, expected: str
) -> None:
assert rewrite(sql) == expected, label
def test_backslash_does_not_escape_a_quote() -> None:
"""Informix answers -282 for ``'a\\'b'``: the backslash is an ordinary
character and the string is unterminated. A scanner that treated it
as an escape would think it was still inside the literal and stop
substituting, or worse, resume in the wrong place."""
sql = "SELECT 'a\\' FROM t WHERE x = :1"
# The quote closes at the character after the backslash, so :1 is
# outside the literal and gets substituted.
assert rewrite(sql) == "SELECT 'a\\' FROM t WHERE x = ?"
def test_unterminated_quote_substitutes_nothing_further() -> None:
"""Under-substituting leaves the server to reject SQL that was
already malformed. Guessing would corrupt a literal."""
assert rewrite("SELECT 'oops :1 FROM t") == "SELECT 'oops :1 FROM t"
def test_unterminated_block_comment_substitutes_nothing_further() -> None:
assert rewrite("SELECT /* oops :1 FROM t") == "SELECT /* oops :1 FROM t"
def test_block_comments_do_not_nest() -> None:
"""Measured: ``/* a /* b */ c */`` is a syntax error on all three
servers, so the first ``*/`` closes the comment. Treating them as
nesting would swallow live SQL."""
assert (
rewrite("SELECT /* a /* b */ :1 FROM t")
== "SELECT /* a /* b */ ? FROM t"
)
# ---------------------------------------------------------------------------
# Against a real server
# ---------------------------------------------------------------------------
def _connect(conn_params: ConnParams) -> informix_db.Connection:
return informix_db.connect(
host=conn_params.host,
port=conn_params.port,
user=conn_params.user,
password=conn_params.password,
database=conn_params.database,
server=conn_params.server,
connect_timeout=10.0,
read_timeout=25.0,
autocommit=True,
)
@pytest.mark.integration
def test_colon_literals_round_trip(conn_params: ConnParams) -> None:
"""The bug as a user meets it: the value stored is not the value
written. Both placeholder styles, because the rewrite ran for both."""
with _connect(conn_params) as conn:
cur = conn.cursor()
cur.execute("CREATE TEMP TABLE t_rw (id INT, s VARCHAR(60))")
cur.execute(
"INSERT INTO t_rw VALUES (?, 'http://host:8080/path')", (1,)
)
cur.execute("INSERT INTO t_rw VALUES (:1, 'at 09:15:30')", (2,))
cur.execute("SELECT id, s FROM t_rw ORDER BY id")
assert cur.fetchall() == [
(1, "http://host:8080/path"),
(2, "at 09:15:30"),
]
@pytest.mark.integration
def test_colon_literals_round_trip_through_executemany(
conn_params: ConnParams,
) -> None:
"""executemany rewrites unconditionally, so it had the bug too."""
with _connect(conn_params) as conn:
cur = conn.cursor()
cur.execute("CREATE TEMP TABLE t_rw2 (id INT, s VARCHAR(40))")
cur.executemany(
"INSERT INTO t_rw2 VALUES (?, '16:9')", [(1,), (2,)]
)
cur.execute("SELECT DISTINCT s FROM t_rw2")
assert cur.fetchall() == [("16:9",)]
@pytest.mark.integration
def test_comment_bearing_sql_still_binds(conn_params: ConnParams) -> None:
"""A leading comment reaches PREPARE now that classification asks the
server. Make sure the rewriter agrees and doesn't eat a placeholder
that follows one."""
with _connect(conn_params) as conn:
cur = conn.cursor()
cur.execute(
"/* report: daily */ SELECT FIRST 1 tabid FROM systables "
"WHERE tabid > :1",
(0,),
)
assert cur.fetchone() is not None
@pytest.mark.integration
def test_placeholder_count_matches_after_rewrite(
conn_params: ConnParams,
) -> None:
"""The old regex could add placeholders the driver never counted,
leaving num_qmarks (from len(params)) disagreeing with the SQL. A
literal containing three colon-digit sequences is the shape that
used to break it."""
with _connect(conn_params) as conn:
cur = conn.cursor()
cur.execute("CREATE TEMP TABLE t_rw3 (id INT, s VARCHAR(40))")
with contextlib.suppress(Exception):
cur.execute("DELETE FROM t_rw3")
cur.execute(
"INSERT INTO t_rw3 VALUES (:1, 'a:1 b:2 c:3')", (7,)
)
cur.execute("SELECT id, s FROM t_rw3")
assert cur.fetchone() == (7, "a:1 b:2 c:3")