fix: harden MySQL driver integer conversions to prevent crash on table open#1041
Merged
fix: harden MySQL driver integer conversions to prevent crash on table open#1041
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
iOS users crashed with
EXC_BREAKPOINT"Not enough bits to represent the passed value" when opening some MySQL tables — TestFlight report on a 100k-record table.MySQLActor.executedidInt(mysql_affected_rows(mysql)), but libmariadb is documented to return~(my_ulonglong)0(=UInt64.max) as an error sentinel ("for a SELECT, mysql_affected_rows() was called prior to mysql_store_result()"). The unchecked Int conversion trapped on the sentinel.A full-codebase audit confirmed the bug pattern is confined to MySQL — Postgres (
PQntuples/PQnfieldsreturnint), SQLite (sqlite3_column_bytes/sqlite3_changesreturnint), and Redis (usesInt64directly viaredisReply.integer) are all clean by the type system. The macOS MySQL plugin (MariaDBPluginConnection.swift) had the same per-cell length conversion shape with identical exposure but no reported crash; hardened in the same PR.Changes
MySQLDriver.swift:298, 341(iOS,MySQLActor.execute): detect the documented~0sentinel frommysql_affected_rowsand map to0rather thanInt(...)-trapping. ClampingUInt64.maxtoInt.maxwould surface "9 quintillion rows affected" in the UI, so the explicit sentinel branch is preferred.MySQLDriver.swift:330(iOS, row-fetch loop): per-cell length now usesInt(clamping: lengths?[i] ?? 0). A pathological length truncates toInt.maxand the downstreamData(bytes:count:)then either succeeds or fails the alloc with a recoverable error — both better thanSIGTRAP.MariaDBPluginConnection.swift:504, 918(macOS plugin, default-fetch + streaming paths): same per-cell length clamping. Removed the intermediatelengthValue: UIntbinding (no longer needed).The macOS plugin's
mysql_affected_rowscall site (line 430) was already safe — stores the value asUInt64directly without conversion. iOS was the regression.What's NOT in scope
for r in 0..<Int32(maxRows)style. Real-world safe (PG caps columns at 1600, maxRows ismin(_, 100_000)). Cosmetic refactor would dilute this PR's focus.TableProTests/Plugins/MySQLCreateTableTests.swiftcovers the SQL-generation path which is unchanged.Test plan
swiftlint lint --strictclean on both touched .swift filesUPDATE … LIMIT 0(zero rows affected) to exercise the no-result-set affected-rows branchSELECTover a TEXT column with multi-MB values to exercise the cell-length pathxcodebuild test -only-testing:TableProTests/MySQLCreateTableTests)