Skip to content

Commit 41e4883

Browse files
jahnvi480jahnvi480
committed
Resolving comments
1 parent 2844c23 commit 41e4883

File tree

2 files changed

+48
-38
lines changed

2 files changed

+48
-38
lines changed

‎mssql_python/pybind/ddbc_bindings.cpp‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ static py::str DecodingString(const char* data, size_t length,
205205
const std::string& errors = "strict") {
206206
try {
207207
py::gil_scoped_acquire gil;
208-
py::bytes byte_data = py::bytes(std::string(data, length));
208+
py::bytes byte_data = py::bytes(data, length);
209209

210210
// Direct decoding - let Python handle errors strictly
211211
py::str decoded = byte_data.attr("decode")(encoding, errors);
@@ -410,28 +410,37 @@ SQLRETURN BindParameters(SQLHANDLE hStmt, const py::list& params,
410410
std::string strValue;
411411

412412
// Check if we have encoding settings and this is SQL_C_CHAR (not SQL_C_WCHAR)
413-
if (encoding_settings && !encoding_settings.is_none() &&
414-
encoding_settings.contains("ctype") &&
415-
encoding_settings.contains("encoding")) {
416-
417-
SQLSMALLINT ctype = encoding_settings["ctype"].cast<SQLSMALLINT>();
418-
419-
// Only use dynamic encoding for SQL_C_CHAR, keep SQL_C_WCHAR unchanged
420-
if (ctype == SQL_C_CHAR) {
421-
try {
422-
py::dict settings_dict = encoding_settings.cast<py::dict>();
423-
auto [encoding, errors] = extract_encoding_settings(settings_dict);
424-
425-
// Use our safe encoding function
426-
py::bytes encoded_bytes = EncodingString(param.cast<std::string>(), encoding, errors);
427-
strValue = encoded_bytes.cast<std::string>();
413+
if (encoding_settings && !encoding_settings.is_none()) {
414+
try {
415+
// SECURITY: Use extract_encoding_settings for full validation
416+
// This validates encoding against allowlist and error mode
417+
py::dict settings_dict = encoding_settings.cast<py::dict>();
418+
auto [encoding, errors] = extract_encoding_settings(settings_dict);
419+
420+
// Validate ctype against allowlist
421+
if (settings_dict.contains("ctype")) {
422+
SQLSMALLINT ctype = settings_dict["ctype"].cast<SQLSMALLINT>();
428423

429-
} catch (const std::exception& e) {
430-
LOG("Encoding failed for parameter {}: {}", paramIndex, e.what());
431-
ThrowStdException("Failed to encode parameter " + std::to_string(paramIndex) + ": " + e.what());
424+
// Only SQL_C_CHAR and SQL_C_WCHAR are allowed
425+
if (ctype != SQL_C_CHAR && ctype != SQL_C_WCHAR) {
426+
LOG("Invalid ctype {} for parameter {}, using default", ctype, paramIndex);
427+
// Fall through to default behavior
428+
strValue = param.cast<std::string>();
429+
} else if (ctype == SQL_C_CHAR) {
430+
// Only use dynamic encoding for SQL_C_CHAR
431+
py::bytes encoded_bytes = EncodingString(param.cast<std::string>(), encoding, errors);
432+
strValue = encoded_bytes.cast<std::string>();
433+
} else {
434+
// SQL_C_WCHAR - use default behavior
435+
strValue = param.cast<std::string>();
436+
}
437+
} else {
438+
// No ctype specified, use default behavior
439+
strValue = param.cast<std::string>();
432440
}
433-
} else {
434-
// Default behavior for other types
441+
} catch (const std::exception& e) {
442+
LOG("Encoding settings processing failed for parameter {}: {}. Using default.", paramIndex, e.what());
443+
// Fall back to safe default behavior
435444
strValue = param.cast<std::string>();
436445
}
437446
} else {

‎tests/test_011_encoding_decoding.py‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3170,9 +3170,9 @@ def test_gbk_encoding_chinese_simplified(db_connection):
31703170
cursor.execute("INSERT INTO #test_gbk VALUES (?, ?)", 1, chinese_text)
31713171
cursor.execute("SELECT data FROM #test_gbk WHERE id = 1")
31723172
result = cursor.fetchone()
3173-
print(f" Testing '{chinese_text}' ({meaning}): {safe_display(result[0])}")
3173+
print(f" Testing {chinese_text!r} ({meaning}): {safe_display(result[0])}")
31743174
else:
3175-
print(f" Skipping '{chinese_text}' (not GBK compatible)")
3175+
print(f" Skipping {chinese_text!r} (not GBK compatible)")
31763176

31773177
print("="*60)
31783178

@@ -3204,9 +3204,9 @@ def test_big5_encoding_chinese_traditional(db_connection):
32043204
cursor.execute("INSERT INTO #test_big5 VALUES (?, ?)", 1, chinese_text)
32053205
cursor.execute("SELECT data FROM #test_big5 WHERE id = 1")
32063206
result = cursor.fetchone()
3207-
print(f" Testing '{chinese_text}' ({meaning}): {safe_display(result[0])}")
3207+
print(f" Testing {chinese_text!r} ({meaning}): {safe_display(result[0])}")
32083208
else:
3209-
print(f" Skipping '{chinese_text}' (not Big5 compatible)")
3209+
print(f" Skipping {chinese_text!r} (not Big5 compatible)")
32103210

32113211
print("="*60)
32123212

@@ -3238,9 +3238,9 @@ def test_shift_jis_encoding_japanese(db_connection):
32383238
cursor.execute("INSERT INTO #test_sjis VALUES (?, ?)", 1, japanese_text)
32393239
cursor.execute("SELECT data FROM #test_sjis WHERE id = 1")
32403240
result = cursor.fetchone()
3241-
print(f" Testing '{japanese_text}' ({meaning}): {safe_display(result[0])}")
3241+
print(f" Testing {japanese_text!r} ({meaning}): {safe_display(result[0])}")
32423242
else:
3243-
print(f" Skipping '{japanese_text}' (not Shift-JIS compatible)")
3243+
print(f" Skipping {japanese_text!r} (not Shift-JIS compatible)")
32443244

32453245
print("="*60)
32463246

@@ -3273,9 +3273,9 @@ def test_euc_kr_encoding_korean(db_connection):
32733273
cursor.execute("INSERT INTO #test_euckr VALUES (?, ?)", 1, korean_text)
32743274
cursor.execute("SELECT data FROM #test_euckr WHERE id = 1")
32753275
result = cursor.fetchone()
3276-
print(f" Testing '{korean_text}' ({meaning}): {safe_display(result[0])}")
3276+
print(f" Testing {korean_text!r} ({meaning}): {safe_display(result[0])}")
32773277
else:
3278-
print(f" Skipping '{korean_text}' (not EUC-KR compatible)")
3278+
print(f" Skipping {korean_text!r} (not EUC-KR compatible)")
32793279

32803280
print("="*60)
32813281

@@ -3315,10 +3315,10 @@ def test_latin1_encoding_western_european(db_connection):
33153315
cursor.execute("INSERT INTO #test_latin1 VALUES (?, ?)", 1, text)
33163316
cursor.execute("SELECT data FROM #test_latin1 WHERE id = 1")
33173317
result = cursor.fetchone()
3318-
match = "✓" if result[0] == text else "✗"
3319-
print(f" {match} {description:15} | '{text}' -> '{result[0]}'")
3318+
match = "PASS" if result[0] == text else "FAIL"
3319+
print(f" {match} {description:15} | {text!r} -> {result[0]!r}")
33203320
else:
3321-
print(f" ✗ {description:15} | Not Latin-1 compatible")
3321+
print(f" SKIP {description:15} | Not Latin-1 compatible")
33223322

33233323
print("="*60)
33243324

@@ -3353,10 +3353,10 @@ def test_cp1252_encoding_windows_western(db_connection):
33533353
cursor.execute("INSERT INTO #test_cp1252 VALUES (?, ?)", 1, text)
33543354
cursor.execute("SELECT data FROM #test_cp1252 WHERE id = 1")
33553355
result = cursor.fetchone()
3356-
match = "✓" if result[0] == text else "✗"
3357-
print(f" {match} {description:15} | '{text}' -> '{result[0]}'")
3356+
match = "PASS" if result[0] == text else "FAIL"
3357+
print(f" {match} {description:15} | {text!r} -> {result[0]!r}")
33583358
else:
3359-
print(f" ✗ {description:15} | Not CP1252 compatible")
3359+
print(f" SKIP {description:15} | Not CP1252 compatible")
33603360

33613361
print("="*60)
33623362

@@ -3504,8 +3504,9 @@ def test_utf16_unicode_preservation(db_connection):
35043504
cursor.execute("INSERT INTO #test_utf16 VALUES (?, ?)", 1, text)
35053505
cursor.execute("SELECT data FROM #test_utf16 WHERE id = 1")
35063506
result = cursor.fetchone()
3507-
match = "✓" if result[0] == text else "✗"
3508-
print(f" {match} {description:10} | '{text}' -> '{result[0]}'")
3507+
match = "PASS" if result[0] == text else "FAIL"
3508+
# Use repr() to avoid console encoding issues on Windows
3509+
print(f" {match} {description:10} | {text!r} -> {result[0]!r}")
35093510
assert result[0] == text, f"UTF-16 should preserve {description}"
35103511

35113512
print("="*60)
@@ -3540,7 +3541,7 @@ def test_encoding_error_strict_mode(db_connection):
35403541
print("="*60)
35413542

35423543
for text, description in non_ascii_strings:
3543-
print(f"\nTesting ASCII encoding with '{text}' ({description})...")
3544+
print(f"\nTesting ASCII encoding with {description!r}...")
35443545
try:
35453546
cursor.execute("INSERT INTO #test_strict VALUES (?, ?)", 1, text)
35463547
cursor.execute("SELECT data FROM #test_strict WHERE id = 1")

0 commit comments

Comments
 (0)