diff --git a/include/object_spy_message.h b/include/object_spy_message.h index 44778a34..53aed674 100644 --- a/include/object_spy_message.h +++ b/include/object_spy_message.h @@ -39,6 +39,8 @@ typedef struct { PyObject_HEAD icsSpyMessage msg; bool noExtraDataPtrCleanup; + // Private bound, independent of writable protocol/length/ownership fields. + size_t extraDataCapacity; } spy_message_object; #pragma pack(pop) @@ -47,6 +49,7 @@ typedef struct { PyObject_HEAD icsSpyMessageJ1850 msg; bool noExtraDataPtrCleanup; + size_t extraDataCapacity; } spy_message_j1850_object; #pragma pack(pop) @@ -63,5 +66,8 @@ extern PyTypeObject spy_message_j1850_object_type; #define PySpyMessageJ1850_GetObject(obj) ((spy_message_j1850_object*)obj) bool setup_spy_message_object(PyObject* module); +size_t spy_message_extra_data_length(const icsSpyMessage& msg); +bool spy_message_validate_extra_data(const spy_message_object* obj); +void spy_message_record_received_extra_data(PyObject* obj); #endif // _OBJECT_SPY_MESSAGE_H_ diff --git a/src/methods.cpp b/src/methods.cpp index 1fde87fd..bb9a48eb 100644 --- a/src/methods.cpp +++ b/src/methods.cpp @@ -20,6 +20,13 @@ #include #include +// Own a Python reference while the GIL is held, including on early returns. +struct PyObjectDecref +{ + void operator()(PyObject* object) const { Py_XDECREF(object); } +}; +using PyObjectRef = std::unique_ptr; + // This class allows RAII of the python GIL. This is a C++ replacement of // Py_BEGIN_ALLOW_THREADS / Py_END_ALLOW_THREADS class PyAllowThreads @@ -794,12 +801,12 @@ bool _convertListOrTupleToArray(PyObject* obj, std::vector* results) PyObject* _getPythonModuleObject(const char* module_name, const char* module_object_name) { // Before we do anything, we need to grab the python s_device_settings ctype.Structure. - PyObject* module = PyImport_ImportModule(module_name); + PyObjectRef module(PyImport_ImportModule(module_name)); if (!module) { return set_ics_exception(exception_runtime_error(), "_getPythonModuleObject(): Failed to import module"); } // Grab the module Dictionary - PyObject* module_dict = PyModule_GetDict(module); + PyObject* module_dict = PyModule_GetDict(module.get()); if (!module_dict) { return set_ics_exception(exception_runtime_error(), "_getPythonModuleObject(): Failed to grab module dict from module"); @@ -823,13 +830,13 @@ PyObject* _getPythonModuleObject(const char* module_name, const char* module_obj int _isPythonModuleObject_IsInstance(PyObject* object, const char* module_name, const char* module_object_name) { // Before we do anything, we need to grab the python s_device_settings ctype.Structure. - PyObject* module = PyImport_ImportModule(module_name); + PyObjectRef module(PyImport_ImportModule(module_name)); if (!module) { set_ics_exception(exception_runtime_error(), "_isPythonModuleObjectInstanceOf(): Failed to import module"); return -1; } // Grab the module Dictionary - PyObject* module_dict = PyModule_GetDict(module); + PyObject* module_dict = PyModule_GetDict(module.get()); if (!module_dict) { set_ics_exception(exception_runtime_error(), "_isPythonModuleObjectInstanceOf(): Failed to grab module dict from module"); @@ -940,14 +947,14 @@ bool PyNeoDeviceEx_GetHandle(PyObject* object, void** handle) set_ics_exception(exception_runtime_error(), "Object is not of type PyNeoDeviceEx"); return false; } - PyObject* _handle = PyObject_GetAttrString(object, "_handle"); + PyObjectRef _handle(PyObject_GetAttrString(object, "_handle")); if (!_handle) { return false; } - if (!PyCapsule_CheckExact(_handle)) { + if (!PyCapsule_CheckExact(_handle.get())) { return true; } - void* ptr = PyCapsule_GetPointer(_handle, NULL); + void* ptr = PyCapsule_GetPointer(_handle.get(), NULL); if (!ptr) { return false; } @@ -965,20 +972,20 @@ bool PyNeoDeviceEx_SetHandle(PyObject* object, void* handle) set_ics_exception(exception_runtime_error(), "Object is not of type PyNeoDeviceEx"); return false; } - PyObject* _handle = PyObject_GetAttrString(object, "_handle"); + PyObjectRef _handle(PyObject_GetAttrString(object, "_handle")); if (!_handle) { return false; } - if (!PyCapsule_CheckExact(_handle) && handle) { - PyObject* capsule = PyCapsule_New(handle, NULL, __destroy_PyNeoDeviceEx_Handle); + if (!PyCapsule_CheckExact(_handle.get()) && handle) { + PyObjectRef capsule(PyCapsule_New(handle, NULL, __destroy_PyNeoDeviceEx_Handle)); if (!capsule) { return false; } - if (PyObject_SetAttrString(object, "_handle", capsule) != 0) { + if (PyObject_SetAttrString(object, "_handle", capsule.get()) != 0) { return false; } } else if (handle) { - if (!PyCapsule_SetPointer(_handle, handle)) { + if (!PyCapsule_SetPointer(_handle.get(), handle)) { return NULL; } } else { @@ -1771,6 +1778,17 @@ PyObject* meth_transmit_messages(PyObject* self, PyObject* args) if (!PyNeoDeviceEx_GetHandle(obj, &handle)) { return NULL; } + // Validate the entire batch before taking native pointers or transmitting. + // A non-tuple argument represents a single message. + const Py_ssize_t message_count = PyTuple_CheckExact(temp) ? PyTuple_Size(temp) : 1; + for (Py_ssize_t i = 0; i < message_count; ++i) { + PyObject* message = PyTuple_CheckExact(temp) ? PyTuple_GetItem(temp, i) : temp; + if (!PySpyMessage_CheckExact(message) && !PySpyMessageJ1850_CheckExact(message)) { + return set_ics_exception(PyExc_TypeError, + "Message must be of type " MODULE_NAME "." SPY_MESSAGE_OBJECT_NAME " or " + MODULE_NAME "." SPY_MESSAGE_J1850_OBJECT_NAME); + } + } PyObject* tuple = temp; if (!PyTuple_CheckExact(temp)) { tuple = Py_BuildValue("(O)", temp); @@ -1782,6 +1800,19 @@ PyObject* meth_transmit_messages(PyObject* self, PyObject* args) if (!PyTuple_CheckExact(tuple)) { return set_ics_exception(exception_argument_error(), "Second argument must be of tuple type!"); } + for (Py_ssize_t i = 0; i < PyTuple_Size(tuple); ++i) { + PyObject* item = PyTuple_GetItem(tuple, i); + if (!PySpyMessage_CheckExact(item) && !PySpyMessageJ1850_CheckExact(item)) { + if (created_tuple) + Py_DECREF(tuple); + return set_ics_exception(exception_runtime_error(), "Expected SpyMessage or SpyMessageJ1850"); + } + if (!spy_message_validate_extra_data((spy_message_object*)item)) { + if (created_tuple) + Py_DECREF(tuple); + return NULL; + } + } try { ice::Library* lib = dll_get_library(); if (!lib) { @@ -1791,20 +1822,12 @@ PyObject* meth_transmit_messages(PyObject* self, PyObject* args) ice::Function icsneoTxMessages(lib, "icsneoTxMessages"); const Py_ssize_t TUPLE_COUNT = PyTuple_Size(tuple); icsSpyMessage** msgs = new icsSpyMessage*[static_cast(TUPLE_COUNT)](); - for (int i = 0; i < TUPLE_COUNT; ++i) { + for (Py_ssize_t i = 0; i < TUPLE_COUNT; ++i) { spy_message_object* _obj = (spy_message_object*)PyTuple_GetItem(tuple, static_cast(i)); - if (!_obj) { - if (created_tuple) { - Py_XDECREF(tuple); - } - delete[] msgs; - return set_ics_exception(exception_runtime_error(), - "Tuple item must be of " MODULE_NAME "." SPY_MESSAGE_OBJECT_NAME); - } msgs[i] = &(_obj->msg); } auto gil = PyAllowThreads(); - for (int i = 0; i < TUPLE_COUNT; ++i) { + for (Py_ssize_t i = 0; i < TUPLE_COUNT; ++i) { if (!icsneoTxMessages(handle, msgs[i], (msgs[i]->NetworkID2 << 8) | msgs[i]->NetworkID, 1)) { gil.restore(); if (created_tuple) { @@ -1901,6 +1924,7 @@ PyObject* meth_get_messages(PyObject* self, PyObject* args) // Looks like icsneo40 does its own memory management so don't delete when we dealloc msg->noExtraDataPtrCleanup = true; } + spy_message_record_received_extra_data(_obj); PyTuple_SetItem(tuple, i, _obj); } PyObject* result = Py_BuildValue("(O,i)", tuple, errors); @@ -2726,6 +2750,7 @@ PyObject* meth_coremini_read_tx_message(PyObject* self, PyObject* args) // Scrip } gil.restore(); } + spy_message_record_received_extra_data(msg); return msg; } catch (ice::Exception& ex) { return set_ics_exception(exception_runtime_error(), (char*)ex.what()); @@ -2777,7 +2802,7 @@ PyObject* meth_coremini_read_rx_message(PyObject* self, PyObject* args) // Scrip auto gil = PyAllowThreads(); if (!icsneoScriptReadRxMessage(handle, index, - &PySpyMessageJ1850_GetObject(msg_mask)->msg, + &PySpyMessageJ1850_GetObject(msg)->msg, &PySpyMessageJ1850_GetObject(msg_mask)->msg)) { gil.restore(); return set_ics_exception(exception_runtime_error(), "icsneoScriptReadRxMessage() Failed"); @@ -2804,6 +2829,8 @@ PyObject* meth_coremini_read_rx_message(PyObject* self, PyObject* args) // Scrip } gil.restore(); } + spy_message_record_received_extra_data(msg); + spy_message_record_received_extra_data(msg_mask); return Py_BuildValue("(O,O)", msg, msg_mask); } catch (ice::Exception& ex) { return set_ics_exception(exception_runtime_error(), (char*)ex.what()); @@ -2844,6 +2871,8 @@ PyObject* meth_coremini_write_tx_message(PyObject* self, PyObject* args) // icsn } msg = (void*)&PySpyMessage_GetObject(msg_obj)->msg; } + if (!spy_message_validate_extra_data((spy_message_object*)msg_obj)) + return NULL; try { ice::Library* lib = dll_get_library(); if (!lib) { @@ -2883,6 +2912,12 @@ PyObject* meth_coremini_write_rx_message(PyObject* self, PyObject* args) // icsn if (!PyNeoDeviceEx_GetHandle(obj, &handle)) { return NULL; } + if ((PySpyMessage_CheckExact(msg_obj) || PySpyMessageJ1850_CheckExact(msg_obj)) && + !spy_message_validate_extra_data((spy_message_object*)msg_obj)) + return NULL; + if ((PySpyMessage_CheckExact(msg_mask_obj) || PySpyMessageJ1850_CheckExact(msg_mask_obj)) && + !spy_message_validate_extra_data((spy_message_object*)msg_mask_obj)) + return NULL; void* msg = NULL; void* msg_mask = NULL; if (j1850) { diff --git a/src/object_spy_message.cpp b/src/object_spy_message.cpp index 0c1c4a7a..4e0edf51 100644 --- a/src/object_spy_message.cpp +++ b/src/object_spy_message.cpp @@ -1,11 +1,37 @@ #include "object_spy_message.h" +size_t spy_message_extra_data_length(const icsSpyMessage& msg) +{ + if (msg.Protocol == SPY_PROTOCOL_A2B || msg.Protocol == SPY_PROTOCOL_ETHERNET || + msg.Protocol == SPY_PROTOCOL_SPI || msg.Protocol == SPY_PROTOCOL_WBMS) + return (msg.NumberBytesHeader << 8) | msg.NumberBytesData; + return msg.NumberBytesData; +} + +bool spy_message_validate_extra_data(const spy_message_object* obj) +{ + if (obj->msg.ExtraDataPtr && spy_message_extra_data_length(obj->msg) > obj->extraDataCapacity) { + PyErr_SetString(PyExc_ValueError, "ExtraDataPtr length exceeds its buffer capacity"); + return false; + } + return true; +} + +void spy_message_record_received_extra_data(PyObject* o) +{ + spy_message_object* obj = (spy_message_object*)o; + // The DLL owns the pointer; its original reported length is the readable bound. + obj->extraDataCapacity = obj->msg.ExtraDataPtr ? spy_message_extra_data_length(obj->msg) : 0; + obj->noExtraDataPtrCleanup = true; +} + static int spy_message_object_alloc(spy_message_object* self, PyObject* args, PyObject* kwds) { (void)args; (void)kwds; memset(&self->msg, 0, sizeof(self->msg)); self->noExtraDataPtrCleanup = false; + self->extraDataCapacity = 0; return 0; } @@ -64,11 +90,8 @@ static PyObject* spy_message_object_getattr(PyObject* o, PyObject* attr_name) #endif PyErr_Format(PyExc_TypeError, "attribute name must be string, not '%.200s'", attr_name->ob_type->tp_name); return NULL; - } else { - Py_INCREF(attr_name); } if (PyUnicode_CompareWithASCIIString(attr_name, "Data") == 0) { - Py_DECREF(attr_name); spy_message_object* obj = (spy_message_object*)o; PyObject* temp = Py_BuildValue("(i,i,i,i,i,i,i,i)", obj->msg.Data[0], @@ -83,7 +106,6 @@ static PyObject* spy_message_object_getattr(PyObject* o, PyObject* attr_name) Py_DECREF(temp); return data; } else if (PyUnicode_CompareWithASCIIString(attr_name, "AckBytes") == 0) { - Py_DECREF(attr_name); spy_message_object* obj = (spy_message_object*)o; return Py_BuildValue("(i,i,i,i,i,i,i,i)", obj->msg.AckBytes[0], @@ -95,7 +117,6 @@ static PyObject* spy_message_object_getattr(PyObject* o, PyObject* attr_name) obj->msg.AckBytes[6], obj->msg.AckBytes[7]); } else if (PyUnicode_CompareWithASCIIString(attr_name, "Header") == 0) { - Py_DECREF(attr_name); spy_message_j1850_object* obj = (spy_message_j1850_object*)o; PyObject* temp = Py_BuildValue("(i,i,i,i)", obj->msg.Header[0], obj->msg.Header[1], obj->msg.Header[2], obj->msg.Header[3]); @@ -103,7 +124,8 @@ static PyObject* spy_message_object_getattr(PyObject* o, PyObject* attr_name) Py_DECREF(temp); return data; } else if (PyUnicode_CompareWithASCIIString(attr_name, "ExtraDataPtr") == 0) { - Py_DECREF(attr_name); + if (!spy_message_validate_extra_data((spy_message_object*)o)) + return NULL; spy_message_j1850_object* obj = (spy_message_j1850_object*)o; unsigned char* ExtraDataPtr = (unsigned char*)obj->msg.ExtraDataPtr; bool extra_data_ptr_enabled = obj->msg.ExtraDataPtrEnabled != 0; @@ -114,14 +136,7 @@ static PyObject* spy_message_object_getattr(PyObject* o, PyObject* attr_name) && obj->msg.ExtraDataPtr != NULL) { extra_data_ptr_enabled = true; } - int actual_size = 0; - // Some newer protocols are packing the length into NumberBytesHeader also so lets handle it here... - if (obj->msg.Protocol == SPY_PROTOCOL_A2B || obj->msg.Protocol == SPY_PROTOCOL_ETHERNET || - obj->msg.Protocol == SPY_PROTOCOL_SPI || obj->msg.Protocol == SPY_PROTOCOL_WBMS) { - actual_size = (obj->msg.NumberBytesHeader << 8) | obj->msg.NumberBytesData; - } else { - actual_size = obj->msg.NumberBytesData; - } + size_t actual_size = spy_message_extra_data_length(((spy_message_object*)o)->msg); if (extra_data_ptr_enabled && actual_size && obj->msg.ExtraDataPtr) { PyObject* tuple = PyTuple_New(actual_size); for (int i = 0; i < actual_size; ++i) { @@ -136,8 +151,13 @@ static PyObject* spy_message_object_getattr(PyObject* o, PyObject* attr_name) } } -static int spy_message_object_setattr(PyObject* o, PyObject* name, PyObject* value) +static int spy_message_object_setattr_impl(PyObject* o, PyObject* name, PyObject* value) { + // tp_setattro receives NULL for deletion. Let descriptors reject deletion + // before any custom setter inspects the value or changes message storage. + if (value == NULL) + return PyObject_GenericSetAttr(o, name, value); + spy_message_object* obj = (spy_message_object*)o; if (PyUnicode_CompareWithASCIIString(name, "Data") == 0) { Py_ssize_t length = _copy_byte_tuple(value, name, obj->msg.Data, sizeof(obj->msg.Data)); @@ -156,14 +176,6 @@ static int spy_message_object_setattr(PyObject* o, PyObject* name, PyObject* val return -1; obj->msg.NumberBytesHeader = static_cast(length); return 0; - } else if (PyUnicode_CompareWithASCIIString(name, "Protocol") == 0) { - // Ethernet behavior is backward to CAN and will crash if enabled. - long protocol = PyLong_AsLong(value); - if (protocol == -1 && PyErr_Occurred()) - PyErr_Clear(); // let PyObject_GenericSetAttr report the type error - else if (protocol == SPY_PROTOCOL_ETHERNET) - obj->msg.ExtraDataPtrEnabled = 0; - return PyObject_GenericSetAttr(o, name, value); } else if (PyUnicode_CompareWithASCIIString(name, "ExtraDataPtr") == 0) { if (!PyTuple_Check(value)) { PyErr_Format(PyExc_AttributeError, @@ -201,6 +213,7 @@ static int spy_message_object_setattr(PyObject* o, PyObject* name, PyObject* val if (obj->msg.ExtraDataPtr != NULL && !obj->noExtraDataPtrCleanup) delete[] (unsigned char*)obj->msg.ExtraDataPtr; obj->msg.ExtraDataPtr = buffer; + obj->extraDataCapacity = (size_t)length; if (packs_length) obj->msg.NumberBytesHeader = static_cast(length >> 8); obj->msg.NumberBytesData = static_cast(length & 0xFF); @@ -220,6 +233,7 @@ static int spy_message_object_setattr(PyObject* o, PyObject* name, PyObject* val if (obj->msg.ExtraDataPtr != NULL) { delete[] (unsigned char*)obj->msg.ExtraDataPtr; obj->msg.ExtraDataPtr = NULL; + obj->extraDataCapacity = 0; } } else if (enabled != 0 && obj->msg.Protocol == SPY_PROTOCOL_ETHERNET) { // Ethernet always needs to be set to 0 @@ -231,6 +245,81 @@ static int spy_message_object_setattr(PyObject* o, PyObject* name, PyObject* val } } +static int spy_message_object_setattr(PyObject* o, PyObject* name, PyObject* value) +{ + // Subclass deletion descriptors may execute Python. Never include them in + // a native-message rollback transaction. + if (!value) + return PyObject_GenericSetAttr(o, name, value); + // These assignments can reinterpret or enlarge the payload length. Roll back + // the complete native message, including Data/Header bytes, on failure. + bool scalar_length = PyUnicode_CompareWithASCIIString(name, "NumberBytesData") == 0 || + PyUnicode_CompareWithASCIIString(name, "NumberBytesHeader") == 0 || + PyUnicode_CompareWithASCIIString(name, "Protocol") == 0; + bool data = PyUnicode_CompareWithASCIIString(name, "Data") == 0; + bool header = PyUnicode_CompareWithASCIIString(name, "Header") == 0; + bool changes_length = scalar_length || data || header; + // __index__ can execute arbitrary Python, including replacing the payload. + // Finish all such conversions BEFORE snapshotting the message for rollback. + PyObject* normalized = NULL; + if (value && scalar_length) { + normalized = PyNumber_Index(value); + if (!normalized) + return -1; + long byte = PyLong_AsLong(normalized); + if ((byte == -1 && PyErr_Occurred()) || byte < 0 || byte > 255) { + Py_DECREF(normalized); + if (!PyErr_Occurred()) + PyErr_SetString(PyExc_ValueError, "Message length and protocol fields must be in range 0..255"); + return -1; + } + // Reject truncation rather than letting T_UBYTE issue a warning inside + // the transaction: a user warning hook can also replace the payload. + } else if (value && (data || header) && PyTuple_Check(value) && + PyTuple_Size(value) <= (data ? 8 : 4)) { + normalized = PyTuple_New(PyTuple_Size(value)); + if (!normalized) + return -1; + for (Py_ssize_t i = 0; i < PyTuple_Size(value); ++i) { + PyObject* byte = PyNumber_Index(PyTuple_GetItem(value, i)); + if (!byte) { + Py_DECREF(normalized); + return -1; + } + PyTuple_SET_ITEM(normalized, i, byte); + } + } + spy_message_object* obj = (spy_message_object*)o; + icsSpyMessage original = obj->msg; + int result = 0; + if (scalar_length) { + // These native fields, like Data/Header, must not dispatch subclass + // descriptors inside the transaction: they can replace/free payloads. + unsigned char byte = (unsigned char)PyLong_AsLong(normalized); + if (PyUnicode_CompareWithASCIIString(name, "NumberBytesData") == 0) + obj->msg.NumberBytesData = byte; + else if (PyUnicode_CompareWithASCIIString(name, "NumberBytesHeader") == 0) + obj->msg.NumberBytesHeader = byte; + else { + obj->msg.Protocol = byte; + if (byte == SPY_PROTOCOL_ETHERNET) + obj->msg.ExtraDataPtrEnabled = 0; + } + } else { + result = spy_message_object_setattr_impl(o, name, normalized ? normalized : value); + } + Py_XDECREF(normalized); + if (result == 0 && changes_length && obj->msg.ExtraDataPtr && + spy_message_extra_data_length(obj->msg) > obj->extraDataCapacity) { + // Restore before allocating the exception: cyclic-GC finalizers may + // execute Python during allocation and must see the committed state. + obj->msg = original; + PyErr_SetString(PyExc_ValueError, "ExtraDataPtr length exceeds its buffer capacity"); + return -1; + } + return result; +} + static PyMemberDef spy_message_object_members[] = { { "StatusBitField", T_UINT, offsetof(spy_message_object, msg.StatusBitField), 0, "StatusBitField" }, { "StatusBitField2", T_UINT, offsetof(spy_message_object, msg.StatusBitField2), 0, "StatusBitField2" }, @@ -484,4 +573,4 @@ bool setup_spy_message_object(PyObject* module) Py_INCREF(&spy_message_j1850_object_type); PyModule_AddObject(module, SPY_MESSAGE_J1850_OBJECT_NAME, (PyObject*)&spy_message_j1850_object_type); return true; -} \ No newline at end of file +} diff --git a/tests/_reference_helpers.py b/tests/_reference_helpers.py new file mode 100644 index 00000000..7130404a --- /dev/null +++ b/tests/_reference_helpers.py @@ -0,0 +1,125 @@ +"""Run in a subprocess by test_reference_leaks.py with a mock native library.""" +import ctypes +import importlib +import os +import sys + +import ics +import pytest + + +@pytest.fixture(autouse=True, scope="module") +def mock_library(): + ics.override_library_name(os.environ["ICS_REFERENCE_MOCK"]) + + +@pytest.fixture +def device(): + device = ics.PyNeoDeviceEx() + device._auto_handle_close = False + return device + + +def assert_stable(target, action): + for _ in range(50): + action() + before = sys.getrefcount(target) + for _ in range(1000): + action() + assert sys.getrefcount(target) == before + + +def expect_error(action, error): + with pytest.raises(error): + action() + + +def assert_capsule_ownership(capsules, device_references=0): + new = ctypes.pythonapi.PyCapsule_New + new.argtypes = [ctypes.c_void_p, ctypes.c_char_p, ctypes.c_void_p] + new.restype = ctypes.py_object + control = [new(0x1234, None, None)] + # Compare equally owned list elements. CPython versions can differ in the + # temporary references they use for local variables and getrefcount calls. + for index in range(len(capsules)): + assert sys.getrefcount(capsules[index]) == sys.getrefcount(control[0]) + device_references + + +def test_handle_creation_and_clear(device): + for _ in range(50): + assert ics.open_device(device) is device + capsules = [device._handle] + assert_capsule_ownership(capsules, device_references=1) + assert device._Handle == 0x1234 + assert ics.close_device(device) == 0 + assert device._handle is None + assert_capsule_ownership(capsules) + + +@pytest.mark.parametrize("kind", ["capsule", "noncapsule", "named_capsule"]) +def test_handle_lookup(device, kind): + if kind == "noncapsule": + handle = object() + else: + new = ctypes.pythonapi.PyCapsule_New + new.argtypes = [ctypes.c_void_p, ctypes.c_char_p, ctypes.c_void_p] + new.restype = ctypes.py_object + handle = new(0x1234, b"wrong_name" if kind == "named_capsule" else None, None) + device._handle = handle + if kind == "named_capsule": + action = lambda: expect_error(lambda: ics.get_device_status(device), ValueError) + else: + action = lambda: ics.get_device_status(device) + assert_stable(handle, action) + + +@pytest.mark.parametrize("failure", ["missing", "raises"]) +def test_handle_attribute_failure(device, monkeypatch, failure): + if failure == "missing": + monkeypatch.delattr(ics.PyNeoDeviceEx, "_handle") + expect_error(lambda: ics.get_device_status(device), AttributeError) + else: + rejected = [] + + def reject(self, value): + rejected.append(value) + raise ValueError("reject handle assignment") + monkeypatch.setattr(ics.PyNeoDeviceEx, "_handle", property(lambda self: sentinel, reject)) + sentinel = object() + assert_stable(sentinel, lambda: expect_error(lambda: ics.open_device(device), ValueError)) + # A failing setter never stole the helper's newly created reference. + assert_capsule_ownership(rejected) + + +@pytest.mark.parametrize("helper", ["construct", "isinstance"]) +@pytest.mark.parametrize("case", ["success", "missing", "raises", "not_module"]) +def test_module_references(device, monkeypatch, helper, case): + name = "ics_device_status" if helper == "construct" else "st_cm_iso157652_rx_message" + module_name = "ics.structures." + name + module = importlib.import_module(module_name) + cls = getattr(module, name) + message = cls() + action = (lambda: ics.get_device_status(device)) if helper == "construct" else ( + lambda: ics.iso15765_receive_message(device, 0, message) + ) + if case == "missing": + monkeypatch.delattr(module, name) + elif case == "raises": + if helper == "construct": + def raises(): + raise ValueError("constructor failed") + replacement = raises + else: + class Meta(type): + def __instancecheck__(self, instance): + raise ValueError("instance check failed") + replacement = Meta("FailingClass", (), {}) + monkeypatch.setattr(module, name, replacement) + elif case == "not_module": + module = object() + monkeypatch.setitem(sys.modules, module_name, module) + if case != "success": + original_action = action + error = ValueError if helper == "isinstance" and case == "raises" else ics.RuntimeError + action = lambda: expect_error(original_action, error) + assert_stable(module, action) diff --git a/tests/extra_data_capacity_mock.cpp b/tests/extra_data_capacity_mock.cpp new file mode 100644 index 00000000..57afceb8 --- /dev/null +++ b/tests/extra_data_capacity_mock.cpp @@ -0,0 +1,45 @@ +#include +#include "icsnVC40.h" + +#ifdef _WIN32 +#define API extern "C" __declspec(dllexport) int __stdcall +#else +#define API extern "C" int +#endif + +static unsigned char payload[300]; +static int calls; + +static void receive(icsSpyMessage* msg) +{ + memset(msg, 0, sizeof(*msg)); + for (int i = 0; i < 300; ++i) + payload[i] = (unsigned char)i; + msg->Protocol = SPY_PROTOCOL_ETHERNET; + msg->NumberBytesHeader = 1; + msg->NumberBytesData = 44; + msg->ExtraDataPtr = payload; +} + +API icsneoGetDLLVersion() { return 1; } +API icsneoWaitForRxMessagesWithTimeOut(void*, unsigned int) { return 1; } +API icsneoGetMessages(void*, icsSpyMessage* msg, int* count, int* errors) +{ + receive(msg); + *count = 1; + *errors = 0; + return 1; +} +API icsneoScriptReadTxMessage(void*, unsigned int, icsSpyMessage* msg) +{ + receive(msg); + return 1; +} +API icsneoScriptReadRxMessage(void*, unsigned int, icsSpyMessage* msg, icsSpyMessage* mask) +{ + receive(msg); + receive(mask); + return 1; +} +API icsneoTxMessages(void*, icsSpyMessage*, int, int) { ++calls; return 1; } +API capacity_mock_calls() { return calls; } diff --git a/tests/reference_mock.c b/tests/reference_mock.c new file mode 100644 index 00000000..0e5ea088 --- /dev/null +++ b/tests/reference_mock.c @@ -0,0 +1,16 @@ +/* No hardware access or external runtime dependencies. */ +#ifdef _WIN32 +#define API __declspec(dllexport) __stdcall +#else +#define API +#endif +int API icsneoOpenDevice(void* device, void** handle, unsigned char* networks, + int config, int options, void* extra, unsigned long reserved) +{ + *handle = (void*)0x1234; + return 1; +} +int API icsneoClosePort(void* handle, int* errors) { *errors = 0; return 1; } +void API icsneoFreeObject(void* handle) {} +int API icsneoGetDeviceStatus(void* handle, void* status, void* size) { return 1; } +int API icsneoISO15765_ReceiveMessage(void* handle, unsigned int index, void* message) { return 1; } diff --git a/tests/test_extra_data_capacity.py b/tests/test_extra_data_capacity.py new file mode 100644 index 00000000..1a483a96 --- /dev/null +++ b/tests/test_extra_data_capacity.py @@ -0,0 +1,221 @@ +"""Payload bounds must not depend on mutable wire-format length fields.""" +import ctypes +import os +import subprocess +import sys + +import ics +import pytest + + +MESSAGE_TYPES = [ics.SpyMessage, ics.SpyMessageJ1850] +PACKED_PROTOCOLS = [ics.SPY_PROTOCOL_ETHERNET, ics.SPY_PROTOCOL_A2B, ics.SPY_PROTOCOL_SPI, ics.SPY_PROTOCOL_WBMS] + + +@pytest.mark.parametrize("message_type", MESSAGE_TYPES) +@pytest.mark.parametrize("attribute", ["NumberBytesData", "NumberBytesHeader", "Protocol"]) +def test_subclass_descriptors_cannot_replace_payload_during_rollback(message_type, attribute): + script = r''' +import ics +import sys + +base = getattr(ics, sys.argv[1]) +attribute = sys.argv[2] +calls = [] +def replace_payload(self, value): + calls.append(True) + self.ExtraDataPtr = (2, 3) + getattr(base, attribute).__set__(self, value) +def delete_payload(self): + self.ExtraDataPtr = (2, 3) + # A deletion callback may leave inconsistent fields; the getter must reject + # them, but rollback must never resurrect the freed original payload. + base.NumberBytesData.__set__(self, 3) +message_type = type("Message", (base,), {attribute: property(fset=replace_payload, fdel=delete_payload)}) +msg = message_type() +base.Protocol.__set__(msg, ics.SPY_PROTOCOL_SPI) +if attribute == "Protocol": + base.Protocol.__set__(msg, ics.SPY_PROTOCOL_CANFD) + base.NumberBytesHeader.__set__(msg, 1) +msg.ExtraDataPtr = (17,) +value = {"NumberBytesData": 3, "NumberBytesHeader": 1, "Protocol": ics.SPY_PROTOCOL_ETHERNET}[attribute] +try: + setattr(msg, attribute, value) +except ValueError: + pass +else: + raise AssertionError("unsafe native field accepted") +assert calls == [] +assert msg.ExtraDataPtr == (17,) +delattr(msg, attribute) +try: + msg.ExtraDataPtr +except ValueError: + pass +else: + raise AssertionError("inconsistent deletion result accepted") +base.NumberBytesData.__set__(msg, 2) +assert msg.ExtraDataPtr == (2, 3) +del msg +''' + env = os.environ.copy() + env["PYTHONPATH"] = os.pathsep.join(str(path) for path in sys.path) + result = subprocess.run([sys.executable, "-c", script, message_type.__name__, attribute], + env=env, capture_output=True, text=True, timeout=30) + assert result.returncode == 0, result.stdout + result.stderr + + +@pytest.mark.parametrize("message_type", MESSAGE_TYPES) +@pytest.mark.parametrize("protocol", [ics.SPY_PROTOCOL_CANFD] + PACKED_PROTOCOLS) +def test_payload_length_cannot_exceed_allocation(message_type, protocol): + msg = message_type() + msg.Protocol = protocol + msg.ExtraDataPtr = (17,) + with pytest.raises(ValueError, match="capacity"): + msg.NumberBytesData = 64 + assert msg.NumberBytesData == 1 + assert msg.ExtraDataPtr == (17,) + if protocol in PACKED_PROTOCOLS: + with pytest.raises(ValueError, match="capacity"): + msg.NumberBytesHeader = 1 + assert msg.NumberBytesHeader == 0 + + +@pytest.mark.parametrize("message_type", MESSAGE_TYPES) +@pytest.mark.parametrize("protocol", PACKED_PROTOCOLS) +def test_protocol_change_cannot_reinterpret_header_as_extra_capacity(message_type, protocol): + msg = message_type() + msg.Protocol = ics.SPY_PROTOCOL_CANFD + msg.NumberBytesHeader = 1 + msg.ExtraDataPtr = (17,) + with pytest.raises(ValueError, match="capacity"): + msg.Protocol = protocol + assert msg.Protocol == ics.SPY_PROTOCOL_CANFD + assert msg.ExtraDataPtrEnabled == 1 + assert msg.ExtraDataPtr == (17,) + + +@pytest.mark.parametrize("message_type", MESSAGE_TYPES) +def test_data_and_header_updates_are_atomic(message_type): + msg = message_type() + msg.Protocol = ics.SPY_PROTOCOL_SPI + msg.ExtraDataPtr = (17,) + with pytest.raises(ValueError, match="capacity"): + msg.Data = (1, 2) + assert msg.Data == (0,) + if message_type is ics.SpyMessageJ1850: + with pytest.raises(ValueError, match="capacity"): + msg.Header = (1,) + assert msg.Header == () + assert msg.ExtraDataPtr == (17,) + + +@pytest.mark.parametrize("message_type", MESSAGE_TYPES) +def test_capacity_survives_shrink_replacement_and_cleanup_flag(message_type): + msg = message_type() + msg.Protocol = ics.SPY_PROTOCOL_SPI + payload = tuple(i & 255 for i in range(300)) + msg.ExtraDataPtr = payload + msg.NumberBytesHeader = 0 + assert msg.ExtraDataPtr == payload[:44] + msg.NumberBytesHeader = 1 + assert msg.ExtraDataPtr == payload + msg.ExtraDataPtr = (2, 3) + msg.noExtraDataPtrCleanup = True + try: + with pytest.raises(ValueError, match="capacity"): + msg.NumberBytesData = 3 + finally: + msg.noExtraDataPtrCleanup = False + with pytest.raises(TypeError): + msg.ExtraDataPtr = (1, "bad") + assert msg.ExtraDataPtr == (2, 3) + msg.ExtraDataPtr = () + with pytest.raises(ValueError, match="capacity"): + msg.NumberBytesData = 1 + assert msg.ExtraDataPtr is None + + +@pytest.mark.parametrize("message_type", MESSAGE_TYPES) +def test_direct_descriptor_write_still_cannot_read_past_payload(message_type): + msg = message_type() + msg.ExtraDataPtr = (17,) + # Member descriptors bypass tp_setattro; the getter must defend itself too. + message_type.NumberBytesData.__set__(msg, 64) + with pytest.raises(ValueError, match="capacity"): + _ = msg.ExtraDataPtr + + +@pytest.mark.parametrize("attribute", ["NumberBytesData", "Data", "Header", "Protocol"]) +def test_reentrant_integer_conversion_preserves_replacement_payload(attribute): + msg = ics.SpyMessageJ1850() + msg.Protocol = ics.SPY_PROTOCOL_CANFD + msg.ExtraDataPtr = (17,) + + class ReplacePayload: + def __index__(self): + msg.ExtraDataPtr = (2, 3) + if attribute == "Header": + msg.Protocol = ics.SPY_PROTOCOL_SPI + if attribute == "Protocol": + msg.NumberBytesHeader = 1 + return ics.SPY_PROTOCOL_SPI + return 3 + + value = ReplacePayload() + if attribute == "Data": + value = (value, 4, 5) + elif attribute == "Header": + value = (value,) + with pytest.raises(ValueError, match="capacity"): + setattr(msg, attribute, value) + assert msg.ExtraDataPtr == (2, 3) + assert msg.NumberBytesData == 2 + + +@pytest.mark.parametrize("attribute", ["NumberBytesData", "NumberBytesHeader", "Protocol"]) +@pytest.mark.parametrize("value", [-1, 259]) +def test_out_of_range_scalar_does_not_call_warning_hook(attribute, value, monkeypatch): + import warnings + + msg = ics.SpyMessage() + msg.ExtraDataPtr = (17,) + calls = [] + + def replace_payload(*args, **kwargs): + calls.append(True) + msg.ExtraDataPtr = (2, 3) + + monkeypatch.setattr(warnings, "showwarning", replace_payload) + with warnings.catch_warnings(): + warnings.simplefilter("always") + with pytest.raises(ValueError, match="0..255"): + setattr(msg, attribute, value) + assert calls == [] + assert msg.ExtraDataPtr == (17,) + + +@pytest.mark.parametrize("message_type", MESSAGE_TYPES) +@pytest.mark.parametrize("operation", ["transmit", "tx_script", "rx_script", "rx_mask"]) +def test_invalid_payload_is_rejected_before_native_library_lookup(message_type, operation): + device = ics.PyNeoDeviceEx() + device._auto_handle_close = False + # No device or native library is opened. The sentinel capsule only passes + # handle extraction; validation must reject the message before native use. + capsule_new = ctypes.pythonapi.PyCapsule_New + capsule_new.argtypes = [ctypes.c_void_p, ctypes.c_char_p, ctypes.c_void_p] + capsule_new.restype = ctypes.py_object + device._handle = capsule_new(1, None, None) + msg = message_type() + msg.ExtraDataPtr = (17,) + message_type.NumberBytesData.__set__(msg, 64) + j1850 = message_type is ics.SpyMessageJ1850 + with pytest.raises(ValueError, match="capacity"): + if operation == "transmit": + ics.transmit_messages(device, (message_type(), msg)) + elif operation == "tx_script": + ics.coremini_write_tx_message(device, 0, msg, j1850) + elif operation == "rx_script": + ics.coremini_write_rx_message(device, 0, msg, message_type(), j1850) + else: + ics.coremini_write_rx_message(device, 0, message_type(), msg, j1850) diff --git a/tests/test_extra_data_capacity_native.py b/tests/test_extra_data_capacity_native.py new file mode 100644 index 00000000..6b48b718 --- /dev/null +++ b/tests/test_extra_data_capacity_native.py @@ -0,0 +1,92 @@ +"""Exercise DLL-owned payloads in a subprocess; never load a hardware library.""" +import os +from pathlib import Path +import subprocess +import sys + +import pytest + + +def test_received_payload_capacity_and_native_transmit(tmp_path): + # setuptools supplies portable compiler discovery, including MSVC. This + # integration test needs the same native toolchain as the extension build. + pytest.importorskip("setuptools") + from setuptools._distutils.ccompiler import new_compiler + from setuptools._distutils.sysconfig import customize_compiler + + compiler = new_compiler() + customize_compiler(compiler) + root = Path(__file__).resolve().parents[1] + objects = compiler.compile( + [str(root / "tests" / "extra_data_capacity_mock.cpp")], + output_dir=str(tmp_path), + include_dirs=[str(root / "include" / "ics")], + macros=[("EXTERNAL_PROJECT", "1")], + ) + library = tmp_path / ("capacity_mock.dll" if os.name == "nt" else "capacity_mock.so") + extra_args = [] + if os.name == "nt" and sys.maxsize <= 2**32: + # stdcall exports are decorated on x86, but the runtime resolves the + # vendor API by its undecorated names. + exports = {"icsneoGetDLLVersion": 0, "icsneoWaitForRxMessagesWithTimeOut": 8, + "icsneoGetMessages": 16, "icsneoScriptReadTxMessage": 12, + "icsneoScriptReadRxMessage": 16, "icsneoTxMessages": 16, "capacity_mock_calls": 0} + extra_args = [f"/EXPORT:{name}=_{name}@{size}" for name, size in exports.items()] + compiler.link_shared_object(objects, str(library), target_lang="c++", extra_postargs=extra_args) + script = r''' +import ctypes +import gc +import sys +import ics + +ics.override_library_name(sys.argv[1]) +dll = (ctypes.WinDLL if sys.platform == "win32" else ctypes.CDLL)(sys.argv[1]) +device = ics.PyNeoDeviceEx() +device._auto_handle_close = False +payload = tuple(i & 255 for i in range(300)) +for j1850 in (False, True): + for source in ("receive", "script", "rx_message", "rx_mask"): + if source == "receive": + messages, errors = ics.get_messages(device, j1850, 0) + assert errors == 0 + msg = messages[0] + elif source == "script": + msg = ics.coremini_read_tx_message(device, 0, j1850) + else: + pair = ics.coremini_read_rx_message(device, 0, j1850) + assert all(m.ExtraDataPtr == payload for m in pair) + msg = pair[source == "rx_mask"] + assert msg.ExtraDataPtr == payload + assert msg.noExtraDataPtrCleanup + msg.NumberBytesHeader = 0 + assert msg.ExtraDataPtr == payload[:44] + msg.NumberBytesHeader = 1 + try: + msg.NumberBytesData = 45 + except ValueError: + pass + else: + raise AssertionError("received capacity was enlarged") + before = dll.capacity_mock_calls() + ics.transmit_messages(device, msg) + assert dll.capacity_mock_calls() == before + 1 + type(msg).NumberBytesHeader.__set__(msg, 2) + for operation in (lambda: msg.ExtraDataPtr, lambda: ics.transmit_messages(device, msg)): + try: + operation() + except ValueError: + pass + else: + raise AssertionError("descriptor bypass exposed native payload") + assert dll.capacity_mock_calls() == before + 1 + # Replacement must not delete the DLL's static buffer, and must set a + # new bound independent of the received length. + msg.ExtraDataPtr = (9,) + assert msg.ExtraDataPtr == (9,) + assert not msg.noExtraDataPtrCleanup + del msg + gc.collect() +print("mock receive, script read, replacement and transmit bounds passed") +''' + result = subprocess.run([sys.executable, "-c", script, str(library)], text=True, capture_output=True) + assert result.returncode == 0, result.stdout + result.stderr diff --git a/tests/test_reference_leaks.py b/tests/test_reference_leaks.py new file mode 100644 index 00000000..23165728 --- /dev/null +++ b/tests/test_reference_leaks.py @@ -0,0 +1,71 @@ +"""Reference ownership regressions; native calls run only against a mock library.""" +import os +from pathlib import Path +import shutil +import struct +import subprocess +import sys + +import ics +import pytest + + +@pytest.mark.parametrize("message_type", [ics.SpyMessage, ics.SpyMessageJ1850]) +@pytest.mark.parametrize("attribute", ["StatusBitField", "Data", "AckBytes", "Header", "ExtraDataPtr", "missing_attribute"]) +def test_attribute_name_reference_count(message_type, attribute): + message = message_type() + name = "".join([attribute[:-1], attribute[-1:]]) + + def read(): + try: + getattr(message, name) + except AttributeError: + assert attribute == "missing_attribute" + + for _ in range(50): + read() + before = sys.getrefcount(name) + for _ in range(1000): + read() + assert sys.getrefcount(name) == before + + +def test_native_helper_reference_counts(tmp_path): + clang = shutil.which("clang") + if not clang: + pytest.skip("clang is required to build the hardware-free mock library") + source = Path(__file__).with_name("reference_mock.c") + library = tmp_path / ("reference_mock.dll" if sys.platform == "win32" else "reference_mock.so") + if sys.platform == "win32": + linker = shutil.which("lld-link") + if not linker: + pytest.skip("lld-link is required to link the hardware-free mock library") + target = "x86_64-pc-windows-msvc" if struct.calcsize("P") == 8 else "i686-pc-windows-msvc" + obj = tmp_path / "reference_mock.obj" + subprocess.run([clang, "-target", target, "-c", str(source), "-o", str(obj)], check=True) + exports = [] + if struct.calcsize("P") == 4: + # ice looks up undecorated names; i686 stdcall exports include stack sizes. + exports = [ + f"/export:{name}=_{name}@{size}" + for name, size in [ + ("icsneoOpenDevice", 28), + ("icsneoClosePort", 8), + ("icsneoFreeObject", 4), + ("icsneoGetDeviceStatus", 12), + ("icsneoISO15765_ReceiveMessage", 12), + ] + ] + subprocess.run( + [linker, "/dll", "/noentry", "/nodefaultlib", str(obj), f"/out:{library}", *exports], check=True + ) + else: + subprocess.run([clang, "-shared", "-fPIC", str(source), "-o", str(library)], check=True) + # Isolate the global library override and module monkeypatches from the suite. + env = os.environ.copy() + env["ICS_REFERENCE_MOCK"] = str(library) + result = subprocess.run( + [sys.executable, "-m", "pytest", "-q", str(Path(__file__).with_name("_reference_helpers.py"))], + env=env, capture_output=True, text=True, timeout=120, + ) + assert result.returncode == 0, result.stdout + result.stderr diff --git a/tests/test_spy_message.py b/tests/test_spy_message.py index f25371da..dbe9bf75 100644 --- a/tests/test_spy_message.py +++ b/tests/test_spy_message.py @@ -1,7 +1,48 @@ +import os +import subprocess +import sys +import textwrap + import pytest import ics +@pytest.mark.parametrize("message_type", ["SpyMessage", "SpyMessageJ1850"]) +@pytest.mark.parametrize("attribute", ["Data", "AckBytes", "Header", "ExtraDataPtr", "Protocol", "ExtraDataPtrEnabled"]) +@pytest.mark.parametrize("initialized", [False, True]) +def test_attribute_deletion_is_safe(message_type, attribute, initialized): + # Isolate native crashes so every affected attribute/type is reported. + code = textwrap.dedent(f""" + import ics + + msg = ics.{message_type}() + if {initialized!r}: + msg.Protocol = ics.SPY_PROTOCOL_CANFD + msg.Data = (1, 2, 3) + msg.AckBytes = (4, 5) + msg.Header = (6, 7) + msg.ExtraDataPtr = (8, 9, 10) + fields = ("Data", "AckBytes", "Header", "ExtraDataPtr", "Protocol", + "ExtraDataPtrEnabled", "NumberBytesData", "NumberBytesHeader") + before = tuple(getattr(msg, field) for field in fields) + try: + delattr(msg, {attribute!r}) + except (AttributeError, TypeError): + pass + else: + raise AssertionError("deletion should be rejected") + assert tuple(getattr(msg, field) for field in fields) == before + # The message remains usable, including replacing and freeing its buffer. + msg.ExtraDataPtr = (11, 12) + assert msg.ExtraDataPtr == (11, 12) + del msg + """) + env = os.environ.copy() + env["PYTHONPATH"] = os.pathsep.join(str(path) for path in sys.path) + result = subprocess.run([sys.executable, "-c", code], env=env, capture_output=True, text=True, timeout=30) + assert result.returncode == 0, f"exit {result.returncode}\n{result.stdout}\n{result.stderr}" + + def test_data_roundtrip(): msg = ics.SpyMessage() msg.Data = (1, 2, 3, 4, 5, 6, 7, 8) diff --git a/tests/test_transmit_messages.py b/tests/test_transmit_messages.py new file mode 100644 index 00000000..4af99c74 --- /dev/null +++ b/tests/test_transmit_messages.py @@ -0,0 +1,140 @@ +"""Hardware-free transmit validation using a native call-counting stub.""" + +import os +from pathlib import Path +import shutil +import subprocess +import sys + +import pytest + + +@pytest.fixture(scope="module") +def transmit_library(tmp_path_factory): + directory = tmp_path_factory.mktemp("transmit-library") + source = directory / "mock.c" + source.write_text( + """ +#ifdef _WIN32 +#define API __declspec(dllexport) +#define CALL __stdcall +#else +#define API +#define CALL +#endif +static int calls; +static int network; +static int result = 1; +API int CALL icsneoTxMessages(void* handle, void* message, int net, int count) { + ++calls; + network = net; + return result; +} +API int review_calls(void) { return calls; } +API int review_network(void) { return network; } +API void review_result(int value) { result = value; } +""" + ) + if sys.platform == "win32": + compiler = shutil.which("clang-cl") + linker = shutil.which("lld-link") + if not compiler or not linker: + pytest.skip("Native transmit tests require clang-cl and lld-link") + library = directory / "mock.dll" + obj = directory / "mock.obj" + bits = 64 if sys.maxsize > 2**32 else 32 + subprocess.run( + [compiler, f"-m{bits}", "/nologo", "/c", "/GS-", "/Zl", str(source), f"/Fo{obj}"], check=True + ) + command = [linker, "/dll", "/noentry", "/nodefaultlib", f"/out:{library}", str(obj)] + if bits == 32: + command.append("/export:icsneoTxMessages=_icsneoTxMessages@16") + subprocess.run(command, check=True) + else: + compiler = shutil.which("cc") + if not compiler: + pytest.skip("Native transmit tests require a C compiler") + library = directory / ("mock.dylib" if sys.platform == "darwin" else "mock.so") + subprocess.run([compiler, "-shared", "-fPIC", str(source), "-o", str(library)], check=True) + return library + + +def test_transmit_message_types(transmit_library): + # Isolate the process-global library override from other tests. No real + # device is opened and the mock never dereferences a message pointer. + script = r''' +import ctypes +import sys +import ics + +ics.override_library_name(sys.argv[1]) +library = ctypes.CDLL(sys.argv[1]) +device = ics.PyNeoDeviceEx() +device._auto_handle_close = False +invalid_device = ics.PyNeoDeviceEx() +invalid_device._auto_handle_close = False + +for invalid in (None, 42, object(), "message", [], {}, invalid_device): + for argument in (invalid, (invalid,), (ics.SpyMessage(), invalid), + (invalid, ics.SpyMessageJ1850()), + (ics.SpyMessage(), invalid, ics.SpyMessageJ1850())): + before = library.review_calls() + try: + ics.transmit_messages(device, argument) + except TypeError as error: + assert "SpyMessage" in str(error) + else: + raise AssertionError(f"accepted invalid input: {argument!r}") + assert library.review_calls() == before, "partially transmitted invalid batch" + +# Measure ownership separately with private objects. Shared singleton counts +# include unrelated interpreter activity and device._handle lookups (#243). +# Check the rejected scalar, batch container, and valid neighbors so neither +# a direct reference leak nor a retained temporary tuple can go unnoticed. +sentinel = object() +neighbors = (ics.SpyMessage(), ics.SpyMessageJ1850()) +for argument in (sentinel, (sentinel,), (neighbors[0], sentinel), + (sentinel, neighbors[1]), (*neighbors, sentinel)): + tracked = (sentinel, argument, *neighbors) + refs = tuple(sys.getrefcount(value) for value in tracked) + before = library.review_calls() + for _ in range(100): + try: + ics.transmit_messages(device, argument) + except TypeError: + pass + else: + raise AssertionError("accepted invalid owned input") + assert library.review_calls() == before, "partially transmitted invalid batch" + assert tuple(sys.getrefcount(value) for value in tracked) == refs, "rejected input retained references" + +standard = ics.SpyMessage() +j1850 = ics.SpyMessageJ1850() +standard.NetworkID = 0x34 +standard.NetworkID2 = 0x12 +j1850.NetworkID = 0x78 +j1850.NetworkID2 = 0x56 +for argument, count, network in ((standard, 1, 0x1234), (j1850, 1, 0x5678), + ((standard, j1850), 2, 0x5678), ((), 0, 0x5678)): + before = library.review_calls() + assert ics.transmit_messages(device, argument) is None + assert library.review_calls() == before + count + assert library.review_network() == network + +library.review_result(0) +before = library.review_calls() +try: + ics.transmit_messages(device, (standard, j1850)) +except ics.RuntimeError: + pass +else: + raise AssertionError("native transmit failure was ignored") +assert library.review_calls() == before + 1 +''' + # Pass the actual imported package location even when pytest was launched + # with an adjusted sys.path instead of an installed wheel. + import ics + + environment = os.environ.copy() + environment["PYTHONPATH"] = os.pathsep.join([str(Path(ics.__file__).resolve().parent.parent), *sys.path]) + subprocess.run([sys.executable, "-c", script, str(transmit_library)], env=environment, check=True)