Fix #80783: PDO ODBC truncates BLOB records at every 256th byte #6716
Conversation
It is not guaranteed, that the driver inserts only a single NUL byte at the end of the buffer. Apparently, there is no way to find out the actual data length in the buffer after calling `SQLGetData()`, so we adjust after the next `SQLGetData()` call.
Just to double check, is the problem really multiple nul bytes, or a missing nul terminator? That's how the trace reads to me at least. For reference, from https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgetdata-function?view=sql-server-ver15:
So C->fetched_len is the total (remaining) length of the data, not the length of the chunk. Really not obvious reading the code. |
With the test and SQLServer, there have actually been two NUL bytes at the end (and only the first 254 bytes of the buffer where actually part of the column). But indeed, the provided ODBC traces indicate that there is no trailing NUL byte. I'm not sure whether that is a driver glitch, or fully conforming to the spec. Anyhow, I think we need to cater to that as well. OTOH, I do not understand php-src/ext/pdo_odbc/odbc_stmt.c Lines 678 to 680 in 963e50c This comment has replaced a single fetch in 10251b2. Why would chunked fetching be faster than a single fetch? |
Codecov Report
@@ Coverage Diff @@
## PHP-7.4 #6716 +/- ##
===========================================
- Coverage 72.99% 72.99% -0.01%
===========================================
Files 803 803
Lines 283735 283798 +63
===========================================
+ Hits 207114 207153 +39
- Misses 76621 76645 +24
Continue to review full report at Codecov.
|
|
I did some more testing. SQLServer with VARBINARY(MAX) column fetches 254 valid bytes (2 trailing NULs) as well. PostgreSQL with BYTEA column fetches 255 valid bytes (1 trailing NUL). MySQL with VARBINARY(4000) fetches 255 valid bytes (1 trailing NUL), but returns SQL_SUCCESS, so only a single SQLGetData() is executed; the rest of the string is arbitrary garbage. One might think of "all these broken driver implementations", but it seems to me the real problem is that we fetch binary data as SQL_C_CHAR. Fetching as SQL_C_BINARY would yield 256 valid bytes (no trailing NUL) for all three drivers. However, changing this would be a massive BC break (results would no longer be bin2hex() encoded, but plain binary data). Well, except we would fetch as binary, and do the bin2hex() conversion ourselves. I'm unsure … thoughts? |
This is actually already catered to, but we need to adjust the assertion.
This is actually an own issue, but can't be fixed without fixing #80783 first, so I add this to this PR. The point is that we need to fetch all chunks with the same C type.
I haven't done any performance tests regarding this issue, but it doesn't really matter, because a single fetch is not really possible (would need to determine the proper length in advance, what is a headache; cf. odbc_bindcols()), so we need at least two SQLGetData() calls, and having possibly more doesn't really hurt. |
|
|
If there are no objections, I'll merge this by the end of the week. |
|
|
`` |
It is not guaranteed, that the driver inserts only a single NUL byte at
the end of the buffer. Apparently, there is no way to find out the
actual data length in the buffer after calling
SQLGetData(), so weadjust after the next
SQLGetData()call.Note that the output of the IMAGE column looks fishy (it is bin2hex() encoded), but that would be different issue.