fix: write statement attributes at full SQLULEN width

Every non-pointer attribute on the SQLSetStmtAttr page is declared "An SQLULEN
value" — not one is SQLUINTEGER — and BufferLength is ignored for them, so the
application's buffer is SQLULEN-wide and it is the driver's job to fill it.
SQLGetStmtAttr's Comments describe the alternative as a defect to be worked
around: "some drivers may only write the lower 32-bit or 16-bit of a buffer and
leave the higher-order bit unchanged. Therefore, applications should use a
buffer of SQLULEN and initialize the value to 0 before calling this function."

An application that did not zero its buffer read SQL_ATTR_MAX_ROWS as
0xFFFFFFFF00000000 where the driver meant "no limit". Reported from the Trino
driver, reproduced here with a poisoned buffer before the fix.

On the connection side only SQL_ATTR_ASYNC_ENABLE and SQL_ATTR_ODBC_CURSORS are
SQLULEN; the rest really are SQLUINTEGER, so that stays a two-attribute
exception rather than a blanket change. The test buffers on the statement side
were u32 slots receiving the wider write, and are now SQLULEN too.

Also adds a pre-commit hook rejecting duplicate registered test names. A
function carrying two #[test] attributes is registered twice while its
neighbour, whose attribute it absorbed, stops running — both leave the suite
green, so the registered names are the only place either shows up. Verified to
fail on an injected duplicate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: maltesander

.pre-commit-config.yaml

@@ -30,6 +30,17 @@ repos:
         stages: [pre-commit, pre-merge-commit]
         pass_filenames: false
         files: \.rs$|Cargo\.(toml|lock)
+      # A test function carrying two `#[test]` attributes is registered twice
+      # and its neighbour, whose attribute was absorbed, silently stops
+      # running. Both outcomes leave the suite green, so only the registered
+      # names expose them.
+      - id: no-duplicate-test-names
+        name: no-duplicate-test-names
+        language: system
+        entry: ./scripts/no-duplicate-test-names.sh
+        stages: [pre-commit, pre-merge-commit]
+        pass_filenames: false
+        files: \.rs$
 
       - id: cargo-rustfmt
         name: cargo-rustfmt

CHANGELOG.md

@@ -878,6 +878,22 @@ Everything a driver has to change for the catalog rework, in one place.
 
 ### Fixed
 
+- **`SQLGetStmtAttr` wrote four bytes where the spec declares `SQLULEN`.** Every
+  non-pointer attribute on the `SQLSetStmtAttr` page is declared "An SQLULEN
+  value" — not one is `SQLUINTEGER` — and `BufferLength` is ignored for them, so
+  the application's buffer is `SQLULEN`-wide and the driver must fill it.
+  `SQLGetStmtAttr`'s own Comments describe the alternative as a defect to work
+  around: "if the value is a SQLULEN value, some drivers may only write the
+  lower 32-bit or 16-bit of a buffer and leave the higher-order bit unchanged.
+  Therefore, applications should use a buffer of SQLULEN and initialize the
+  value to 0 before calling this function." An application that did not zero its
+  buffer read `SQL_ATTR_MAX_ROWS` as `0xFFFFFFFF00000000` — an enormous row
+  limit — where the driver meant "no limit". All twenty integer-valued reads now
+  write a full `SQLULEN`; the pointer-valued ones already did. On the connection
+  side only `SQL_ATTR_ASYNC_ENABLE` and `SQL_ATTR_ODBC_CURSORS` are `SQLULEN`
+  and were widened; every other connection attribute really is `SQLUINTEGER` and
+  is unchanged.
+
 - **`SQLSetStmtAttr` now answers for every value it cannot honour.** Core drives
   one forward-only, read-only cursor over one parameter set, and the spec gives
   two ways to say so. The `01S02` row names the eight attributes a driver may

scripts/no-duplicate-test-names.sh

@@ -0,0 +1,20 @@
+#!/usr/bin/env bash
+# Fails when the test binary registers the same test name twice.
+#
+# A function with two `#[test]` attributes is registered twice, and the
+# function whose attribute was absorbed stops running altogether. Both leave
+# `cargo test` green — the count moves, but nothing fails — so the registered
+# names are the only place either shows up.
+set -euo pipefail
+
+duplicates=$(cargo test --locked --lib -- --list 2>/dev/null | grep ': test$' | sort | uniq -d)
+
+if [[ -n "$duplicates" ]]; then
+    echo "These test names are registered more than once:" >&2
+    echo "$duplicates" >&2
+    echo >&2
+    echo "A duplicate almost always means one function carries two #[test]" >&2
+    echo "attributes, having absorbed the attribute of the function above or" >&2
+    echo "below it — which is no longer running." >&2
+    exit 1
+fi

src/ffi/connect_attr.rs

@@ -519,6 +519,32 @@ pub unsafe fn sql_get_connect_attr_w<B: Backend>(
                 }
             };
 
