Make the database portable and encryptable (#3848) - #5526
Open
shai-almog wants to merge 30 commits into
Open
Conversation
The database API was five unrelated implementations sharing an interface. Cursors counted from zero on some ports and one on others, iOS reported success on an empty result set and returned null for every blob, the simulator could not seek at all, and no port could encrypt anything. This lands the port-independent half: - package-info.java now carries the normative contract every port must satisfy: zero-based positions, first() lands on a row, execute() runs a whole script while the parameterized forms take exactly one statement, typed parameter binding, flat transactions, IOException with a chained cause, idempotent close. - AbstractDBCursor derives all navigation from two primitives, rewind() and stepForward(), so every port gets identical semantics rather than each reimplementing them. Seeks rewind and re-step, which is what Android's windowed cursor already does on a window miss; buffering rows instead would mean materializing every column of every row stepped past. - SQLStatementSplitter splits a script the way SQLite does, respecting string literals, quoted identifiers, comments and CREATE TRIGGER bodies. - DatabaseConfig, DatabaseEncryptionException and ManagedKeys add keyed opens. Managed keys are resolved in the core so every platform derives identical material from an alias, and a key that cannot be stored is fatal rather than a silent downgrade to plaintext. - db.legacy restores each platform's previous behaviour for the ten changes that alter a previously successful result. It is read lazily, because the generated stubs set it after Display.init. Blob parameters now raise IOException rather than RuntimeException, and the truncated javadoc samples in Database, Cursor and Row are replaced with complete ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The simulator was the weakest database implementation, which mattered more than it sounds: it is where people develop. Its cursor could not seek at all, because the JDBC driver only produces TYPE_FORWARD_ONLY result sets and first(), last(), prev() and position() each threw outright. execute() silently ran the first statement of a script and discarded the rest. rollbackTransaction() left the connection outside autocommit, so every following statement quietly joined a new implicit transaction. Every query leaked its PreparedStatement. - SECursor now extends AbstractDBCursor, rewinding by re-executing the statement. The simulator has working random access for the first time. - execute(String) splits the script and runs each statement, rather than trusting a driver to decide how much of it to run. - The parameterized forms reject a multi-statement script instead of dropping its tail. - Statements are closed on the success path, cursors are closed with the database, close() is idempotent and rollback restores autocommit. - getColumnName reports the result set label, matching getColumnIndex, so an aliased column can be found under the name it was found by. The shaded driver moves from org.xerial to io.github.willena, which is the same driver with SQLite3MC compiled in: same package, same config, verified identical on plaintext databases, plus the SQLCipher-compatible cipher the simulator needs to open a database written on a device. getV4Defaults() is required over getDefault() - the latter selects SQLite3MC's own variant, which real SQLCipher cannot read. That driver also stops being frozen. Freezing assumed the shaded content never changed; it now carries a crypto-bearing engine that has to track upstream security releases. SEDatabaseConformanceTest runs the portable contract against the real SEDatabase headlessly in about two seconds, including both the strict and legacy modes and the encrypt/decrypt round trip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
iOS was the port the "radically different implementations" complaint is
really about, and it had real bugs behind the divergence:
- sqlDbClose called sqlite3_free on the connection handle. That never
closed it, leaked the file descriptor, skipped the WAL checkpoint and
handed the pointer to the wrong allocator. Now sqlite3_close_v2.
- sqlCursorValueAtColumnBlob was { return nil; }, so iOS could not read
a blob at all, in either direction.
- Opening a database called sqlite3_config(SQLITE_CONFIG_SERIALIZED) and,
on failure, sqlite3_shutdown(). That has to run before
sqlite3_initialize() to do anything, and calling shutdown with
connections open is undefined behaviour. Replaced with per-connection
SQLITE_OPEN_FULLMUTEX.
Behaviour now matches the portable contract:
- CursorImpl extends AbstractDBCursor, so last(), prev() and position()
work instead of throwing "Unsupported", and first() lands on a row and
reports false for an empty result set rather than reporting success and
leaving the statement unpositioned.
- Parameters bind by runtime type through new statement natives. They
used to be stringified, which stored an Integer as TEXT, and a comment
conceded it "will probably fail with blobs".
- Parameter count mismatches and multi-statement scripts in the
parameterized forms are rejected rather than silently mis-executed.
- Errors carry sqlite3_errmsg unconditionally; the dead XMLVM branches
that gated error reporting are gone.
- finalize() is removed from the database and cursor. Closing sqlite
handles from the GC thread is the "platform specific nuance" that
defeated ThreadSafeDatabase.
- Custom file:// database paths work, matching Android and the simulator.
Keying is a separate native that reports success rather than throwing, so
the Java side can tell a wrong key from a failure to open the file
without the native layer naming a core exception class.
isDatabaseEncryptionSupported() asks the linked engine via PRAGMA
cipher_version rather than assuming, so it reports honestly on a build
that does not bundle a cipher-capable SQLite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Android was already the most capable port, so this is mostly tightening rather than rebuilding: - A null element in a String[] now binds SQL NULL. bindString rejects null, so passing one used to fail the whole statement. - execute(sql, (Object[]) null) no longer dereferences a null array. - execute(String) runs a whole script. execSQL refuses anything after the first statement, so the script is split and run statement by statement. - executeQuery forces the window fill before returning, so malformed SQL is reported there rather than from the first next(). rawQuery is lazy. - Transactions use the shared flat-transaction guards, so a nested begin is rejected here as it already was everywhere else. - Exceptions carry their cause and are no longer printStackTrace'd on the way out. - Cursors are invalidated when the database closes, close() is idempotent, getRow() off a row throws, getColumnIndex is case insensitive, and wasNull() is false before any value has been read. - Blob query parameters work, bound through a cursor factory, which is the only supported route: rawQuery can carry text arguments only. This is what androidx.sqlite does for the same reason. Encryption lives in a new com/codename1/impl/android/cipher package built on net.zetetic:sqlcipher-android. It compiles against classes that are only on the classpath of app builds that use encryption, so it is excluded from the port's own javac and reached purely by reflection, letting the builder delete it for every app that never touches DatabaseConfig. That gating is why the package is a near copy of AndroidDB rather than a shared supertype: any shared type naming net.zetetic would have to live in the part of the port that must stay deletable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both ports inherited the base openOrCreateDB, which returns null, so Database.openOrCreate() handed back null and calling code failed with a NullPointerException. They now have a full implementation that satisfies the same contract as every other port, encryption included. Neither runs a JVM, so JDBC was never an option; they needed a C binding. That is cheap because both are ParparVM C targets whose CMake project already compiles every .c in the source root. - The engine is SQLite3 Multiple Ciphers, bundled once in the translator and emitted only for applications that use com.codename1.db. iOS shares the same copy, so those three targets run one engine at one version, and the simulator's JDBC driver is built from the same upstream project. - The amalgamation is named .h deliberately. The iOS project generator lists .h but excludes it from the compile phase; CMake globs *.c for sources; and the ParparVM native symbol scanner reads only .c and .m. Named .c it would be compiled twice without its build options, named .inc it would ship inside the .ipa as 13MB of dead weight. - cn1_sqlite3.c is the single translation unit that compiles it, with the build options set immediately before the include so they cannot leak into unrelated sources. It is gated internally, so an emitted but disabled build produces an empty object rather than a link error. - The binding itself is shared. Both ports need identical code but mangle their entry points from different Java classes, so the logic lives once in cn1_db_sqlite_impl.h and each port's .c expands CN1_DB_DEFINE_NATIVES for its own prefix. Verified that every declared native has both its plain and its _R_ symbol in both ports. - iOS stops linking the system libsqlite3 when the bundled engine is used, rather than carrying two SQLite implementations in one process. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The JavaScript port sat on WebSQL, which Chrome removed in 119 and Firefox never implemented, so its database was dead on every current browser. What it did support was thin: transactions were printlns, getBlob threw, position(n) always returned the first row, close() did nothing, and the bridge busy-waited a CN1 thread on a lock. It now runs the same SQLite build the other ports use, compiled to WebAssembly, inside the application's own worker. Every call after the first is an ordinary synchronous call; only the initial load suspends, through the runtime's existing yield-on-promise support, so the lock and its 200ms poll are gone. Storage uses the opfs-sahpool VFS rather than the default OPFS one. The default needs crossOriginIsolated, which needs COOP/COEP response headers, which we cannot require of the arbitrary static hosting these bundles are deployed to. Browsers without synchronous OPFS access fall back to memory with a console warning, because silently losing every write on reload is not a failure anyone should discover in production. Gating, so nobody pays for what they do not use: - iOS emits the bundled engine, and drops the system libsqlite3, only for applications that reference DatabaseConfig. Everyone else keeps the system SQLite exactly as before. - Windows and Linux emit it for anything referencing com.codename1.db, since they have no system SQLite at all, and its cipher only when encryption is configured. - Android's SQLCipher package is deleted unless DatabaseConfig is referenced, and the AAR arrives through a new PlatformFeatureCatalog entry keyed on that same class. - The JavaScript builder prunes the 1.5MB engine from bundles that never open a database. The catalog entry is keyed on DatabaseConfig rather than the db package on purpose, and two new tests hold that line: every database application references com.codename1.db, so keying it there would bundle SQLCipher for all of them and push the minimum Android SDK from 19 to 23 for people who never asked for encryption. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The contract and the encryption are only real if they are checked, and the portability claim in particular is the kind that fails silently: a cipher misconfiguration produces files each platform reads perfectly well on its own and nothing else can touch. - Seven device tests run the shared conformance suite on every port through the existing screenshot harness. They are assertion only, so they take no screenshots and sit before the ordering-sensitive graphics baselines. Ports without a database self-skip, so a port turns green on its own once it has one. - Two of the seven run in legacy mode, which is what makes the compatibility promise testable rather than aspirational: they fail the moment a refactor changes what db.legacy restores. - Two Port Status features expose the results publicly, split so a threading regression cannot blank the whole database row. - scripts/ci/db-cipher-interop.sh checks our encrypted files against the stock sqlcipher client in both directions, with a raw key to isolate the cipher configuration and a passphrase leg to cover the key derivation. Wired into the pull request workflow. The developer guide's SQL section said the iOS SQLite "isn't threadsafe" and warned that the garbage collector closing a connection would crash the app. That was true, and this branch is what fixes it, so the section is rewritten and extended with encryption, key management, threading, cursor cost and the legacy compatibility table. ThreadSafeDatabase is un-deprecated. Its note blamed platform nuances; the nuance was the iOS finalizers, now gone. Its close() was fire and forget, so it returned before the database was closed and a following delete() raced it, which is fixed here too. The cursor inner classes are static: with an explicit owner field the implicit outer reference was dead weight, which SpotBugs flagged on iOS and would eventually have flagged everywhere. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce77b834d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Companion PR with the build-side gating: codenameone/BuildDaemon#172 |
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
- The Ant build for the JavaSE port links whichever sqlite-jdbc is pinned in cn1-binaries, which has no org.sqlite.mc, so importing the driver's config builder broke that build for everyone. JavaSEPort now writes the SQLCipher connection properties out literally, which needs no extra class at compile time, and reports isDatabaseEncryptionSupported() by probing for the cipher-capable driver rather than assuming it. The simulator therefore answers honestly under either build. - The Windows cross-compile failed to link. The sample application now uses com.codename1.db, but that integration test drives the translator directly rather than through the builder, so the engine was never emitted and the natives had no definitions. Two fixes: the shared binding header is always emitted and defines every entry point either way, as real bindings or as stubs that raise a clear IOException, so an application always links however the translator was invoked; and the integration tests ask for the engine explicitly, so those ports actually exercise the database instead of only ever self-skipping. Verified that both branches of the header export an identical symbol set. - The developer guide requires snippets to live in docs/demos and be included by tag. Migrated with the repository's own migration script. The snippet harness had no com.codename1.db import, which is why all three failed to compile once moved; added, since it is a core package the guide documents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Cloudflare Preview
|
The Maven build already excluded it, but the Ant target compiles every source in the port, so it tried to build the package against net.zetetic and failed for anyone building that way -- including BuildDaemon CI, which clones this repo and runs the Ant target. Mirrors the exclusion into both places the ARCore and AI packages already use: the javac in Ports/Android/build.xml and the excludes property in nbproject/project.properties. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d595bd94da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 12 screenshots: 12 matched. |
Collaborator
Author
|
Compared 146 screenshots: 146 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 146 screenshots: 146 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 146 screenshots: 146 matched. Benchmark ResultsDetailed Performance Metrics
|
Review findings, all eight real: - Database.encrypt() could never work on Android. The system SQLite has no cipher, so a plaintext database opened through it can never be re-keyed. Added openOrCreateDBForRekey(), which Android routes through SQLCipher (an empty key opens an unencrypted file, which can then be re-keyed). - A managed key resolves its keystore alias from the database name, and every port passed null when re-keying, so changeKey(managed()) raised a NullPointerException instead of encrypting. Each Database now retains the name it was opened under. - Two threads first-opening the same managed database could each see nothing stored, generate different keys and overwrite each other, leaving one of them holding data nobody could ever read. The read-generate-store sequence is now serialized. - isKeyHardwareBacked() inferred hardware backing from the API level, but emulators and plenty of real devices back AndroidKeyStore keys in software. It now asks the key itself, via KeyInfo. Applications are told they may use this to refuse to store sensitive data, so it has to be true. - checkEndTransaction() cleared the flag before the engine had ended the transaction, so a failed commit left the transaction open while the API believed it was closed, and the recovering rollback was rejected. Splitting out markTransactionEnded() means the flag drops only on success. A conformance check covers the failed-commit path. - An encrypted Android database opened by file:// URL had no toNativePath() conversion, so java.io.File treated the URL as a literal relative name. - Calling next() past the end repeatedly re-derived the row count each time, inflating it, after which last() would seek to a row that does not exist. Verified the new check fails against the old code (5 became 8). - PRAGMA rekey interpolated the key directly, so a passphrase containing a quote produced a different statement. Both Android and the simulator now go through one helper that quotes text and passes a raw key literal through untouched. CI failures: - Six SpotBugs findings in core-unittests, a module the earlier local runs had not covered: boxed constructors, a default-encoding String, and a Boolean-returning method that could return null. - The arm64 Linux and Windows cross-builds failed compiling the engine's ARM AES intrinsics. Where the compiler defines __ARM_FEATURE_CRYPTO the engine uses them directly, which is what Apple's toolchain does, so iOS is unaffected; otherwise it tags individual functions with __attribute__((target)), which the cross-compiling clang does not honour for these intrinsics. Rather than require ARM crypto extensions of every chip, that path now uses the software implementation. - DatabaseStatementLegacyTest failed on Android because the legacy expectation was wrong, not the code: only iOS ran a whole script before this branch, through sqlite3_exec. Android's execSQL and the simulator's PreparedStatement both dropped everything after the first statement. Corrected in the suite and in both places it is documented. - The migrated guide snippet fixture needed a copyright header. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f2f2c70ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e28281912b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Measured on API 34 rather than reasoned about, after the previous two
attempts at this both failed in CI.
SQLiteDatabase.endTransaction() pops its own transaction record before it
sends the COMMIT, so a failed COMMIT leaves the engine holding the
transaction while the wrapper believes there is none. In that state
inTransaction() reports false, the uncommitted row is still visible to every
later read, and the next beginTransaction() fails with "cannot start a
transaction within a transaction".
The wrapper cannot end it. The session layer classifies a statement by its
first three characters, so execSQL("ROLLBACK") never reaches SQLite: "ROL"
becomes its own endTransaction(), which throws because it thinks no
transaction is open. A second endTransaction() throws for the same reason,
and compileStatement and rawQuery route through the same classifier. A
statement that does not begin with those characters is passed to the engine
unexamined, so the rollback goes out behind a leading comment.
Probed against every candidate on a real emulator: no attempt, plain
ROLLBACK, a second endTransaction, and the comment-prefixed form. Only the
last recovers. The shipped sequence is verified end to end -- a later
transaction commits normally and the uncommitted row is gone rather than
lingering.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 730a69a2aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Android encryption could never have worked. The reflective lookup asked for open(String, String) while the factory declares open(String, String, String), so every encrypted open raised NoSuchMethodException. What hid it is the more interesting half: the catch reported any failure as NOT_SUPPORTED, and the conformance suite treats an unsupported platform as a skip once the refusal is clean, which that satisfies. The bug produced a passing device run. A missing method on a class that is present is a broken build, not an unsupported platform, so the two are now separated and the second is loud. The same split applies to the re-key path, which used to fall back to the plaintext engine and would have turned a re-key into a silent no-op. Android bound every non-blob query argument as text, so a Long reached SQLite as TEXT and "SELECT ? = 42" and typeof(?) both answered wrongly. The cursor factory that already carried blobs carries every type, and is now the only strict-mode path; text coercion survives under db.legacy alone. The no-argument executeQuery overload skipped the parameter-count check on iOS, Linux, Windows and JavaScript, so a statement with placeholders ran with every slot left as NULL. Other fixes in this round: encrypt() and decrypt() created and converted an empty database when handed a name that does not exist, reporting success while the intended database sat untouched. ThreadSafeDatabase reported isInTransaction() false even directly after a begin, because the flag it read belongs to the wrapper and beginTransaction moves the underlying one; its CursorWrapper implemented only Cursor, so wrapping a database dropped CursorExt and count() answered -1 even on Android. wasNull() was the one shared cursor operation without a closed check. AndroidCipherFactory reported an unwritable directory or a full disk as WRONG_KEY, sending applications into prompting for a passphrase that cannot help; the simulator leaked the JDBC connection on that same rejection path because DatabaseEncryptionException is an IOException and missed the SQLException handler that owned the cleanup. The JavaScript bridge collapsed every open failure to the WRONG_KEY sentinel, including for opens with no key at all. Native delete discarded the result of remove(), so a read-only or locked file left delete() reporting success. The SLF4J finding on scripts/ci/db-cipher-interop.sh does not reproduce and is left alone: the script passes locally on JDK 8 with only the unshaded driver on the classpath, and CI has already run it green on this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb0456a7c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The cipher package could not compile. AndroidCipherDB lives in com.codename1.impl.android.cipher and uses three package-private members of AndroidCursor - CloseListener, setCloseListener and invalidate - from another package. The package is excluded from the port jar by design and compiled inside the generated Gradle app, so any application referencing DatabaseConfig would have failed its compile. Those three are now public, with a comment saying why the seam crosses a package boundary at all. Nothing caught it because nothing built it. The builders gate the cipher payload on the application's own classes referencing DatabaseConfig, and the device test reached encryption only through DatabaseConformanceSuite, which lives in the core jar and is not scanned. So the CI app shipped without a cipher, isEncryptionSupported() answered false, and the whole group logged status=SKIPPED. That is also why the reflective arity bug fixed in the previous commit survived: the path was never built, let alone run. DatabaseEncryptionTest now references DatabaseConfig directly, which is what a real application does and what the gate is designed to detect. Also from a self-review of the previous commit: Routing every Android query through the typed cursor factory dropped blob support in legacy mode, because the legacy branch coerced arguments to text including byte[]. Blob query parameters used to throw on every port, so the compatibility switch deliberately does not cover them; a blob now takes the factory in both modes. This is what the Android suite went red on. SQLCipher reads page one during open to settle the page size, so a wrong key surfaces there and not only from the schema probe - the same place the simulator's driver reports it. Splitting the catch had reclassified that as a plain IOException, losing WRONG_KEY on the most common path of all. The JavaScript bridge tagged any probe failure as a key failure even when no key was supplied, so a corrupt plaintext file still reported WRONG_KEY. The tag is now conditional on a key being present, and a failure to close the pooled handle no longer replaces the tagged error. The parameter-count checks on iOS, Linux, Windows and JavaScript ignored legacy mode, so an Ant project - which now defaults to compatibility mode - would have started throwing on a query it used to run unbound, on three platforms but not the other two. executeQuery(String, String[]) on Android still rejected a null element rather than binding SQL NULL, so the same query behaved differently depending only on the declared type of the array. isInTransaction() reads the underlying flag directly rather than through the worker. It is the one Database method with no IOException, so callers treat it as a cheap accessor and poll it from the event thread; dispatching would block that thread behind whatever statement is running. Restored the 26 bare-LF lines in AndroidImplementation.java that later edits normalized again, taking that file back to 114 changed lines from 140. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6f57d42cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
With the encryption path finally being built and run, the device suite reached code nobody had executed, and two SQLCipher behaviours the port had assumed turned out to be wrong. Both were measured on an API 34 emulator rather than reasoned about. PRAGMA rekey cannot go through execSQL. It answers with a row, and execSQL rejects anything that returns one - "Queries can be performed using SQLiteDatabase query or rawQuery methods only" - so every re-key failed before it began. That is the error the suite reported. It goes through rawQuery now. More seriously, SQLCipher re-keys only between two encrypted states. On a plaintext database, and on a re-key to the empty key, it refuses outright: "PRAGMA rekey can only be run on an existing encrypted database. Use sqlcipher_export() and ATTACH to convert encrypted/plaintext databases." Those two cases are precisely Database.encrypt and Database.decrypt, so both were broken on Android. The SQLite3MC build the other ports carry does re-key all three directions in place, which is why only this port needs the other route. Those conversions now ATTACH the target with its key, run sqlcipher_export, and swap the finished file in. sqlcipher_export copies schema and rows but not the header pragmas, so user_version is carried across explicitly - an application using it for schema versioning would otherwise silently come back at zero. The converted database is built beside the original and only replaces it once complete, so an interruption leaves the original live. Verified on-device end to end: encrypt leaves ciphertext on disk that no longer opens without the key, decrypt leaves a "SQLite format 3" header, a round trip returns the file to plaintext, and the row and user_version survive all three. Documented the platform difference in the guide, including what a leftover .cn1migrate file next to a database means. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 452c6cf58e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The first JavaScript suite run that reached the database tests wedged on DatabaseLifecycleTest and never finished, taking the whole 40 minute budget with it. The thread dump is unambiguous: the event thread was parked in "await" with every other thread queued behind it and nothing runnable. The engine bring-up is reached through the runtime's yield-on-promise bridge, which resumes the calling thread when the promise settles. Neither half of the bring-up is guaranteed to settle. Acquiring a synchronous OPFS access handle blocks while another context holds the same file, so a browser that never releases it leaves the promise pending indefinitely rather than rejecting, and a pending promise parks the event thread with nothing logged anywhere. A rejection was no better: the bridge resumes on resolution, so an importScripts or module-init failure hung in the same way. Both halves are now bounded by a timeout and the promise never rejects. A storage pool that will not open falls back to the in-memory VFS, which already existed for browsers without synchronous OPFS, so the database still works and the tests still run. An engine that will not load at all resolves to false, which the port already turns into a clean IOException and the suite reports as a skip with a reason. The same class of problem was in the open binding, which this also fixes: it had been changed to rethrow non-key failures so callers could tell storage errors from authentication errors, but an exception raised inside a native binding does not arrive in the translated code as a Java throwable - it unwinds the worker and hangs every thread waiting on the call. The classification now travels as a zero peer plus two accessors, so the caller still distinguishes a wrong key from a corrupt or unreadable database without anything being thrown across the bridge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 952f337007
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
…cked runLifecycle required the value from getDatabasePath to resolve through FileSystemStorage. That holds where databases are files, which is every port except JavaScript: there they live in a browser storage pool keyed by name, and the port says so by reporting no custom-path support and returning the name as an opaque handle. The check would have gone red the moment the JavaScript suite stopped hanging and actually reached it. The assertion is now gated on isCustomPathSupported(), with a note recorded for the ports where it does not apply, and the stale javadoc claiming getDatabasePath returns null on JavaScript is corrected to describe what it actually returns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a70937adb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The migration swap could lose data. Deleting the original before renaming
the converted file in leaves an interval where the only copy is under a name
nothing looks for; the error path then recreated an empty database at the
live name, and the next conversion removed the stranded copy as stale
leftovers. It now renames the original aside instead of deleting it, so a
complete database exists under one of the two names at every instant, and
AndroidCipherFactory restores from the backup before opening if the process
died in the gap.
Both schema probes reported every failure as a wrong key. A malformed image
or a read error is not something a key repairs, so an application following
the error codes would prompt for a passphrase forever. Android now applies
the same isNotADatabase test the open path uses, and JavaScript gained its
counterpart: a wrong key looks like a file that is not a database, because
the plaintext it produces has no valid header, while SQLITE_CORRUPT is a
different thing entirely.
A non-null empty argument array took the no-parameter shortcut on all seven
implementations, so execute("INSERT ... VALUES (?)", new Object[0]) ran with
the slot unbound. Only a null array takes that path now; an empty one is a
parameterized call and is held to the count like any other.
The OPFS pool was installed at its six-file default, which is a hard ceiling
on databases and lower than it looks because journal files take slots too.
Raised to 64, where an unused slot is an empty file and costs nothing.
Android leaked a cursor whenever eager validation rejected a query: the
cursor was never handed out and never registered, so nothing would close it
and its query kept the database referenced until GC.
ThreadSafeDatabase read the underlying transaction flag with no
happens-before against the worker that writes it, so a polling thread could
see stale state indefinitely on a class whose whole purpose is sharing. The
read now takes the dispatch lock, which is what publishes the write, while
still not queueing work behind a running statement.
On the JavaScript builders the staging directory holds the framework's own
classes, so a scan of it reports database use for every application and
cannot be made precise. The launcher now reaches the compatibility switch
reflectively, which removes both consequences: nothing is pinned for the
optimizer, and a project built against a core predating setLegacyBehavior
still compiles rather than failing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62e22710b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The open binding was fixed last round and the reason documented there, but the rest of the bindings still let exceptions escape - so a syntax error, a constraint violation or any ordinary SQL failure unwound the worker and left every thread blocked on a call that never returned. An application would hang rather than see the error. This is the same defect the suite already hung on once, in a different binding. The whole SPI now reports failure by value. A shared error channel holds the message, and each fallible call answers with a sentinel the Java side turns back into the IOException this API promises: execScript, rekey and executeAndFinish return a boolean, prepare keeps its zero peer, and step became an int because a boolean had no room to say "failed". The rest are wrapped too, so nothing in the file can throw at all. Also in this round: The native open leaked its handle on failure. throwException longjmps, so the close after it never ran, and SQLite returns a usable handle even from an unsuccessful open. The message is now built and the handle closed before raising. A zero length blob came back as null on Linux and Windows, because SQLite may return a null pointer with a count of zero and that was read as SQL NULL - disagreeing with both the column type and wasNull(). sqlcipher_export does not carry application_id any more than it carries user_version, so the Android migration now restores both. The iOS key probe reported any non-OK result as a wrong key. It now returns the SQLite result, and only SQLITE_NOTADB - which is what a key that did not decrypt the file looks like - is reported as WRONG_KEY. The JavaScript builder scanned for database use only on the bundled-jar path, so a build from a source checkout never set the flag and neither the compatibility default nor the asset pruning applied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a11dc9bae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ThreadSafeDatabase deadlocked on any unchecked exception. The dispatch callbacks caught only IOException, so anything else escaped into EasyThread, whose outer loop swallowed it without delivering a result - while the caller waited synchronously holding dispatchLock. One CursorIndexOutOfBoundsException from a bad column index wedged the database and everything queued behind it forever. Both callbacks now catch Throwable and carry it back, re-raised in kind: unchecked as itself, anything else wrapped in IOException. The value-returning overload uses a Failure holder rather than an instanceof test, since a query may legitimately return a Throwable as a value. Migration recovery only ran in AndroidCipherFactory, which a plaintext open never reaches, so a database interrupted mid-conversion could be opened by the ordinary path and replaced with an empty one. It moved to AndroidImplementation, which both paths go through - and which survives the build-time deletion of the cipher package, so a database left behind by a build that had encryption is still recoverable by one that does not. The migration backup was deleted without checking. It holds the database in its previous form, so after an encrypt it is a plaintext copy of what is now an encrypted database sitting at a predictable name. A failed delete left it there while encrypt() reported success, which defeats encryption at rest more quietly than a failed conversion would. It now raises rather than returning as though nothing were wrong. Also, the reflective compatibility-switch call from the round before was wrong in the other direction: ParparVM does not retain a member reached only reflectively, so it would have been culled and the failure silently left the application on the new behaviour. The launcher above it calls SVGRegistry.installGlobal directly for exactly that reason. It is a direct call again, emitted only when the staged core actually carries the switch, which DatabaseConfig's presence establishes. A core predating it has nothing to set and already behaves the old way, so emitting nothing there reaches the same result rather than compromising. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4d3c89764
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Recovery handled only half the interrupted-conversion cases. It restored a backup when the live file was missing, but ignored the rename result: if that failed, the open continued and created an empty database over nothing, and the next conversion removed the backup holding the real data as stale. And when both files existed - the swap completed but the cleanup was lost - it deliberately did nothing, leaving the database in its previous form at a predictable name. After an encrypt that is a plaintext copy of an encrypted database, which is the encryption-at-rest hole again, arrived at slowly. Both cases now act: a failed restore refuses to open rather than replacing the data, and a completed swap removes the leftover or says why it could not. changeKey's file swap bypassed the cursor cleanup that close() does, so a cursor open across a conversion kept the old connection alive while still reporting itself usable, and would then read the file about to be replaced or fail with an unchecked error from a closed pool. The tracked cursors are invalidated before the connection is released, as they are on close. On JavaScript, any OPFS failure fell back to an in-memory database. That is right for a browser with no synchronous OPFS at all, and wrong for everything else: contention with another tab, a quota, a timeout. In those cases an existing persistent database would appear empty and every write would vanish at page close - silent data loss presented as a working database. Only genuine unsupport takes the memory path now; anything else reports the database as unavailable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 483acb2bfd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if (!target.renameTo(original)) { | ||
| // Put it back exactly as it was. | ||
| backup.renameTo(original); |
There was a problem hiding this comment.
When installing the converted file fails, this ignores a failed backup.renameTo(original) and immediately calls the open-or-create openAt(path, currentKey). If restoration fails transiently, that call can create an empty live database; the next recovery then sees both files and deletes the backup containing the real data. Fresh evidence after the earlier recovery fix is that this target-install failure branch still does not check the restoration result, so it must fail without reopening when the backup cannot be restored.
Useful? React with 👍 / 👎.
| if (sqlite3_key(db, k, (int)strlen(k)) != SQLITE_OK) { \ | ||
| return JAVA_FALSE; \ | ||
| } \ | ||
| return cn1DbProbeKey(db) == SQLITE_OK ? JAVA_TRUE : JAVA_FALSE; \ |
There was a problem hiding this comment.
When the Linux or Windows key probe returns SQLITE_CORRUPT, SQLITE_IOERR, or another non-authentication failure, this still collapses it to false, and the Java constructors convert that boolean to WRONG_KEY. Fresh evidence after the earlier key-probe finding was marked fixed is that the current shared binding still returns only this boolean; preserve the SQLite status so only SQLITE_NOTADB becomes a wrong-key error.
Useful? React with 👍 / 👎.
| int rc = sqlite3_open_v2([nsPath UTF8String], &db, | ||
| SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX, NULL); | ||
| if (rc != SQLITE_OK) { | ||
| cn1ThrowSqlError(CN1_THREAD_STATE_PASS_ARG db, "Failed to open the database"); |
There was a problem hiding this comment.
When sqlite3_open_v2() fails after returning a non-null handle, cn1ThrowSqlError() invokes throwException(), which longjmps, so the following sqlite3_close_v2(db) is unreachable. The shared Linux/Windows implementation now copies the message and closes first, but this iOS path retains the old ordering; repeated caught open failures therefore leak SQLite handles and file descriptors.
Useful? React with 👍 / 👎.
| sqlite3_config(SQLITE_CONFIG_SERIALIZED); | ||
| sqlite3_initialize(); | ||
| NSString* nsPath = toNSString(CN1_THREAD_STATE_PASS_ARG path); | ||
| [[NSFileManager defaultManager] removeItemAtPath:nsPath error:nil]; |
There was a problem hiding this comment.
When iOS cannot remove an existing database—for example because file protection or a filesystem error prevents deletion—the NSError is discarded and Database.delete() returns successfully while the file remains. This contradicts the new portable deletion contract and the binding's own claim that every error becomes an IOException; capture the removal error and raise it while continuing to treat a nonexistent file as a no-op.
Useful? React with 👍 / 👎.
The cross-package visibility mistake that broke the Android app build was the same one made two rounds ago, for the same reason: that package is excluded from the port's compilation so its sources can be staged into the generated application, which means no build in this repository type-checks it. A member of com.codename1.impl.android it reaches can quietly stop being public, every local build passes, and an Android device job says so forty minutes into CI. DATABASE_BACKUP_SUFFIX and recoverInterruptedDatabaseMigration are public now. More usefully, scripts/ci/check-android-cipher-package-compiles.sh compiles the package the way the application build does - against the compiled port classes, with stub net.zetetic types - and takes seconds. Wired into the Java 8 CI leg after the port is built. Verified by reversion: with the member package-private again the port itself compiles with zero errors and the check reports exactly what CI reported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81cbb366f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+271
to
+273
| try { | ||
| underlying.close(); | ||
| } catch (IOException err) { |
There was a problem hiding this comment.
If the underlying database's close() throws a RuntimeException or Error, this synchronous callback still lets it escape; EasyThread catches it in its outer loop without invoking the success callback, so close() blocks forever while holding dispatchLock and never reaches et.kill(). Fresh evidence after the earlier unchecked-dispatch finding is that the normal invocation helpers now catch Throwable, but this newly synchronous close branch still catches only IOException; it must also guarantee callback completion.
Useful? React with 👍 / 👎.
Comment on lines
+6794
to
+6795
| if (cn1SqlitePool) { | ||
| cn1SqlitePool.unlink(cn1SqliteDbPath(n)); |
There was a problem hiding this comment.
When OPFS unlink() throws—for example because the storage operation fails or the database file is still in use—the exception escapes this native binding instead of reaching deleteDB() as its declared IOException; as the open binding documents, an escaping native exception unwinds the JavaScript worker and leaves the translated Java caller waiting indefinitely. Fresh evidence after the earlier native-error-transport fix is that exec/prepare/step are guarded now, but this deletion binding remains unguarded and has no result sentinel for Java to check.
Useful? React with 👍 / 👎.
| return; \ | ||
| } \ | ||
| arr = (JAVA_ARRAY)value; \ | ||
| sqlite3_bind_blob(stmt, index, arr->data, arr->length, SQLITE_TRANSIENT); \ |
There was a problem hiding this comment.
When a blob exceeds SQLite's configured length limit or SQLite cannot allocate its copy, sqlite3_bind_blob() returns SQLITE_TOOBIG or SQLITE_NOMEM, but this result is discarded. SQLite unbinds the previous value before attempting the copy, so execution can continue with SQL NULL rather than the caller's blob and report success; check every bind return code and raise an IOException before stepping. The corresponding iOS bind functions have the same unchecked-return path.
Useful? React with 👍 / 👎.
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.
Resolves #3848.
The request was database encryption. Encryption is here, but the reason it took a
whole PR is that
com.codename1.dbwas not one API over SQLite -- it was fiveunrelated implementations that happened to share an interface, and there was no
sensible place to add a key to.
What was actually wrong
Verified in the source, not from memory:
openOrCreatenull, callers NPElast()/prev()/position()IOException("Unsupported")position(n)always gave row 0getPosition()basefirst()trueon an empty set, then reads unset memorygetBlob{ return nil; }execute(sql)multi-statementBEGINprintlnno-opsRuntimeExceptionon every portPlus three defects worth calling out on their own:
sqlDbClosecalledsqlite3_freeon asqlite3*, so no iOS connection was ever closed, the WAL wasnever checkpointed and the handle went to the wrong allocator;
SEDatabaseleakeda
PreparedStatementper query; andThreadSafeDatabase.close()was fire andforget, so a following
delete()raced it.And no device test touched
Databaseat all -- 142 test classes in the screenshotsuite, none of them about databases. That is why Windows and Linux were allowed to
ship with no implementation.
What this does
One contract.
com.codename1.db/package-info.javanow states what every portmust do, and
DatabaseConformanceSuitein the framework checks it. Seven devicetests run that suite on every port in CI; two of them run in legacy mode.
One cursor implementation.
AbstractDBCursorderives all navigation from twoprimitives,
rewind()andstepForward(), so ports stop re-deriving it. Seeksrewind and re-step rather than buffering:
sqlite3_column_*is only valid on thecurrent row, so buffering would mean copying every column of every row stepped
past, blobs included. This is what Android's windowed cursor already does on a
window miss.
Encryption, with a passphrase, a keystore-managed random key, or raw bytes.
Managed keys resolve in the core so every platform derives identical material from
an alias, and a key that cannot be stored is fatal rather than a silent downgrade
to plaintext.
Windows and Linux get a database at all.
JavaScript stops using WebSQL, which Chrome removed in 119 and Firefox never
implemented, in favour of the same SQLite compiled to WebAssembly.
Compatibility
Ten behaviours change in ways an application could depend on. All ten are restored
by the
db.legacybuild hint, per platform, and two device tests assert that itreally does restore them -- so the promise is testable rather than aspirational.
The table is in the developer guide.
The hint deliberately does not cover defects, or capabilities that used to throw
and now work. Nobody can depend on
getBlobreturning null.Cost, when unused
Nothing. iOS keeps the system SQLite unless the app references
DatabaseConfig;Android's SQLCipher package is deleted and its AAR never added; Windows and Linux
compile the engine to an empty object; the JavaScript builder prunes 1.5MB from
bundles that never open a database. Two catalog tests hold that line, because the
entry is keyed on
DatabaseConfigrather than the package -- keying it on thepackage would bundle SQLCipher for every database app and push Android's minimum
SDK from 19 to 23 for people who never asked for encryption.
Verification
SEDatabaseConformanceTestcases, all green.android,ios,codenameone-maven-pluginandByteCodeTranslator.scripts/ci/db-cipher-interop.sh, wired into PR CI, writes an encrypted databasewith our engine and reads it with the stock
sqlcipherclient, and vice versa,with both a raw key and a passphrase. This is the check that matters: a cipher
misconfiguration produces files each platform reads happily and nothing else can
touch, which no single-platform test would catch.
sqlcipher4.17.0 client and the realnet.zetetic:sqlcipher-androidAAR, not against assumed APIs.Three things the spikes caught
Worth recording, because each would have shipped broken:
sqlcipher_export()does not exist in SQLite3MC, so the ATTACH-basedmigration everyone writes would have failed.
PRAGMA rekeyworks, and alsopreserves
user_version, whichsqlcipher_exportdrops.getConnection()on the simulator but on first read onthe device ports, so both paths need handling.
SQLiteMCSqlCipherConfig.getDefault()really does produce files real SQLCiphercannot open;
getV4Defaults()is required. One line, and nothing but across-engine test would have found it.
Review rounds
Nineteen findings from the automated reviewers, all real, all fixed. The ones worth knowing about:
Database.encrypt()could never have worked on Android. The system SQLite has no cipher, so aplaintext database opened through it can never be re-keyed; there is now a platform hook that
routes the migration through SQLCipher.
nullwhen re-keying, so
changeKey(managed())raised aNullPointerExceptionrather than encrypting./,\,:and space all to_, socustomer/dbandcustomer_dbshared one key and forgetting either destroyed the other.
and
sqlite3_close_v2then leaves a zombie connection alive forever.isEncrypted()reported every plaintext JavaScript database as encrypted, because that port hasno readable path and a failed header read is indistinguishable from ciphertext.
PRAGMA rekeyinterpolated the key directly, so a passphrase containing a quote changed thestatement.
Two of the fixes are covered by new conformance checks, including one verified by reinstating the
old code and watching it fail: the exhausted-cursor count went 5 to 8 before the fix.
Two decisions worth a second opinion
maven/sqlite-jdbcis no longer frozen. It was pinned and excluded frompublication because a shade of a fixed driver never changed. It now carries the
engine used to read encrypted databases, so it has to track upstream security
releases. Costs ~13.5MB per release, which is what the freeze was avoiding.
compile. It ships a prebuilt amalgamation where SQLCipher would need its
configure script run per build, and it is what the simulator's JDBC driver is
already built from -- so iOS, Windows, Linux, JavaScript and the simulator all
run one engine at one version. Android still uses the SQLCipher AAR because it
cannot compile C in our build; both write the same format, which is the part
that matters.
Companion PR
The build-side gating is mirrored in codenameone/BuildDaemon#172, which is green.
🤖 Generated with Claude Code