Skip to content

Client#prepare computes the connection-encoding conversion and then discards it — statements are sent in the wrong encoding - #1455

Merged
sodabrew merged 1 commit into
brianmario:masterfrom
jeremy:prepare-uses-exported-sql
Aug 6, 2026
Merged

Client#prepare computes the connection-encoding conversion and then discards it — statements are sent in the wrong encoding#1455
sodabrew merged 1 commit into
brianmario:masterfrom
jeremy:prepare-uses-exported-sql

Conversation

@jeremy

@jeremy jeremy commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

This is the two-line change; the evidence is below so it can be checked rather than taken on trust.

Summary

rb_mysql_stmt_new transcodes the SQL string to the connection encoding, then hands
mysql_stmt_prepare the pointer and length of the original, un-transcoded string. The
converted string is computed, stored in args.sql, and never used. So when the SQL string's
encoding differs from the connection encoding, Client#prepare sends the wrong bytes while
character_set_client says otherwise — and Client#query, given the identical string on the
identical connection, sends the right ones.

The result is silent: no error, no warning, just different data. client.query("SELECT 'é'…")
returns é and client.prepare("SELECT 'é'…").execute returns é.

This is a correctness bug, not a memory-safety bugsql_ptr and sql_len are both taken
from the original string, so they are consistent with each other and there is no over-read. I
checked that by execution rather than assuming it; details below.

It is reachable, not latent: Client#prepare is public API, and ActiveRecord's mysql2 adapter
reaches it for any query carrying binds. But it only bites when the statement text contains
non-ASCII and the string's encoding differs from the connection's — pure-ASCII SQL is
byte-identical in every ASCII-compatible encoding, which is presumably why this has gone unnoticed
since 2012.

Reproduction

Standalone; needs only a MySQL server. The control is Client#query on the identical string,
so the two lines that differ are the whole finding.

require "mysql2"

# The connection charset must DIFFER from the Ruby string's encoding, or
# rb_str_export_to_enc is a no-op and nothing can go wrong. latin1 vs UTF-8.
c = Mysql2::Client.new(host: "127.0.0.1", username: "root", password: "…",
                       database: "test", encoding: "latin1")

# A normal UTF-8 Ruby literal. U+00E9 exists in BOTH encodings but with
# different bytes and different lengths: C3 A9 in UTF-8, E9 in ISO-8859-1.
sql = "SELECT 'Aé' AS v, HEX('Aé') AS server_saw"

# Assert the conversion is actually real, so the test can't pass vacuously:
conv = sql.encode(Encoding::ISO_8859_1)
raise "vacuous" if conv.bytes == sql.bytes

q = c.query(sql).first                     # control  -- client.c, correct
p_ = c.prepare(sql).execute.first          # test     -- statement.c

puts "query   : #{q['v'].bytes.inspect}  server saw #{q['server_saw']}"
puts "prepare : #{p_['v'].bytes.inspect}  server saw #{p_['server_saw']}"

HEX() is evaluated server-side, so it reports what MySQL received, not what Ruby sent.

query   : [65, 233]       server saw 41E9        <- 'Aé', correct
prepare : [65, 195, 169]  server saw 41C3A9      <- 'Aé', raw UTF-8 bytes

3/3 on every run. Applying the fix below makes both lines read 41E9, 3/3.

Confirmed independently against the server's general log, which records the statement byte-exact:

bytes for the literal statement length
Query 41 E9 84
Prepare 41 C3 A9 86
Prepare, patched 41 E9 84

The prepared statement's length is the original string's bytesize, confirming both pointer and
length come from the pre-conversion string.

It can change SQL parse structure, not only produce mojibake

Worth knowing before deciding severity. In the UTF-8-string/latin1-connection direction above the
damage is confined to wrong characters — every byte of a UTF-8 multibyte sequence is ≥ 0x80 and
latin1 maps each to a printable character, so ' and \ can never be synthesised and quoting
stays intact.

But it is not confined in the other direction. Shift_JIS has characters whose trail byte is an
ASCII backslash — U+8868 「表」 is 95 5C. With encoding: "utf8mb4" and a Shift_JIS SQL string:

