From 3f454751fa9a3b920597f56981fc4f0427a8876c Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Wed, 1 Jul 2026 09:24:59 +0000 Subject: [PATCH] errno_from_exception: require int in args fallback so non-errno first args don't silently bypass the comparison errno_from_exception has two branches: it returns the exception's errno attribute when present, otherwise it falls back to args[0]. Every internal caller compares the result against an int errno constant (e.g. errno_from_exception(e) == errno.EBADF), so the args fallback has to return an int. The pre-fix code returned args[0] as-is, which meant a str message (the shape used by most third-party OSError subclasses, by OSError subclasses that only set a human-readable message, and by any value like ValueError or RuntimeError raised with a string) flowed into the int-comparison call sites and silently turned every comparison into False. The real error class (and the right recovery branch) was hidden. Pin the fallback to return args[0] only when it is an int, otherwise None -- which matches the call sites' expectations and surfaces the 'unknown errno' case the same way as the empty-args path. Add six tests in tornado/test/util_test.py:ErrnoFromExceptionTest covering the errno attribute winning over args, an int args[0] being returned, an empty args tuple returning None (the function used to raise TypeError), a non-int args[0] returning None, an int args[1] not being picked up when args[0] is a message, and a message-only exception returning None. --- tornado/test/util_test.py | 56 +++++++++++++++++++++++++++++++++++++++ tornado/util.py | 13 ++++++++- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/tornado/test/util_test.py b/tornado/test/util_test.py index 83b78e5cf..ec792907f 100644 --- a/tornado/test/util_test.py +++ b/tornado/test/util_test.py @@ -10,6 +10,7 @@ from tornado.util import ( ArgReplacer, Configurable, + errno_from_exception, exec_in, import_object, raise_exc_info, @@ -366,3 +367,58 @@ def test_version_info_compatible(self): def test_current_version(self): self.assert_version_info_compatible(tornado.version, tornado.version_info) + + +class ErrnoFromExceptionTest(unittest.TestCase): + """Tests for tornado.util.errno_from_exception. + + errno_from_exception has two branches: it returns the exception's + ``errno`` attribute when present, otherwise it falls back to + ``args[0]``. Every internal caller compares the result against + an int errno constant (e.g. ``errno_from_exception(e) == errno.EBADF``). + The args fallback therefore must return an int, otherwise a + str/float/None in args[0] silently turns every comparison into + False and the real error is hidden from the caller. Each test + below pins one shape of the fallback contract. + """ + + def test_errno_attr_wins_over_args(self): + # The errno attribute takes precedence even if args[0] is + # something else entirely. + e = OSError("not the errno", 5) + e.errno = 42 + self.assertEqual(errno_from_exception(e), 42) + + def test_args0_int_returned_when_no_errno_attr(self): + # A bare exception with an int first arg is the documented + # "use args[0] as the errno" path. + e = ValueError(7) + self.assertEqual(errno_from_exception(e), 7) + + def test_empty_args_returns_none(self): + # An exception with no args must not raise (the function used + # to raise ``TypeError: tuple index out of range`` here). + e = ValueError() + self.assertIsNone(errno_from_exception(e)) + + def test_non_int_args0_returns_none(self): + # The pre-fix behaviour: any non-empty args returned args[0] + # as-is, so a human-readable message (str) flowed into the + # int-comparison call sites. This test would have asserted + # ``errno_from_exception(ValueError("not an errno")) == 7`` + # (always False) and the bug stayed invisible. + e = ValueError("not an errno") + self.assertIsNone(errno_from_exception(e)) + + def test_non_int_args0_with_int_args1_returns_none(self): + # ``args[0]`` is the only fallback slot -- a stray int tucked + # into ``args[1]`` (e.g. by ``OSError(errno, message)``) must + # not be picked up here when ``args[0]`` is a message. + e = ValueError("not an errno", 7) + self.assertIsNone(errno_from_exception(e)) + + def test_message_only_args_returns_none(self): + # The most common real-world shape: an exception whose only + # argument is a human-readable message and no errno attribute. + e = RuntimeError("connection broken") + self.assertIsNone(errno_from_exception(e)) diff --git a/tornado/util.py b/tornado/util.py index 37b595e0b..633cc8f67 100644 --- a/tornado/util.py +++ b/tornado/util.py @@ -164,7 +164,18 @@ def errno_from_exception(e: BaseException) -> int | None: if hasattr(e, "errno"): return e.errno # type: ignore elif e.args: - return e.args[0] + # Some third-party exceptions subclass OSError but only set the + # human-readable message (a str) as the first argument rather + # than an int errno. Returning the raw value would let it flow + # into call sites that compare it to int errno constants + # (e.g. ``errno_from_exception(exc) == errno.EBADF``) and + # silently turn every comparison into False, which masks the + # error class instead of reporting it. Require an int so callers + # fall through to their "not a recognised errno" branch. + arg = e.args[0] + if isinstance(arg, int): + return arg + return None else: return None