Fix #44618: Fetching may rely on uninitialized data

[cmb69]

Unless SQLGetData() returns SQL_SUCCESS or SQL_SUCCESS_WITH_INFO,
the StrLen_or_IndPtr output argument is not guaranteed to be properly
set. Thus we handle retrieval failure other than SQL_ERROR by
silently yielding false for those column values.

---

Alternative handling of retrieval failure would be to raise E_WARNING as well, and/or let the whole odbc_fetch_into()/odbc_fetch_array() fail.

I don't really mind how exactly we solve this, but that bug needs to be fixed.

[nikic]

Unless SQLGetData() returns SQL_SUCCESS or SQL_SUCCESS_WITH_INFO,
the StrLen_or_IndPtr output argument is not guaranteed to be properly
set. Thus we handle retrieval failure other than SQL_ERROR by
silently yielding false for those column values.

What is the actual return value we end up handling here?

[cmb69]

What is the actual return value we end up handling here?

In this case, it is SQL_NO_DATA, but SQLGetData() may also return SQL_STILL_EXECUTING, and theoretically SQL_INVALID_HANDLE which have the same issue. And maybe some drivers/driver managers may return other values as well. In any case, these failures do not provide further info in an associated SQLSTATE value.

[nikic]

What is the actual return value we end up handling here?

In this case, it is SQL_NO_DATA, but SQLGetData() may also return SQL_STILL_EXECUTING, and theoretically SQL_INVALID_HANDLE which have the same issue. And maybe some drivers/driver managers may return other values as well. In any case, these failures do not provide further info in an associated SQLSTATE value.

Independently of the problem you're trying to fix here ... why are we getting SQL_NO_DATA? The PHP code looks to be doing something sensible, why do we fail to fetch the text1 column?

[cmb69]

This connection requests use of the (deprecated) ODBC cursor library, which implements SQLGetData() in a limited way. But even when not using the ODBC cursor library, there can be issues with our mixing of SQLBindCol() and SQLGetData() (see https://bugs.php.net/76732#1601901051).

[cmb69]

Any objections to merge this?

[nikic]

My 2c here is that we should treat these cases as close to errors as we can. I understand we can't use odbc_sql_error(), so we should explicitly throw a warning and return false. Otherwise we will return corrupted data, and I doubt that anyone who uses these APIs will have appropriate checks in place to recognize false values in individual columns.

[cmb69]

Good point. Thanks! Maybe the error message can be improved (cc @kocsismate). I've chosen the column numbers to start at 1, because that is the convention for odbc_result() anyway.

[nikic]

Looks reasonable to me!