c = Mysql2::Client.new(..., encoding: "utf8mb4")
sql = "SELECT '表' AS a, 'SECOND' AS b".encode(Encoding::Shift_JIS)

c.query(sql).to_a      # => [{"a" => "表", "b" => "SECOND"}]     correct
c.prepare(sql).execute # => Mysql2::Error: …right syntax to use near '' AS b' at line 1

prepare ships the raw 95 5C; the utf8mb4 parser reads the 5C as an escape and swallows the
closing quote, so the literal does not terminate where the author wrote it. Here that lands on a
syntax error, but the mechanism is a quote-context change, i.e. the classic charset-mismatch shape.

I do not think this makes it a security issue, and I have not filed it as one: what selects the
mismatch is the statement text's encoding plus the connection option, both constants in
application source/config, and bound parameter values — where untrusted data actually lives —
go through rb_mysql_stmt_execute, which converts and uses the converted string correctly.
Flagging the reasoning explicitly so you can disagree with it rather than reconstruct it.

Cause

ext/mysql2/statement.c, rb_mysql_stmt_new:168-171. The comment states the
intent that the next two lines defeat:

    // ensure the string is in the encoding the connection is expecting
    args.sql = rb_str_export_to_enc(sql, conn_enc);   // the CONVERTED String
    args.sql_ptr = RSTRING_PTR(sql);                  // ...but from the ORIGINAL
    args.sql_len = RSTRING_LEN(sql);                  // ...and so is the length

rb_str_export_to_enc returns a new String when a conversion is needed, leaving sql
untouched — so args.sql is the only place the converted bytes exist, and it is the one thing not
passed to mysql_stmt_prepare.

The sibling query path in the same repo does it correctly —
ext/mysql2/client.c :865-867:

  args.sql = rb_str_export_to_enc(sql, rb_to_encoding(wrapper->encoding));
  args.sql_ptr = RSTRING_PTR(args.sql);
  args.sql_len = RSTRING_LEN(args.sql);

Provenance

Introduced in 162e8e7ae ("utf8 fieldnames", 2012-07-29), the commit that split the single
args.sql field into sql + sql_ptr/sql_len:

+    args.sql = sql;
+    args.sql = rb_str_export_to_enc(args.sql, conn_enc);
+    args.sql_ptr = StringValuePtr(sql);        // <- wrong variable
     args.sql_len = RSTRING_LEN(sql);           // <- wrong variable

The intent is legible in the diff itself: args.sql was introduced to be the buffer. It looks
like a straightforward slip that pure-ASCII SQL has hidden ever since.

git log --all -S "args.sql_ptr" -- ext/mysql2/statement.c returns exactly that one commit — the
line has not been touched since.

Not a memory-safety bug — checked, not assumed

sql_ptr and sql_len both come from sql, so they agree with each other and
mysql_stmt_prepare reads exactly RSTRING_LEN(sql) live bytes. I verified this with the
server's general log as the detector, using a deliberately-broken build as a positive control
(ptr from the converted string, len from the original, over a statement padded to a 200-byte
over-read):

build outcome
deliberately mismatched ptr/len Mysql2::Error syntax error — 200 bytes of heap the app never wrote arrive at the server
as-shipped prepare succeeds; log shows exactly bytesize bytes received, zero past the end of the SQL

So the detector demonstrably can see an over-read, and this defect does not produce one.

