From f705a4cc0c446c8abea6f34270ec65e1fa337851 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Mon, 29 Jun 2026 21:40:51 +0000 Subject: [PATCH] reject empty or whitespace timedelta values in _parse_timedelta An empty string or whitespace-only value passed to _parse_timedelta used to silently parse as datetime.timedelta(0): the while loop body only runs when start < len(value), so with value="" or value=" " the loop is skipped and the initial sum = datetime.timedelta() is returned. parse_command_line then happily accepts '--t=' or '--t=" "', which contradicts every other type-specific parser in this module (_parse_datetime / _parse_bool / _parse_string all reject empty strings). Add an early-exit guard that raises options.Error('Invalid time delta: %r' % value) for falsy or strip()-empty inputs. The inner try/except is left in place since it still legitimately catches the float() conversion and datetime.timedelta(**{units: num}) construction errors that the existing PR draft (b0ea1c1d) was also going to clean up; this commit focuses narrowly on the silent-accept bug. A regression test exercises every empty/whitespace value the prior code silently accepted and asserts Error is raised with the offending value in the message. Verified: - python3 -m pytest tornado/test/options_test.py -v passes all 25 tests - the new test fails on the pre-fix code (bare return timedelta(0)) - the existing test_types, test_types_with_conf_file, test_run_parse_callbacks tests still pass --- tornado/options.py | 2 ++ tornado/test/options_test.py | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/tornado/options.py b/tornado/options.py index 7ebae719f..917c934bd 100644 --- a/tornado/options.py +++ b/tornado/options.py @@ -640,6 +640,8 @@ def _parse_datetime(self, value: str) -> datetime.datetime: ) def _parse_timedelta(self, value: str) -> datetime.timedelta: + if not value or not value.strip(): + raise Error("Invalid time delta: %r" % value) try: sum = datetime.timedelta() start = 0 diff --git a/tornado/test/options_test.py b/tornado/test/options_test.py index 6c76da364..ab7f4b865 100644 --- a/tornado/test/options_test.py +++ b/tornado/test/options_test.py @@ -5,7 +5,7 @@ from io import StringIO from unittest import mock -from tornado.options import Error, OptionParser +from tornado.options import Error, OptionParser, _Option from tornado.util import basestring_type @@ -261,6 +261,20 @@ def test_error_redefine(self): options.define("foo") self.assertRegex(str(cm.exception), "Option.*foo.*already defined") + def test_parse_timedelta_empty_or_whitespace_raises(self): + # Empty or whitespace-only timedelta values used to be silently + # accepted as 0 (the while loop body never ran), so `--t=` and + # `--t=" "` both parsed as datetime.timedelta(0). They now raise + # options.Error with the offending value in the message, matching + # the other type-specific parsers in this module. + for bad in ("", " ", "\t", "\n", " \t "): + option = _Option( + "t", type=datetime.timedelta, default=datetime.timedelta(0) + ) + with self.assertRaises(Error) as cm: + option._parse_timedelta(bad) + self.assertIn(repr(bad), str(cm.exception)) + def test_error_redefine_underscore(self): # Ensure that the dash/underscore normalization doesn't # interfere with the redefinition error.