+            // The two connection attributes the spec declares `SQLULEN` rather
+            // than `SQLUINTEGER`: `SQL_ATTR_ASYNC_ENABLE` ("A SQLULEN value
+            // that specifies whether a function called with a statement on the
+            // specified connection is executed asynchronously") and
+            // `SQL_ATTR_ODBC_CURSORS` ("An SQLULEN value specifying how the
+            // Driver Manager uses the ODBC cursor library"). Every other
+            // integer-valued connection attribute really is `SQLUINTEGER`, so
+            // this is a two-attribute exception rather than the blanket rule
+            // that applies to statement attributes — where *no* non-pointer
+            // attribute is `SQLUINTEGER`.
+            //
+            // SAFETY: as `write_u32` above, but the buffer is SQLULEN-wide.
+            let write_ulen = |v: usize| {
+                if !value_ptr.is_null() {
+                    // SAFETY: non-null checked above; caller guarantees writable SQLULEN
+                    std::ptr::write_unaligned(value_ptr as *mut usize, v);
+                }
+                if !string_length_ptr.is_null() {
+                    // SAFETY: non-null checked above; caller guarantees writable i32
+                    std::ptr::write_unaligned(
+                        string_length_ptr,
+                        std::mem::size_of::<usize>() as i32,
+                    );
+                }
+            };
+
             match attr {
                 _ if attr == ConnectionAttribute::ACCESS_MODE => {
                     let v = conn
@@ -579,7 +605,7 @@ pub unsafe fn sql_get_connect_attr_w<B: Backend>(
                         .get(&attribute)
                         .copied()
                         .unwrap_or(SQL_CUR_USE_DRIVER);
-                    write_u32(v as u32);
+                    write_ulen(v);
                     Ok(SqlReturn::SUCCESS)
                 }
                 _ if attr == ConnectionAttribute::PACKET_SIZE => {
@@ -611,7 +637,7 @@ pub unsafe fn sql_get_connect_attr_w<B: Backend>(
                         .get(&attribute)
                         .copied()
                         .unwrap_or(SQL_ASYNC_ENABLE_OFF);
-                    write_u32(v as u32);
+                    write_ulen(v);
                     Ok(SqlReturn::SUCCESS)
                 }
                 _ if attr == ConnectionAttribute::TRANSLATE_OPTION => {
@@ -906,6 +932,48 @@ mod tests {
         }
     }
 
+    /// `SQL_ATTR_ASYNC_ENABLE` and `SQL_ATTR_ODBC_CURSORS` are the two
+    /// connection attributes the spec declares `SQLULEN` — "A SQLULEN value
+    /// that specifies whether a function called with a statement on the
+    /// specified connection is executed asynchronously" and "An SQLULEN value
+    /// specifying how the Driver Manager uses the ODBC cursor library". Every
+    /// other integer-valued connection attribute is `SQLUINTEGER`, which is why
+    /// this is a two-attribute exception here and a blanket rule for statement
+    /// attributes.
+    #[test]
+    fn the_two_sqlulen_connection_attributes_are_written_at_full_width() {
+        for (attribute, name, expected) in [
+            (
+                ConnectionAttribute::ODBC_CURSORS.0,
+                "SQL_ATTR_ODBC_CURSORS",
+                SQL_CUR_USE_DRIVER,
+            ),
+            (
+                ConnectionAttribute::ASYNC_ENABLE.0,
+                "SQL_ATTR_ASYNC_ENABLE",
+                SQL_ASYNC_ENABLE_OFF,
+            ),
+        ] {
+            unsafe {
+                let (env, conn) = alloc_env_conn();
+                let mut value: usize = usize::MAX;
+                let ret = sql_get_connect_attr_w::<MockBackend>(
+                    conn,
+                    attribute,
+                    std::ptr::from_mut(&mut value).cast(),
+                    0,
+                    std::ptr::null_mut(),
+                );
+                assert_eq!(ret, SqlReturn::SUCCESS);
+                assert_eq!(
+                    value, expected,
+                    "{name}: the high half of the SQLULEN buffer kept its poison"
+                );
+                cleanup(env, conn);
+            }
+        }
+    }
+
     /// An attribute `SQLSetConnectAttr` stores is one `SQLGetConnectAttr` can
     /// report: the spec makes the getter the way an application reads back what
     /// it set, so a stored value that answers HY092 would be unreachable.
@@ -969,7 +1037,12 @@ mod tests {
                     SqlReturn::SUCCESS,
                     "{name} was not accepted"
                 );
-                let mut out: u32 = u32::MAX;
+                // An SQLULEN buffer, zeroed: two of these attributes are
+                // SQLULEN and the rest SQLUINTEGER, and the spec tells
+                // applications to "use a buffer of SQLULEN and initialize the
+                // value to 0" precisely so one buffer serves both. A `u32`
+                // here would be overflowed by the SQLULEN pair.
+                let mut out: usize = 0;
                 assert_eq!(
                     sql_get_connect_attr_w::<MockBackend>(
                         conn,
@@ -981,7 +1054,7 @@ mod tests {
                     SqlReturn::SUCCESS,
                     "{name} was stored but cannot be read back"
                 );
-                assert_eq!(out as usize, *value, "{name} read back a different value");
+                assert_eq!(out, *value, "{name} read back a different value");
                 cleanup(env, conn);
             }
         }