openlink / iODBC

An open-source ODBC driver manager and SDK that facilitates the development of database-independent applications on linux, freebsd, unix and MacOS X platforms.
http://www.iodbc.org/
Other
162 stars 52 forks source link

3.52.15 DM truncating SQL_C_WCHAR parameter value when bound w/ BufferLength=0 & length indicator of SQL_NTS #68

Open matthew-wozniczka opened 3 years ago

matthew-wozniczka commented 3 years ago

I'm not sure if passing a bufferlength of 0 to SQLBindParameter for a string parameter while using a length indicator of SQL_NTS is completely kosher, but it used to work (circa 3.52.8, and in other DMs).

I find that the DM seems to try to support it, in _SQLExecute_ConvParams it computes elementSize to be the size in bytes, without the null terminator, and then in _ConvParam -> _ExecConv_W2W it copies the entire value back to the buffer, but then overwrites the last character with a null terminator.

matthew-wozniczka commented 3 years ago

The same thing happens if BufferLength is set exactly to the length of the string in bytes (not including the null terminator).

In this case, we're setting the length indicator to SQL_NTS, and the buffer actually contains a length indicator, but I'm not sure if this is 'correct' (since technically the null terminator is outside of the buffer as described to the DM/Driver), but again, it used to work.

kidehen commented 3 years ago

As indicated by @pkleef this issue has been assigned for fixing. Thus, expect a fix later today or tomorrow.

smalinin commented 3 years ago

@matthew-wozniczka

The same thing happens if BufferLength is set exactly to the length of the string in bytes (not including the null terminator).

In this case, we're setting the length indicator to SQL_NTS, and the buffer actually contains a length indicator, but I'm not sure if this is 'correct' (since technically the null terminator is outside of the buffer as described to the DM/Driver), but again, it used to work.

It is unclear, why BufferLength isn't include null terminator => https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlbindparameter-function?view=sql-server-ver15#bufferlength-argument

For character C data, if the number of bytes available to return is greater than or equal to BufferLength,
 the data in *ParameterValuePtr is truncated to BufferLength less the length of a null-termination character 
and is null-terminated by the driver.
matthew-wozniczka commented 3 years ago

that passage is about output parameters:

For input/output and output parameters, it is used to determine whether to truncate character and binary C data on output

smalinin commented 3 years ago

@matthew-wozniczka Did you use one row binding or Array parameters binding ?

matthew-wozniczka commented 3 years ago

In this case I'm not using parameter arrays (having a bufferlength of 0 definitely would not work there)

smalinin commented 3 years ago

@matthew-wozniczka About call

SQLRETURN SQLBindParameter(  
      SQLHSTMT        StatementHandle,  
      SQLUSMALLINT    ParameterNumber,  
      SQLSMALLINT     InputOutputType,  
      SQLSMALLINT     ValueType,  
      SQLSMALLINT     ParameterType,  
      SQLULEN         ColumnSize,  
      SQLSMALLINT     DecimalDigits,  
      SQLPOINTER      ParameterValuePtr,  
      SQLLEN          BufferLength,  
      SQLLEN *        StrLen_or_IndPtr);  

What kind of value did you use for ColumnSize in your code (for example) ? Is it equal max value len in parameter ?

smalinin commented 3 years ago

What kind of ODBC driver did you use Unicode or ANSI ? Does it support UCS4 codepage ?

smalinin commented 3 years ago

Fixed.

The issue, when BufferLength == 0 was fixed,

About:

The same thing happens if BufferLength is set exactly to the length of the string in bytes (not including the null terminator).

In this case, we're setting the length indicator to SQL_NTS, and the buffer actually contains a length indicator, but I'm not sure if this is 'correct' (since technically the null terminator is outside of the buffer as described to the DM/Driver), but again, it used to work.

It will work ONLY, if Unicode conversion isn't used(Unicode->Ansi or UCS4->UCS2 and etc), but it is better not use such calls, I think.

TallTed commented 3 years ago

Fixed by: https://github.com/openlink/iODBC/commit/0b90ca1436a38af2c377aacbe9887c82ab4f0bad