I also checked the lifetime question — args.sql holds the converted string, so nothing in the
struct anchors the original across the GVL-releasing call. 3,000 prepares under GC.stress, plus
240 prepares racing 39,291 GC.compacts from a second thread, across both embedded and
non-embedded String slot classes: 0 anomalies, GC.verify_compaction_references clean. As a
positive control that the harness can actually fail, the known fieldTypes unmarked-VALUE bug
(#1453) aborts 3/3 through it on the same build. sql is a method argument live on the VM stack
and the nogvl thread's machine stack is scanned conservatively, so it is pinned — the code is
correct here by luck of the calling convention rather than by design, but it is correct.

Affected versions

Every release that has prepared statements is affected — 0.4.0 through 0.5.7, and master.
I checked each tag mechanically rather than bisecting:

for t in $(git tag | sort -V); do
  git cat-file -e $t:ext/mysql2/statement.c 2>/dev/null || continue
  printf "%-8s %s\n" "$t" \
    "$(git show $t:ext/mysql2/statement.c | grep -c 'sql_ptr = \(StringValuePtr\|RSTRING_PTR\)(sql)')"
done
# 0.4.0 … 0.5.7  -> 1   (every one)
version affected how checked
master @ 2026-08-06 yes source — identical to 0.5.6
0.5.7 (latest release) yes source
0.5.6 yes built and reproduced, 3/3
0.5.0 – 0.5.5 yes source
0.4.0 – 0.4.10 yes source (args.sql_ptr = RSTRING_PTR(sql) at 0.4.0 :145)
< 0.4.0 n/a no statement.c — prepared statements did not exist

No version in the affected range is safe, incidentally or otherwise. Anything whose connection
encoding happens to equal the Ruby string encoding is spared by configuration, not by code —
the defect is still present.

Suggested fix

Two lines, matching the idiom already used at client.c:866-867:

--- a/ext/mysql2/statement.c
+++ b/ext/mysql2/statement.c
@@
     // ensure the string is in the encoding the connection is expecting
     args.sql = rb_str_export_to_enc(sql, conn_enc);
-    args.sql_ptr = RSTRING_PTR(sql);
-    args.sql_len = RSTRING_LEN(sql);
+    args.sql_ptr = RSTRING_PTR(args.sql);
+    args.sql_len = RSTRING_LEN(args.sql);

args.sql must keep holding the converted String — it is what keeps it reachable for the GC
across rb_thread_call_without_gvl.

Yes, I ran it. Rebuilt from source with only this change and re-ran the reproducer: prepare
and query agree, 3/3, on both 0.5.6 and a 0.5.4-based tree. The general log shows both paths
sending byte-identical statements.

A regression test would be the reproducer above: connect with encoding: "latin1", prepare a
statement whose text contains a non-ASCII character, and assert the round-trip matches
Client#query. Asserting the conversion is real first (sql.encode(...) != sql) matters — with a
matching connection encoding the test passes vacuously.

Environment

Printed from the running process, not from memory:

ruby            : ruby 3.4.10 (2026-06-30 revision 2b0b7728dc) +PRISM [aarch64-linux]
platform        : aarch64-unknown-linux (aarch64-linux)
Mysql2::VERSION : 0.5.6
extension       : built from source (not the packaged gem), in the same step that ran the test
client library  : MariaDB Connector/C -- linked libmariadb.so.3, Debian
                  libmariadb3 1:11.8.6-0+deb13u1 (via default-libmysqlclient-dev 1.1.1)
                  Mysql2::Client.info => {id: 30409, version: "3.4.9", header_version: "11.8.6"}
MySQL server    : 8.4.11 (official mysql:8 image), general_log=ON

The verdict depends only on encoding: differing from the Ruby string's encoding — that is the
whole precondition. It does not depend on the server version, the client library, or the
platform: the divergence is created entirely on the Ruby side, in rb_mysql_stmt_new, before any
bytes reach the wire. I note the MariaDB connector explicitly because it is what was linked, not
because it is implicated — mysql_stmt_prepare is handed a pointer and a length and faithfully
sends what it is given, which is the whole problem.

rb_mysql_stmt_new converts the SQL to the connection encoding and then
takes its pointer and length from the ORIGINAL string, so the conversion
is computed and discarded. client.c:877-879 does the same thing correctly
on the query path; this makes prepare match it.
Copilot AI lite review requested due to automatic review settings August 6, 2026 08:28
@jeremy jeremy changed the title Title Client#prepare computes the connection-encoding conversion and then discards it — statements are sent in the wrong encoding Aug 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@sodabrew

sodabrew commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Wow, that's been there a while! Nice catch!

@sodabrew
sodabrew merged commit 401b999 into brianmario:master Aug 6, 2026
1 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants