postgresql-interfaces/psqlodbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feedFrom: davecramer (@davecramer) <[email protected]>
To: postgresql-interfaces/psqlodbc <[email protected]>
Subject: Re: [postgresql-interfaces/psqlodbc] PR #178: Reject overlong cursor names
Date: Tue, 28 Apr 2026 10:41:11 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
Summary
This PR adds length validation to PGAPI_SetCursorName() so it rejects cursor names longer than
MAX_CURSOR_LEN (32), which is the value the driver advertises via
SQLGetInfo(SQL_MAX_CURSOR_NAME_LEN). Previously, the driver accepted arbitrarily long names despite
advertising a 32-character limit — an ODBC conformance gap.
The Problem
The ODBC spec says that if a driver reports SQL_MAX_CURSOR_NAME_LEN = 32, applications can rely on
that as the maximum. But PGAPI_SetCursorName() blindly accepted any length via make_string() and
stored it. This creates a contract violation: an application that queries the limit and trusts it
could be surprised by behavior differences, and overlong names could cause issues when sent to
PostgreSQL (which has its own NAMEDATALEN limit, typically 63).
The Fix
cursor_name = make_string(szCursor, cbCursor, NULL, 0);
if (!cursor_name)
{
SC_set_error(stmt, STMT_NO_MEMORY_ERROR, "No memory available to store cursor name", func);
return SQL_ERROR;
}
if (strlen(cursor_name) > MAX_CURSOR_LEN)
{
free(cursor_name);
SC_set_error(stmt, STMT_INVALID_CURSOR_NAME, "Cursor name exceeds SQL_MAX_CURSOR_NAME_LEN",
func);
return SQL_ERROR;
}
NULL_THE_NAME(stmt->cursor_name);
SET_NAME_DIRECTLY(stmt->cursor_name, cursor_name);
Assessment
Correctness: ✅ Good
The approach is sound: materialize the name first, validate, then commit. This avoids modifying
statement state on failure. The error codes are appropriate — STMT_NO_MEMORY_ERROR for allocation
failure, STMT_INVALID_CURSOR_NAME for the length violation. Looking at the ODBC spec,
SQLSetCursorName should return SQLSTATE 34000 for invalid cursor names, and
STMT_INVALID_CURSOR_NAME maps to that in psqlodbc's error handling.
Atomicity: ✅ Good
The old code did NULL_THE_NAME then SET_NAME_DIRECTLY(make_string(...)) — if make_string returned
NULL (OOM), the cursor name would be silently nulled out. The new code validates everything before
touching stmt->cursor_name. This is a nice improvement in error handling robustness.
Test: ✅ Appropriate
The test:
1. Queries SQL_MAX_CURSOR_NAME_LEN to get the actual limit (doesn't hardcode 32)
2. Creates a 63-byte cursor name (well over the 32 limit)
3. Asserts SQLSetCursorName fails
4. Continues with the rest of the existing test (setting a valid cursor name)
This is well-integrated into the existing cursor-name-test rather than creating a separate test
file.
Issues
1. Off-by-one question (non-blocking): > vs >=
The check is strlen(cursor_name) > MAX_CURSOR_LEN. This means a name of exactly 32 characters is
accepted. The ODBC spec says SQL_MAX_CURSOR_NAME_LEN is "the maximum length of a cursor name in the
data source" — so 32 characters should be valid, and 33+ should be rejected. The > comparison is
correct.
2. The make_string call allocates before validating (minor)
The function allocates a potentially large string just to measure its length and reject it. For a
cursor name this is negligible, but a pedantic approach would check the length from
szCursor/cbCursor before allocating. However, make_string handles SQL_NTS and other length
conventions, so replicating that logic would be error-prone. The current approach is the right
tradeoff — simple and correct.
3. Consider: should the test also verify the error state?
The test checks that SQLSetCursorName returns a failure code, but doesn't verify the SQLSTATE
(34000) or error message. This would make the test more precise, but it's not critical — the
important thing is that the overlong name is rejected.
4. SQLFreeStmt(hstmt, SQL_CLOSE) after the rejection
The test calls SQLFreeStmt(hstmt, SQL_CLOSE) after the rejection test. This is fine — it clears any
error state on the statement handle before proceeding with the valid cursor name test. Good
practice.
view thread (3+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: github://postgresql-interfaces/psqlodbc
Cc: [email protected], [email protected]
Subject: Re: [postgresql-interfaces/psqlodbc] PR #178: Reject overlong cursor names
In-Reply-To: <<[email protected]>>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox