fix(ODBC): defensive UTF-16 length handling in makeUTF8 for Informix#5245
fix(ODBC): defensive UTF-16 length handling in makeUTF8 for Informix#5245
Conversation
There was a problem hiding this comment.
Pull request overview
Adds IBM Informix support to the Poco::Data::ODBC test suite and provides a Nix-based local environment to run those tests via a containerized Informix instance.
Changes:
- Register new
ODBCInformixTestin the ODBC test suite and build. - Add Informix-specific ODBC test implementation (schema recreation + suite wiring).
- Add
informix.nixNix shell to provision Informix (Podman) and configure unixODBC for running the tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Data/ODBC/testsuite/src/ODBCTestSuite.cpp | Adds Informix test suite registration. |
| Data/ODBC/testsuite/src/ODBCInformixTest.h | Declares Informix ODBC test class and overrides. |
| Data/ODBC/testsuite/src/ODBCInformixTest.cpp | Implements Informix-specific schema helpers and test suite wiring. |
| Data/ODBC/testsuite/Makefile | Includes Informix test object in build. |
| Data/ODBC/informix.nix | Provides Nix shell + Podman setup to run Informix ODBC tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Data/ODBC/informix.nix
Outdated
| # Also copy any versioned .so files | ||
| for lib in $(podman exec "$tmp_container" sh -c 'ls /opt/ibm/informix/lib/libif*.so* /opt/ibm/informix/lib/cli/libif*.so* 2>/dev/null' 2>/dev/null); do | ||
| local basename=$(basename "$lib") | ||
| local dirname=$(dirname "$lib" | sed 's|/opt/ibm/informix/||') | ||
| podman cp "$tmp_container:$lib" "$INFORMIX_CLIENT_DIR/$dirname/" 2>/dev/null || true | ||
| done |
There was a problem hiding this comment.
Valid — podman exec requires a running container, but this one is only podman created. The block is also redundant: lines 157-158 already podman cp the entire cli/ and esql/ directories (including versioned .so files), and lines 161-163 copy the named libraries from lib/. Removing the dead block.
| if [ ! -f "$HOME/.config/containers/policy.json" ]; then | ||
| cat > "$HOME/.config/containers/policy.json" << 'POLICYJSON' | ||
| { | ||
| "default": [ | ||
| { | ||
| "type": "insecureAcceptAnything" | ||
| } | ||
| ] | ||
| } | ||
| POLICYJSON | ||
| fi |
There was a problem hiding this comment.
Valid concern. The conditional only writes it if the file does not already exist, but it does affect all rootless Podman on the host. Could be improved by setting CONTAINERS_POLICY_JSON env var to a test-local file instead of writing to the global location.
matejk
left a comment
There was a problem hiding this comment.
Review
Overall the test class follows the existing ODBC test patterns (DB2, MySQL, PostgreSQL) closely — Informix-specific SQL types are correct (BYTE for BLOBs, DATETIME YEAR TO FRACTION(5) for timestamps), dropObject handles the right Informix error codes (-206, -319), and the Nix shell is well-documented.
Bug
podman exec on a stopped container (informix.nix line 172):
for lib in $(podman exec "$tmp_container" sh -c 'ls /opt/ibm/informix/lib/libif*.so* …'); doThe temp container is created with podman create (not started), so podman exec will always fail and this loop never runs. The earlier podman cp calls work on stopped containers, but exec does not. Fix by either starting/stopping the temp container around this block, or replacing the exec with a podman cp-based approach to enumerate files.
Issues
-
Comment says
sysmaster, DSN sayspoco_test(informix.nixline 15):# Test database: sysmaster (user: informix, password: poco)The actual test database is
poco_test(configured in the DSN on line 229, created on line 300).sysmasteris only used for health checks. -
Global
policy.jsonweakens Podman security (informix.nixlines 125-135):
WritinginsecureAcceptAnythingto~/.config/containers/policy.jsondisables image signature verification for all rootless Podman usage on the host, not just this test. Consider scoping viaCONTAINERS_POLICY_JSONenv var pointing to a test-local file, or at minimum documenting the trade-off. -
--privilegedcontainer (informix.nixline 271):
Running with--privilegedis common for Informix due to shared memory requirements, but a brief comment explaining why it's needed would help future readers. -
Copyright year in new files (
ODBCInformixTest.cpp,ODBCInformixTest.h):
Both sayCopyright (c) 2006— copied from existing test files. Since these are brand-new files, the year should reflect when they were actually written (2026).
Note on Copilot review
The Copilot review flagged recreateLogTable() line 287 as a bug, claiming the %s placeholder is never formatted. That's a false positive — session() << sql, "T_POCO_LOG", now; is the standard Poco Data binding pattern, used identically in ODBCPostgreSQLTest::recreateLogTable() and ODBCMySQLTest::recreateLogTable().
Added: Informix makeUTF8 fix (port of aleph-us/macchina.io#99)New commit ProblemInformix ODBC driver reports UTF-16 string lengths larger than the allocated wide-char buffer during column metadata retrieval ( FixReplaced strict validation with defensive clamping in both platform paths:
The fix is safe for other drivers — clamp conditions are only triggered when the driver over-reports lengths, which well-behaved drivers don't do. Tested on both MS SQL and Informix in macchina.io. Other fixes in this commit
|
Informix ODBC driver may report string lengths larger than the allocated wide-char buffer during metadata retrieval (e.g. SQLColAttribute, SQLDescribeCol), causing makeUTF8 to throw "Specified length exceeds available length". Replace strict validation with defensive clamping: handle non-positive lengths, clamp to buffer size, and align to wchar_t/SQLWCHAR boundary. Port of aleph-us/macchina.io#99 / PR #100. Safe for other drivers as clamp conditions are never triggered with correct length reporting. Also fix informix.nix comment (sysmaster -> poco_test) and copyright years in ODBCInformixTest files.
223151f to
4509cf0
Compare
|
@aleks-f , @pavledragisic: The docker image installed in informix.nix with padman does not contain Informix ODBC drivers. I tried with other docker images with no luck. Where did you get the drivers to test with? |
The Informix Developer Edition container does not include the Client SDK (CSDK/ODBC driver). Remove the non-functional extraction function and related code. Add a runtime check that warns if the driver is not registered with unixODBC, with a link to the IBM download page. Also document the --privileged flag requirement for Informix shared memory setup.
No description provided.