diff --git a/docs/changes.rst b/docs/changes.rst index 058bf032..155835b8 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -11,6 +11,13 @@ Version 2.6.0 **Bugs Fixed** +* Calling ``wrapt.BoundFunctionWrapper`` with a non-string binding value + could crash the interpreter when using the C extension. The call path + now checks that the value is a string before comparing the binding + names with the Unicode-only C API. With thanks to theosotr for the + report in `issue #368 + `_. + * When ``wrapt.lru_cache`` was applied to a class method, the single shared cache was created from the method as bound to whichever class first called it. Calls made later through a subclass, or via an diff --git a/src/wrapt/_wrappers.c b/src/wrapt/_wrappers.c index af1868de..e25463a4 100644 --- a/src/wrapt/_wrappers.c +++ b/src/wrapt/_wrappers.c @@ -5116,9 +5116,9 @@ WraptBoundFunctionWrapper_call(WraptFunctionWrapperObject *self, PyObject *args, * wrapping an instance method vs a static method or class method. */ - matched = - PyUnicode_CompareWithASCIIString(binding, "function") == 0 || - PyUnicode_CompareWithASCIIString(binding, "callable") == 0; + matched = PyUnicode_Check(binding) && + (PyUnicode_CompareWithASCIIString(binding, "function") == 0 || + PyUnicode_CompareWithASCIIString(binding, "callable") == 0); if (matched) { diff --git a/tests/core/test_function_wrapper.py b/tests/core/test_function_wrapper.py index 5a0519c3..57374de5 100644 --- a/tests/core/test_function_wrapper.py +++ b/tests/core/test_function_wrapper.py @@ -562,6 +562,65 @@ def wrapper(wrapped, instance, args, kwargs): self.assertTrue(_bound_wrapper_1 is not _bound_wrapper_2) +class TestBindingValues(unittest.TestCase): + + def check_fallback_binding(self, wrapped, expected_instance, binding): + calls = [] + + def wrapper(wrapped, instance, args, kwargs): + calls.append((wrapped, instance, args, kwargs)) + return "ok" + + bound = wrapt.BoundFunctionWrapper( + wrapped, object(), wrapper, None, binding + ) + + self.assertEqual(bound("value", flag=True), "ok") + self.assertEqual(len(calls), 1) + self.assertIs(calls[0][0], wrapped) + self.assertIs(calls[0][1], expected_instance) + self.assertEqual(calls[0][2], ("value",)) + self.assertEqual(calls[0][3], {"flag": True}) + + def test_non_string_binding_free_function(self): + def function(): + pass + + for binding in (object(), None, 0, False, b"function", [], {}): + with self.subTest(binding_type=type(binding).__name__): + self.check_fallback_binding(function, None, binding) + + def test_non_string_binding_class_method(self): + class Target: + @classmethod + def method(cls): + pass + + for binding in (object(), None, 0, False, b"function", [], {}): + with self.subTest(binding_type=type(binding).__name__): + self.check_fallback_binding(Target.method, Target, binding) + + def test_unknown_string_binding_keeps_fallback(self): + def function(): + pass + + self.check_fallback_binding(function, None, "custom") + + def test_disabled_non_string_binding_calls_wrapped(self): + def function(value): + return value + + def wrapper(wrapped, instance, args, kwargs): + self.fail("Disabled wrapper must not be called") + + for enabled in (False, lambda: False): + with self.subTest(enabled=enabled): + bound = wrapt.BoundFunctionWrapper( + function, None, wrapper, enabled, object() + ) + self.assertEqual(bound("value"), "value") + + class TestInvalidWrapper(unittest.TestCase): def test_none_for_wrapped(self):