Fix IvyStmtPrepare to honor query length from callers

[jiangdaoli11]

Fixes #2172

Summary

IvyStmtPrepare() mirrors OCI's OCIStmtPrepare2(stmttxt, stmtlen) semantics: the caller passes the exact statement length in query_len, and the statement text is not required to be NUL-terminated. The implementation, however, ignored query_len and stored the query via strdup(query) (i.e. strlen-based).

For any caller that passes a fixed-length buffer without a NUL terminator, strdup() over-reads past the caller's buffer until it happens upon a NUL byte, storing garbage as the statement text. The subsequent Parse/prepare then fails on the server (or executes the wrong statement), and stmtHandle->query_len becomes inconsistent with the actually-stored string, which also skews the escaping-buffer sizing in Ivyreplacenamebindtoposition().

Change

• src/interfaces/libpq/ivy-exec.c: in IvyStmtPrepare(), allocate query_len + 1 bytes, copy exactly query_len bytes and NUL-terminate the stored query; also add the missing malloc() failure check. After the fix, strlen(query) == query_len always holds.
• src/interfaces/libpq/ivytest/testlibpq_prepare_dml.c and expected output: add exec_prepare_nonul(), a regression test that prepares and executes a statement from a 4096-byte heap buffer filled with 'X' and containing no NUL anywhere, exactly the OCI pattern that used to break. Verified to fail with the old code (PQexecPrepared statement not return tuples properly, garbage XXXX... statement text) and to pass with the fix.

Notes

• pgindent was intentionally not run: the ivy files use the fork's own local style and are not pgindent-clean even at HEAD (e.g. >2000 diff lines for ivy-exec.c on a pristine copy); a full-file reformat would drown the fix in noise. The added code follows the existing local style of the file.
• Compilation: both changed C files build cleanly under the project's strict warning flags (-Wall -Werror=vla, etc.).
• Runtime: testlibpq_prepare_dml run against a live server produces output identical to expected/testlibpq_prepare_dml.out (including the new 40 nonul row); the same test fails against the unfixed library, confirming it guards the bug.
• The branch is based on the latest upstream master (63fb0bfe61) and contains a single commit touching only these three files.

Summary by CodeRabbit

• Bug Fixes

  • Prepared statements now correctly handle query text provided without a trailing NUL character.
  • Query lengths are preserved accurately, preventing potential reads beyond the supplied query data.
  • Memory allocation failures during statement preparation now return a clear error instead of proceeding unexpectedly.

• Tests

  • Added coverage validating preparation and execution of non-NUL-terminated queries.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: IvorySQL/IvorySQL/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: fc9c9917-f0db-4522-98db-f3806d2ddc60

📥 Commits

Reviewing files that changed from the base of the PR and between 63fb0bf and 09e0588.

📒 Files selected for processing (3)

• src/interfaces/libpq/ivy-exec.c
• src/interfaces/libpq/ivytest/expected/testlibpq_prepare_dml.out
• src/interfaces/libpq/ivytest/testlibpq_prepare_dml.c

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

IvyStmtPrepare now handles non-NUL-terminated query buffers using query_len. The prepared-DML test adds coverage for this input and expects the inserted nonul row.

Changes

Non-NUL Query Preparation

Layer / File(s)	Summary
Length-aware query storage src/interfaces/libpq/ivy-exec.c	IvyStmtPrepare allocates and copies exactly query_len bytes, appends a terminator, and handles allocation failure.
Non-NUL prepared-DML coverage src/interfaces/libpq/ivytest/testlibpq_prepare_dml.c, src/interfaces/libpq/ivytest/expected/testlibpq_prepare_dml.out	The test prepares and executes a non-NUL-terminated query buffer and adds the expected row with id 40 and name nonul.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 09e05

IvyStmtPrepare now safely handles non-NUL-terminated query buffers, and the added test exercises preparation and execution through that path. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: updating IvyStmtPrepare to use the caller-provided query length.

Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Wang1rrr]

Thanks for picking this up — the fix also resolves the variant reported in #2172.

One gap in the new coverage: exec_prepare_nonul() only exercises the case where the caller's buffer has no NUL within its bounds, i.e. the heap over-read. The original report in #2172 is a different and fully deterministic manifestation of the same root cause:

• the caller's buffer is NUL-terminated and holds valid SQL,
• but query_len is shorter than strlen(query), so the caller is asking to prepare a prefix.

With strdup() the trailing bytes are prepared and executed anyway, so no crash or sanitizer is needed to observe it:

const char *query = "SELECT :v + 1";
IvyStmtPrepare(stmt, error, query, strlen("SELECT :v"), 0, 0);
/* bind :v = 10, then execute */
printf("%s\n", Ivygetvalue(result, 0, 0));   /* before: 11, after this fix: 10 */

Before the patch that prints 11 (the whole literal was stored); with the exact-length copy it prints 10. The full repro is in #2172.

Could a second case along these lines be added next to exec_prepare_nonul()? It would pin the prefix semantics of query_len directly. The poisoned-buffer case only covers them indirectly: a future change that special-cases unterminated buffers (for example falling back to strlen() when a NUL is found inside the buffer) would still pass exec_prepare_nonul() while silently breaking prefix prepares.

[NotHimmel]

Thanks for contributing to IvorySQL!