Fix #44618: Fetching may rely on uninitialized data#6281
Fix #44618: Fetching may rely on uninitialized data#6281cmb69 wants to merge 3 commits intophp:PHP-7.3from
Conversation
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? |
In this case, it is |
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? |
|
This connection requests use of the (deprecated) ODBC cursor library, which implements |
|
Any objections to merge this? |
|
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 |
|
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 |
Unless
SQLGetData()returnsSQL_SUCCESSorSQL_SUCCESS_WITH_INFO,the
StrLen_or_IndPtroutput argument is not guaranteed to be properlyset. Thus we handle retrieval failure other than
SQL_ERRORbysilently yielding
falsefor those column values.Alternative handling of retrieval failure would be to raise
E_WARNINGas well, and/or let the wholeodbc_fetch_into()/odbc_fetch_array()fail.I don't really mind how exactly we solve this, but that bug needs to be fixed.