Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion mssql_python/pybind/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -370,10 +370,13 @@ if(MSVC)
endif()

# Add warning flags for GCC/Clang on Linux and macOS
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR
CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
target_compile_options(ddbc_bindings PRIVATE
-Werror # Treat warnings as errors
-Wattributes # Enable attribute warnings (cross-compiler)
-Wformat=2 # Check nonliteral formats and printf argument types
-Werror=format-security # Explicitly keep format-security diagnostics fatal
)

# GCC-specific warning flags
Expand Down
19 changes: 10 additions & 9 deletions mssql_python/pybind/ddbc_bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2746,8 +2746,9 @@ SQLRETURN BindParameterArray(SqlHandle& handle, SQLHANDLE hStmt, const py::list&
if (PyBytes_GET_SIZE(b.ptr()) != 16) {
LOG("BindParameterArray: GUID bytes wrong "
"length - param_index=%d, row=%zu, "
"length=%d",
paramIndex, i, PyBytes_GET_SIZE(b.ptr()));
"length=%lld",
paramIndex, i,
static_cast<long long>(PyBytes_GET_SIZE(b.ptr())));
ThrowStdException("UUID binary data must be "
"exactly 16 bytes long.");
}
Expand All @@ -2774,8 +2775,8 @@ SQLRETURN BindParameterArray(SqlHandle& handle, SQLHANDLE hStmt, const py::list&
strLenOrIndArray[i] = sizeof(SQLGUID);
}
LOG("BindParameterArray: SQL_C_GUID bound - "
"param_index=%d, null=%zu, bytes=%zu, uuid_obj=%zu",
paramIndex);
"param_index=%d, count=%zu",
paramIndex, paramSetSize);
dataPtr = guidArray;
bufferLength = sizeof(SQLGUID);
break;
Expand Down Expand Up @@ -4357,7 +4358,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum
}
if (dataLen == SQL_NO_TOTAL) {
LOG("Cannot determine the length of the data. Returning NULL "
"value instead. Column ID - {}",
"value instead. Column ID - %d",
col);
Py_INCREF(Py_None);
PyList_SET_ITEM(row, col - 1, Py_None);
Expand All @@ -4383,7 +4384,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum
if (dataLen == 0) {
// Handle zero-length (non-NULL) data for complex types
LOG("Column data length is 0 for complex datatype. Setting "
"None to the result row. Column ID - {}",
"None to the result row. Column ID - %d",
col);
Py_INCREF(Py_None);
PyList_SET_ITEM(row, col - 1, Py_None);
Expand Down Expand Up @@ -4418,7 +4419,7 @@ SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, py::list& colum
} catch (const py::error_already_set& e) {
// Handle the exception, e.g., log the error and set
// py::none()
LOG("Error converting to decimal: {}", e.what());
LOG("Error converting to decimal: %s", e.what());
Py_INCREF(Py_None);
PyList_SET_ITEM(row, col - 1, Py_None);
}
Expand Down Expand Up @@ -5020,7 +5021,7 @@ SQLRETURN FetchArrowBatch_wrap(SqlHandlePtr StatementHandle, py::list& capsules,
errorString << "Unsupported data type for Arrow batch fetch for column - "
<< columnName.c_str() << ", Type - " << dataType << ", column ID - "
<< (i + 1);
LOG(errorString.str().c_str());
LOG("FetchArrowBatch: %s", errorString.str().c_str());
ThrowStdException(errorString.str());
break;
}
Expand Down Expand Up @@ -5545,7 +5546,7 @@ SQLRETURN FetchArrowBatch_wrap(SqlHandlePtr StatementHandle, py::list& capsules,
std::ostringstream errorString;
errorString << "Unsupported data type for column ID - " << (idxCol + 1)
<< ", Type - " << dataType;
LOG(errorString.str().c_str());
LOG("FetchArrowBatch: %s", errorString.str().c_str());
ThrowStdException(errorString.str());
break;
}
Expand Down
15 changes: 13 additions & 2 deletions mssql_python/pybind/logger_bridge.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ namespace py = pybind11;
namespace mssql_python {
namespace logging {

#if defined(__GNUC__) || defined(__clang__)
#define MSSQL_PRINTF_FORMAT(format_index, first_argument) \
__attribute__((format(printf, format_index, first_argument))) // DevSkim: ignore DS154189
#else
#define MSSQL_PRINTF_FORMAT(format_index, first_argument)
#endif

// Log level constants (matching Python levels)
// Note: Avoid using ERROR as it conflicts with Windows.h macro
const int LOG_LEVEL_DEBUG = 10; // Debug/diagnostic logging
Expand Down Expand Up @@ -81,7 +88,8 @@ class LoggerBridge {
* @param format Printf-style format string
* @param ... Variable arguments for format string
*/
static void log(int level, const char* file, int line, const char* format, ...);
static void log(int level, const char* file, int line, const char* format, ...)
MSSQL_PRINTF_FORMAT(4, 5);

/**
* Get the current log level.
Expand Down Expand Up @@ -124,7 +132,8 @@ class LoggerBridge {
* @param args Variable arguments
* @return Formatted string
*/
static std::string formatMessage(const char* format, va_list args);
static std::string formatMessage(const char* format, va_list args)
MSSQL_PRINTF_FORMAT(1, 0);

/**
* Helper to extract filename from full path.
Expand All @@ -138,6 +147,8 @@ class LoggerBridge {
} // namespace logging
} // namespace mssql_python

#undef MSSQL_PRINTF_FORMAT

// Convenience macros for logging
// Single LOG() macro for all diagnostic logging (DEBUG level)

Expand Down
62 changes: 62 additions & 0 deletions tests/test_039_native_logging_format_security.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
"""Regression guards for native printf-style logging."""

import re
from pathlib import Path

import pytest

_PYBIND_DIR = Path(__file__).resolve().parents[1] / "mssql_python" / "pybind"
_LOGGER_HEADER = _PYBIND_DIR / "logger_bridge.hpp"
_CMAKE = _PYBIND_DIR / "CMakeLists.txt"
pytestmark = pytest.mark.skipif(
not _PYBIND_DIR.is_dir(),
reason="requires a source checkout; isolated wheel tests omit the source tree",
)


def _code_without_comments(text):
text = re.sub(
r"/\*.*?\*/",
lambda match: "\n" * match.group().count("\n"),
text,
flags=re.DOTALL,
)
return "\n".join(
"" if line.lstrip().startswith("#define LOG") else re.sub(r"//.*", "", line)
for line in text.splitlines()
)


def test_native_log_calls_use_literal_format_strings():
dynamic_calls = []
pattern = re.compile(r"\bLOG(?:_INFO|_WARNING|_ERROR)?\s*\(\s*(.)")
paths = (
path
for suffix in ("*.cpp", "*.hpp", "*.h")
for path in _PYBIND_DIR.rglob(suffix)
if "build" not in path.relative_to(_PYBIND_DIR).parts
)
for path in paths:
code = _code_without_comments(path.read_text(encoding="utf-8"))
for match in pattern.finditer(code):
if match.group(1) != '"':
line_number = code.count("\n", 0, match.start()) + 1
source_line = code.splitlines()[line_number - 1].strip()
dynamic_calls.append(
f"{path.relative_to(_PYBIND_DIR)}:{line_number}: {source_line}"
)

assert not dynamic_calls, f"native LOG calls must use literal format strings: {dynamic_calls}"


def test_logger_bridge_enables_compile_time_format_checks():
header = _LOGGER_HEADER.read_text(encoding="utf-8")
cmake = _CMAKE.read_text(encoding="utf-8")

assert "__attribute__((format(printf, format_index, first_argument)))" in header
assert "DevSkim: ignore DS154189" in header
assert "MSSQL_PRINTF_FORMAT(4, 5)" in header
assert "MSSQL_PRINTF_FORMAT(1, 0)" in header
assert 'CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang"' in cmake
assert "-Wformat=2" in cmake
assert "-Werror=format-security" in cmake
Loading