Skip to content

Commit 36c9bed

Browse files
authored
FIX: Handling Access token with class level variables and removing static defination (#323)
### Work Item / Issue Reference <!-- mssql-python maintainers: ADO Work Item --> > [AB#40420](https://sqlclientdrivers.visualstudio.com/c6d89619-62de-46a0-8b46-70b92a84d85e/_workitems/edit/40420) ------------------------------------------------------------------- ### Summary This pull request refactors the way temporary buffers are managed for setting connection attributes in the `Connection` class, improving memory safety and simplifying buffer handling. Instead of using static vectors to store temporary string and binary data, the code now uses member variables to hold these buffers directly within each `Connection` instance. **Buffer Management Refactor:** * Removed static vectors (`wstr_buffers`, `buffers`) for storing temporary wide string and binary buffers, eliminating the need for manual buffer growth management and cleanup. [[1]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL211-L212) [[2]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL221-R219) [[3]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL269-R262) * Added member variables `wstrStringBuffer` and `strBytesBuffer` to the `Connection` class for managing temporary buffers per connection instance, improving encapsulation and memory safety. * Updated all relevant buffer usage in `setAttribute` to use these new member variables, replacing references to static buffers with instance variables. [[1]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL248-R237) [[2]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL221-R219) [[3]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL269-R262) **Code Simplification:** * Removed unnecessary static buffer and related logic, such as buffer size limits and periodic cleanup, resulting in cleaner and more maintainable code. [[1]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL191) [[2]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL221-R219) [[3]](diffhunk://#diff-eca696c13d997f510e5f9b16288ed1deb0ad132768c283eda1518f78edf9b6ecL269-R262)
1 parent aae194c commit 36c9bed

File tree

2 files changed

+12
-32
lines changed

2 files changed

+12
-32
lines changed

mssql_python/pybind/connection/connection.cpp

Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,6 @@ SQLRETURN Connection::setAttribute(SQLINTEGER attribute, py::object value) {
188188
LOG("Setting SQL attribute");
189189
// SQLPOINTER ptr = nullptr;
190190
// SQLINTEGER length = 0;
191-
static std::string buffer; // to hold sensitive data temporarily
192191

193192
if (py::isinstance<py::int_>(value)) {
194193
// Get the integer value
@@ -208,8 +207,6 @@ SQLRETURN Connection::setAttribute(SQLINTEGER attribute, py::object value) {
208207
return ret;
209208
} else if (py::isinstance<py::str>(value)) {
210209
try {
211-
// Keep buffers alive
212-
static std::vector<std::wstring> wstr_buffers;
213210
std::string utf8_str = value.cast<std::string>();
214211

215212
// Convert to wide string
@@ -218,24 +215,16 @@ SQLRETURN Connection::setAttribute(SQLINTEGER attribute, py::object value) {
218215
LOG("Failed to convert string value to wide string");
219216
return SQL_ERROR;
220217
}
221-
222-
// Limit static buffer growth for memory safety
223-
constexpr size_t MAX_BUFFER_COUNT = 100;
224-
if (wstr_buffers.size() >= MAX_BUFFER_COUNT) {
225-
// Remove oldest 50% of entries when limit reached
226-
wstr_buffers.erase(wstr_buffers.begin(),
227-
wstr_buffers.begin() + (MAX_BUFFER_COUNT / 2));
228-
}
229-
230-
wstr_buffers.push_back(wstr);
218+
this->wstrStringBuffer.clear();
219+
this->wstrStringBuffer = std::move(wstr);
231220

232221
SQLPOINTER ptr;
233222
SQLINTEGER length;
234223

235224
#if defined(__APPLE__) || defined(__linux__)
236225
// For macOS/Linux, convert wstring to SQLWCHAR buffer
237-
std::vector<SQLWCHAR> sqlwcharBuffer = WStringToSQLWCHAR(wstr);
238-
if (sqlwcharBuffer.empty() && !wstr.empty()) {
226+
std::vector<SQLWCHAR> sqlwcharBuffer = WStringToSQLWCHAR(this->wstrStringBuffer);
227+
if (sqlwcharBuffer.empty() && !this->wstrStringBuffer.empty()) {
239228
LOG("Failed to convert wide string to SQLWCHAR buffer");
240229
return SQL_ERROR;
241230
}
@@ -245,9 +234,8 @@ SQLRETURN Connection::setAttribute(SQLINTEGER attribute, py::object value) {
245234
sqlwcharBuffer.size() * sizeof(SQLWCHAR));
246235
#else
247236
// On Windows, wchar_t and SQLWCHAR are the same size
248-
ptr = const_cast<SQLWCHAR*>(wstr_buffers.back().c_str());
249-
length = static_cast<SQLINTEGER>(
250-
wstr.length() * sizeof(SQLWCHAR));
237+
ptr = const_cast<SQLWCHAR*>(this->wstrStringBuffer.c_str());
238+
length = static_cast<SQLINTEGER>(this->wstrStringBuffer.length() * sizeof(SQLWCHAR));
251239
#endif
252240

253241
SQLRETURN ret = SQLSetConnectAttr_ptr(_dbcHandle->get(),
@@ -266,21 +254,11 @@ SQLRETURN Connection::setAttribute(SQLINTEGER attribute, py::object value) {
266254
} else if (py::isinstance<py::bytes>(value) ||
267255
py::isinstance<py::bytearray>(value)) {
268256
try {
269-
static std::vector<std::string> buffers;
270257
std::string binary_data = value.cast<std::string>();
271-
272-
// Limit static buffer growth
273-
constexpr size_t MAX_BUFFER_COUNT = 100;
274-
if (buffers.size() >= MAX_BUFFER_COUNT) {
275-
// Remove oldest 50% of entries when limit reached
276-
buffers.erase(buffers.begin(),
277-
buffers.begin() + (MAX_BUFFER_COUNT / 2));
278-
}
279-
280-
buffers.emplace_back(std::move(binary_data));
281-
SQLPOINTER ptr = const_cast<char*>(buffers.back().c_str());
282-
SQLINTEGER length = static_cast<SQLINTEGER>(
283-
buffers.back().size());
258+
this->strBytesBuffer.clear();
259+
this->strBytesBuffer = std::move(binary_data);
260+
SQLPOINTER ptr = const_cast<char*>(this->strBytesBuffer.c_str());
261+
SQLINTEGER length = static_cast<SQLINTEGER>(this->strBytesBuffer.size());
284262

285263
SQLRETURN ret = SQLSetConnectAttr_ptr(_dbcHandle->get(),
286264
attribute, ptr, length);

mssql_python/pybind/connection/connection.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ class Connection {
5959
bool _autocommit = true;
6060
SqlHandlePtr _dbcHandle;
6161
std::chrono::steady_clock::time_point _lastUsed;
62+
std::wstring wstrStringBuffer; // wstr buffer for string attribute setting
63+
std::string strBytesBuffer; // string buffer for byte attributes setting
6264
};
6365

6466
class ConnectionHandle {

0 commit comments

Comments
 (0)