Subject: [postgresql-interfaces/psqlodbc] PR #178: Reject overlong cursor names

From: "jarvis24young (@jarvis24young)" <[email-masked]>
Date: Tue, 28 Apr 2026 03:26:23 +0000

## Summary

This patch makes `SQLSetCursorName()` reject cursor names longer than the length advertised by the driver through `SQLGetInfo(SQL_MAX_CURSOR_NAME_LEN)`.

The driver reports `MAX_CURSOR_LEN` (`32`) for `SQL_MAX_CURSOR_NAME_LEN`, but `PGAPI_SetCursorName()` previously accepted and stored longer application-provided cursor names. The new check validates the fully materialized cursor name before replacing the statement's current cursor name.

## Details

`PGAPI_SetCursorName()` now:

- builds the requested cursor name into a temporary buffer first;
- reports `STMT_NO_MEMORY_ERROR` if allocation fails;
- rejects names longer than `MAX_CURSOR_LEN` with `STMT_INVALID_CURSOR_NAME`;
- only replaces `stmt->cursor_name` after allocation and validation both succeed.

This keeps the statement state unchanged on failure and makes `SQLSetCursorName()` consistent with the driver's advertised ODBC cursor-name limit.

## Regression test

The existing `cursor-name` black-box ODBC test now:

- queries `SQL_MAX_CURSOR_NAME_LEN` from the connected driver;
- builds an invalid cursor name with length `SQL_MAX_CURSOR_NAME_LEN + 1`;
- verifies that `SQLSetCursorName()` rejects it;
- verifies the diagnostic SQLSTATE is `34000`;
- then continues through the existing valid cursor-name path.

Verified in WSL against the unixODBC test path with an ASan/UBSan/gcov build:

```sh
cd ~/psqlodbc-build
make -j4
cd test
make LIBODBC='-lodbc' exe/cursor-name-test
ODBCSYSINI=. ODBCINSTINI=./odbcinst.ini ODBCINI=./odbc.ini \
  ASAN_OPTIONS=verify_asan_link_order=0:detect_leaks=0 \
  LD_PRELOAD=/lib/x86_64-linux-gnu/libasan.so.8 \
  ./runsuite cursor-name --inputdir=.
```

Result:

```text
TAP version 13
1..1
ok 1 - cursor-name
```

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Tue, 28 Apr 2026 10:41:11 +0000

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.

---

From: "jarvis24young (@jarvis24young)" <[email-masked]>
Date: Tue, 28 Apr 2026 11:41:35 +0000

Thanks for the review. I updated the regression test to also verify the diagnostic state for the rejected overlong cursor name. The test now builds the invalid name as SQL_MAX_CURSOR_NAME_LEN + 1, calls SQLGetDiagRec() after SQLSetCursorName() fails, and requires SQLSTATE 34000 before continuing with the existing valid cursor-name path.

Re-tested in WSL with the unixODBC black-box path and ASan/UBSan/gcov build:

```text
TAP version 13
1..1
ok 1 - cursor-name
```