pythongh-131434: trailing % in strptime format strings should raise an error

duaneg · duaneg · commit 8c386e06bdd3 · 2025-03-22T16:12:29.000+13:00
Treat trailing % at the end of a format string consistently with % followed by
spaces inside a format string. The IndexError which was previously raised when
there was a trailing % can no longer happen, so we can remove the dead code
dealing with it.

Note the ValueError's message in this case has changed from "stray % in format
'...'" to "'%' is a bad directive in format '...'". We could treat this as a
special case and retain the previous wording if desired, but ISTM that
consistency is better than strict backwards compatibility here.
diff --git a/Lib/_strptime.py b/Lib/_strptime.py
@@ -365,7 +365,7 @@ def repl(m):
                     nonlocal day_of_month_in_format
                     day_of_month_in_format = True
             return self[format_char]
-        format = re_sub(r'%(O?.)', repl, format)
+        format = re_sub(r'%(O?.)?', repl, format)
         if day_of_month_in_format and not year_in_format:
             import warnings
             warnings.warn("""\
@@ -436,17 +436,15 @@ def _strptime(data_string, format="%a %b %d %H:%M:%S %Y"):
             try:
                 format_regex = _TimeRE_cache.compile(format)
             # KeyError raised when a bad format is found; can be specified as
-            # \\, in which case it was a stray % but with a space after it
+            # \\, in which case it was a stray % but with a space after it, or
+            # None, in which case it was a trailing %
             except KeyError as err:
                 bad_directive = err.args[0]
-                if bad_directive == "\\":
+                if bad_directive is None or bad_directive == "\\":
                     bad_directive = "%"
                 del err
                 raise ValueError("'%s' is a bad directive in format '%s'" %
                                     (bad_directive, format)) from None
-            # IndexError only occurs when the format string is "%"
-            except IndexError:
-                raise ValueError("stray %% in format '%s'" % format) from None
             _regex_cache[format] = format_regex
     found = format_regex.match(data_string)
     if not found:
diff --git a/Lib/test/test_strptime.py b/Lib/test/test_strptime.py
@@ -286,10 +286,11 @@ def test_strptime_exception_context(self):
         with self.assertRaises(ValueError) as e:
             _strptime._strptime_time('', '%D')
         self.assertIs(e.exception.__suppress_context__, True)
-        # additional check for IndexError branch (issue #19545)
+
+        # additional check for trailing % (issue #19545)
         with self.assertRaises(ValueError) as e:
             _strptime._strptime_time('19', '%Y %')
-        self.assertIsNone(e.exception.__context__)
+        self.assertIs(e.exception.__suppress_context__, True)
 
     def test_unconverteddata(self):
         # Check ValueError is raised when there is unconverted data
@@ -592,6 +593,24 @@ def test_percent(self):
                          strp_output[1] == self.time_tuple[1],
                         "handling of percent sign failed")
 
+    def test_lone_percent(self):
+        # Trailing escaped %s should work
+        strptime = _strptime._strptime
+        self.assertEqual(strptime('2000 %', '%Y %%')[0][0], 2000)
+        self.assertEqual(strptime('2000 %%', '%Y %%%%')[0][0], 2000)
+
+        def test(tstr, format):
+            with self.assertRaises(ValueError) as err:
+                strptime(tstr, format)
+            self.assertEqual("'%' is a bad directive in format '{}'".format(format),
+                             str(err.exception))
+
+        # A lone or unescaped trailing % should not
+        test("%", "%")
+        test("2000 %", "%Y %")
+        test("2000 % %", "%Y % %%")
+        test("2000 %%", "%Y %%%")
+
     def test_caseinsensitive(self):
         # Should handle names case-insensitively.
         strf_output = time.strftime("%B", self.time_tuple)
