FIX: Access Token Handling, Shutdown Safety, and Connection Attribute Tests

[bewithgaurav]

Work Item / Issue Reference

AB#39047

GitHub Issue: #<ISSUE_NUMBER>

---

Summary

This pull request introduces important safety and robustness improvements to the handling of ODBC connection attributes, particularly focusing on access token authentication and defensive programming against known driver bugs. It also adds comprehensive test coverage for these changes, ensuring correct behavior and memory safety in various edge cases.

Defensive fixes and error handling

• Added a defensive check in Connection::setAttribute (C++), which blocks access tokens shorter than 32 bytes to prevent a known crash in Microsoft ODBC Driver 18. An informative error is raised if a short token is detected.
• Improved resource management in SqlHandle::~SqlHandle to avoid calling SQLFreeHandle during Python interpreter shutdown, preventing potential crashes or undefined behavior.

Test coverage for connection attributes

• Added a comprehensive suite of tests in tests/test_003_connection.py to verify the behavior of the attrs_before parameter, including handling of supported and unsupported attribute types, invalid keys, empty dictionaries, multiple attributes, memory safety with binary data, and compatibility with connection pooling and autocommit.
• Added targeted tests in tests/test_008_auth.py to verify the defensive fix for short access tokens, ensuring that tokens under 32 bytes are blocked and legitimate tokens are allowed (with expected authentication errors if invalid). These tests run in subprocesses to isolate potential segfaults.

Minor test cleanups

• Minor formatting and whitespace cleanups in threading and connection lifecycle tests to improve readability and maintainability. [1] [2]

[Copilot]

Pull Request Overview

This PR adds comprehensive test coverage for custom connection attributes, focusing on the attrs_before parameter functionality and implementing defensive fixes for ODBC driver segfaults.

• Adds extensive test coverage for the attrs_before parameter with various data types and edge cases
• Implements defensive protection against ODBC driver crashes with short access tokens (< 32 bytes)
• Adds tests for Python shutdown scenarios that previously caused segfaults with SQL syntax errors

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/test_008_auth.py	Adds test for access token length validation to prevent ODBC driver crashes
tests/test_005_connection_cursor_lifecycle.py	Adds comprehensive tests for cursor cleanup scenarios during Python shutdown
tests/test_003_connection.py	Adds extensive test coverage for attrs_before parameter with various data types and configurations
mssql_python/pybind/ddbc_bindings.cpp	Implements Python shutdown detection and safe cleanup to prevent segfaults
mssql_python/pybind/connection/connection.cpp	Adds access token length validation and secure memory handling for sensitive data

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The comment contains a grammatical error. Should be 'This would NOT print anyway' (without the 's' in 'anyways').

Suggested change

	print("Script completed, shutting down...") # This would NOT print anyways
	print("Script completed, shutting down...") # This would NOT print anyway

[Copilot]

The error message is too generic and doesn't provide enough context about what specific error occurred. Consider logging the actual exception details or being more specific about the type of error.

Suggested change

	} catch (...) {
	std::cerr << "Error occurred while checking Python finalization state." << std::endl;
	} catch (const std::exception& e) {
	std::cerr << "Error occurred while checking Python finalization state: " << e.what() << std::endl;
	// Be conservative - don't assume shutdown on any exception
	// Only return true if we're absolutely certain Python is shutting down
	return false;
	} catch (...) {
	std::cerr << "Unknown error occurred while checking Python finalization state." << std::endl;

[Copilot]

The function returns SQL_ERROR but doesn't set any specific error message that can be retrieved by the caller. This makes debugging difficult. Consider setting a more descriptive error state or returning a custom error code.

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 100%

🎯 Overall Coverage 75%

📈 Total Lines Covered: 4220 out of 5613 📁 Project: mssql-python

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

• mssql_python/pybind/connection/connection.cpp (100%)
• mssql_python/pybind/ddbc_bindings.cpp (100%)

Summary

• Total: 8 lines
• Missing: 0 lines
• Coverage: 100%

---

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.ddbc_bindings.py: 68.5%
mssql_python.pybind.ddbc_bindings.cpp: 69.4%
mssql_python.pybind.connection.connection_pool.cpp: 78.9%
mssql_python.cursor.py: 79.6%
mssql_python.pybind.connection.connection.cpp: 80.1%
mssql_python.connection.py: 81.7%
mssql_python.helpers.py: 84.7%
mssql_python.auth.py: 85.3%
mssql_python.type.py: 86.8%
mssql_python.pooling.py: 88.8%

---

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report