Skip to content

Commit 09b934a

Browse files
committed
Trim PR #289 to ASAN/Valgrind CI scope
Revert the MainUnicode.cpp changes (832d8e7, 0f8f90f, bea6403) per irodushka's plan in #289 comment 4279111183 — the Unicode rewrite is moving to its own PR, preceded by a widechar-tests PR. Restore continue-on-error on the ASAN matrix job temporarily; Valgrind stays strict. ASAN will flag the heap-buffer-overflow in SQLGetDiagRecW again; this is the known issue the follow-up PRs will fix and is tracked as issue #287 Tier 1b.
1 parent bea6403 commit 09b934a

2 files changed

Lines changed: 14 additions & 60 deletions

File tree

.github/workflows/build-and-test.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,12 @@ jobs:
6565

6666
runs-on: ${{ matrix.os }}
6767

68+
# Temporary: the ASAN job is expected to fail on the heap-buffer-overflow
69+
# inside SQLGetDiagRecW until the Linux widechar conversion paths in
70+
# MainUnicode.cpp are rewritten. Tracked in issue #287 Tier 1b. Revert
71+
# this back to strict once the Unicode fix PR lands.
72+
continue-on-error: ${{ matrix.sanitizer == 'Asan' }}
73+
6874
steps:
6975
- uses: actions/checkout@v6
7076
with:

MainUnicode.cpp

Lines changed: 8 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,6 @@
3838
extern FILE *logFile;
3939
using namespace OdbcJdbcLibrary;
4040

41-
#ifndef _WINDOWS
42-
// SQLWCHAR-aware length (in SQLWCHAR units), safe on Linux where
43-
// sizeof(wchar_t) != sizeof(SQLWCHAR). Do NOT use wcslen() on SQLWCHAR
44-
// data on Linux — it reads two SQLWCHARs per wchar_t and runs off the end.
45-
static size_t sqlwcharLen( const SQLWCHAR *s )
46-
{
47-
size_t n = 0;
48-
if ( !s )
49-
return 0;
50-
while ( s[n] )
51-
++n;
52-
return n;
53-
}
54-
#endif
55-
5641
#ifdef _WINDOWS
5742
extern UINT codePage; // from Main.cpp
5843
#endif
@@ -100,7 +85,7 @@ class ConvertingString
10085
if ( length == SQL_NTS )
10186
lengthString = 0;
10287
else if ( retCountOfBytes )
103-
lengthString = length / sizeof(SQLWCHAR);
88+
lengthString = length / sizeof(wchar_t);
10489
else
10590
lengthString = length;
10691
}
@@ -150,33 +135,13 @@ class ConvertingString
150135
if ( len > 0 )
151136
len--;
152137
#else
153-
// SQLWCHAR is 2 bytes on Linux (unixODBC defines it as unsigned short),
154-
// but wchar_t is 4 bytes, so mbstowcs((wchar_t*)unicodeString, ...)
155-
// both corrupts the output and risks overflowing the caller's buffer.
156-
// Widen byte-by-byte into SQLWCHAR units, matching what unixODBC's
157-
// ansi_to_unicode_copy() does internally. This is correct for the
158-
// ASCII-only error/state strings that reach this code path; non-ASCII
159-
// input will be handled by the broader ConvertingString rewrite tracked
160-
// in issue #287 (Tier 9.1).
161-
{
162-
const SQLCHAR *src = byteString;
163-
size_t i = 0;
164-
while ( i < (size_t)lengthString && src[i] != 0 )
165-
{
166-
unicodeString[i] = (SQLWCHAR)( src[i] & 0xFF );
167-
++i;
168-
}
169-
len = i;
170-
}
138+
len = mbstowcs( (wchar_t*)unicodeString, (const char*)byteString, lengthString );
171139
#endif
172140
}
173141

174142
if ( len > 0 )
175143
{
176-
// NUL-terminate in SQLWCHAR units. LPWSTR assignment of L'\0' writes
177-
// sizeof(wchar_t) bytes, which overruns the output buffer by 2 bytes
178-
// on Linux.
179-
unicodeString[len] = 0;
144+
*(LPWSTR)(unicodeString + len) = L'\0';
180145

181146
if ( realLength )
182147
{
@@ -205,18 +170,12 @@ class ConvertingString
205170
wchar_t saveWC;
206171

207172
if ( length == SQL_NTS )
208-
#ifdef _WINDOWS
209173
length = (int)wcslen( (const wchar_t*)wcString );
210-
#else
211-
length = (int)sqlwcharLen( wcString );
212-
#endif
213-
else if ( wcString[length] != 0 )
174+
else if ( wcString[length] != L'\0' )
214175
{
215176
ptEndWC = (wchar_t*)&wcString[length];
216177
saveWC = *ptEndWC;
217-
// Write a SQLWCHAR-sized NUL so we don't overrun the input by 2 bytes
218-
// on Linux (wchar_t is 4 bytes there).
219-
wcString[length] = 0;
178+
*ptEndWC = L'\0';
220179
}
221180

222181
if ( connection )
@@ -226,10 +185,7 @@ class ConvertingString
226185
#ifdef _WINDOWS
227186
bytesNeeded = WideCharToMultiByte( codePage, (DWORD)0, wcString, length, NULL, (int)0, NULL, NULL );
228187
#else
229-
// See the symmetric comment in the destructor above: wcstombs assumes
230-
// wchar_t-sized input, which corrupts SQLWCHAR data on Linux. The
231-
// byte-narrowing loop below produces exactly `length` output bytes.
232-
bytesNeeded = (size_t)length;
188+
bytesNeeded = wcstombs( NULL, (const wchar_t*)wcString, length );
233189
#endif
234190
}
235191

@@ -242,15 +198,7 @@ class ConvertingString
242198
#ifdef _WINDOWS
243199
bytesNeeded = WideCharToMultiByte( codePage, 0, wcString, length, (LPSTR)byteString, (int)bytesNeeded, NULL, NULL );
244200
#else
245-
{
246-
size_t i = 0;
247-
while ( i < (size_t)length && wcString[i] != 0 )
248-
{
249-
byteString[i] = (SQLCHAR)( wcString[i] & 0xFF );
250-
++i;
251-
}
252-
bytesNeeded = i;
253-
}
201+
bytesNeeded = wcstombs( (char *)byteString, (const wchar_t*)wcString, bytesNeeded );
254202
#endif
255203
}
256204

@@ -272,7 +220,7 @@ class ConvertingString
272220
if ( lengthString )
273221
{
274222
byteString = new SQLCHAR[ lengthString + 2 ];
275-
memset( byteString, 0, lengthString + 2 );
223+
memset(byteString, 0, lengthString + 2);
276224
}
277225
else
278226
byteString = NULL;

0 commit comments

Comments
 (0)