diff --git a/libnvme/libnvme/meson.build b/libnvme/libnvme/meson.build index d2b2aa8851..ac5eda2b43 100644 --- a/libnvme/libnvme/meson.build +++ b/libnvme/libnvme/meson.build @@ -84,6 +84,7 @@ if want_python [ 'sigsegv-during-gc', [ files('tests/gc.py'), ] ], [ 'read-nbft-file', [ files('tests/test-nbft.py'), '--filename', join_paths(meson.current_source_dir(), 'tests', 'NBFT') ] ], [ 'object-properties-and-errors', [ files('tests/test-objects.py'), ] ], + [ 'setattr-guards', [ files('tests/test-setattr.py'), ] ], ] foreach test: py_tests description = test[0] diff --git a/libnvme/libnvme/nvme.i b/libnvme/libnvme/nvme.i index 714cdbac14..5c7067c13d 100644 --- a/libnvme/libnvme/nvme.i +++ b/libnvme/libnvme/nvme.i @@ -105,6 +105,14 @@ static inline PyObject *Py_NewRef(PyObject *obj) cfg = libnvmf_context_get_fabrics_config(fctx); while (PyDict_Next(dict, &pos, &key, &value)) { + /* Already consumed above via dict_get_str() */ + if (!PyUnicode_CompareWithASCIIString(key, "subsysnqn") || + !PyUnicode_CompareWithASCIIString(key, "transport") || + !PyUnicode_CompareWithASCIIString(key, "traddr") || + !PyUnicode_CompareWithASCIIString(key, "trsvcid") || + !PyUnicode_CompareWithASCIIString(key, "host_traddr") || + !PyUnicode_CompareWithASCIIString(key, "host_iface")) + continue; if (!PyUnicode_CompareWithASCIIString(key, "queue_size")) { libnvme_fabrics_config_set_queue_size(cfg, PyLong_AsLong(value)); continue; @@ -216,7 +224,8 @@ static inline PyObject *Py_NewRef(PyObject *obj) has_persistent = true; continue; } - /* Unknown keys are silently ignored */ + PyErr_Format(PyExc_KeyError, "unknown ctrl config key: '%U'", key); + return -1; } if (hostnqn || hostid) @@ -686,6 +695,20 @@ struct libnvme_ns { while h: yield h h = libnvme_next_host(self, h) + def __setattr__(self, name, value): + if name == 'this' or name.startswith('_'): + object.__setattr__(self, name, value) + return + for klass in type(self).__mro__: + attr = klass.__dict__.get(name) + if attr is not None: + if isinstance(attr, property) and attr.fset is not None: + attr.fset(self, value) + return + break + raise AttributeError( + f"'{type(self).__name__}' object has no writable attribute '{name}'" + ) %} void refresh_topology() { libnvme_refresh_topology($self); @@ -750,6 +773,20 @@ struct libnvme_ns { while s: yield s s = libnvme_next_subsystem(self, s) + def __setattr__(self, name, value): + if name == 'this' or name.startswith('_'): + object.__setattr__(self, name, value) + return + for klass in type(self).__mro__: + attr = klass.__dict__.get(name) + if attr is not None: + if isinstance(attr, property) and attr.fset is not None: + attr.fset(self, value) + return + break + raise AttributeError( + f"'{type(self).__name__}' object has no writable attribute '{name}'" + ) %} } @@ -806,6 +843,20 @@ struct libnvme_ns { while ns: yield ns ns = libnvme_subsystem_next_ns(self, ns) + def __setattr__(self, name, value): + if name == 'this' or name.startswith('_'): + object.__setattr__(self, name, value) + return + for klass in type(self).__mro__: + attr = klass.__dict__.get(name) + if attr is not None: + if isinstance(attr, property) and attr.fset is not None: + attr.fset(self, value) + return + break + raise AttributeError( + f"'{type(self).__name__}' object has no writable attribute '{name}'" + ) %} %immutable name; const char *name; @@ -1035,6 +1086,20 @@ struct libnvme_ns { while ns: yield ns ns = libnvme_ctrl_next_ns(self, ns) + def __setattr__(self, name, value): + if name == 'this' or name.startswith('_'): + object.__setattr__(self, name, value) + return + for klass in type(self).__mro__: + attr = klass.__dict__.get(name) + if attr is not None: + if isinstance(attr, property) and attr.fset is not None: + attr.fset(self, value) + return + break + raise AttributeError( + f"'{type(self).__name__}' object has no writable attribute '{name}'" + ) %} } diff --git a/libnvme/libnvme/tests/test-setattr.py b/libnvme/libnvme/tests/test-setattr.py new file mode 100644 index 0000000000..776e054e7d --- /dev/null +++ b/libnvme/libnvme/tests/test-setattr.py @@ -0,0 +1,63 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# This file is part of libnvme. +# Copyright (c) 2026, Dell Technologies Inc. or its subsidiaries. +# Authors: Martin Belanger +"""Tests that __setattr__ guards on SWIG-generated classes raise on bad names.""" + +import unittest +from libnvme import nvme + + +class TestCtrlSetattr(unittest.TestCase): + + def setUp(self): + self.ctx = nvme.global_ctx() + self.ctrl = nvme.ctrl(self.ctx, { + 'subsysnqn': nvme.NVME_DISC_SUBSYS_NAME, + 'transport': 'loop', + }) + + def test_valid_writable_property(self): + """Writing a valid writable property must not raise.""" + self.ctrl.discovery_ctrl = True + + def test_typo_raises(self): + """A typo in a property name must raise AttributeError immediately.""" + with self.assertRaises(AttributeError): + self.ctrl.dhchap_key = 'somekey' # correct name is dhchap_ctrl_key + + def test_readonly_raises(self): + """Writing a read-only (%immutable) property must raise AttributeError.""" + with self.assertRaises(AttributeError): + self.ctrl.transport = 'tcp' + + def test_unknown_attr_raises(self): + """A completely unknown attribute name must raise AttributeError.""" + with self.assertRaises(AttributeError): + self.ctrl.does_not_exist = 42 + + +class TestCtrlDictValidation(unittest.TestCase): + + def setUp(self): + self.ctx = nvme.global_ctx() + + def test_unknown_dict_key_raises(self): + """An unknown key in the constructor dict must raise KeyError.""" + with self.assertRaises(KeyError): + nvme.ctrl(self.ctx, { + 'subsysnqn': nvme.NVME_DISC_SUBSYS_NAME, + 'transport': 'loop', + 'typo_key': 'value', + }) + + def test_missing_required_key_raises(self): + """Omitting a required key (subsysnqn or transport) must raise KeyError.""" + with self.assertRaises(KeyError): + nvme.ctrl(self.ctx, {'transport': 'loop'}) + with self.assertRaises(KeyError): + nvme.ctrl(self.ctx, {'subsysnqn': nvme.NVME_DISC_SUBSYS_NAME}) + + +if __name__ == '__main__': + unittest.main()