gh-127527: Improve error handling in time_strftime1

Doc/library/datetime.rst

@@ -2638,11 +2638,15 @@ For :class:`date` objects, the format codes for hours, minutes, seconds, and
 microseconds should not be used, as :class:`date` objects have no such
 values. If they're used anyway, 0 is substituted for them.
 
-For the same reason, handling of format strings containing Unicode code points
-that can't be represented in the charset of the current locale is also
-platform-dependent. On some platforms such code points are preserved intact in
-the output, while on others ``strftime`` may raise :exc:`UnicodeError` or return
-an empty string instead.
+If a format directive is unrecognized, the behavior is platform-dependent. glibc
+will return the directive unmodified, Windows will raise a :exc:`ValueError`,
+and macOS, musl, and BSD will return an empty string.
+
+Handling of format strings containing Unicode code points that can't be
+represented in the charset of the current locale is also platform-dependent. On
+some platforms such code points are preserved intact in the output, while on
+others ``strftime`` may raise :exc:`UnicodeError` or return an empty string
+instead.
 
 Notes:
 

Doc/library/time.rst

@@ -607,6 +607,10 @@ Functions
    and thus does not necessarily support all directives available that are not
    documented as supported.
 
+   If a format directive is unrecognized, the behavior is platform-dependent.
+   glibc will return the directive unmodified, Windows will raise a
+   :exc:`ValueError`, and macOS, musl, and BSD will return an empty string.
+
 
 .. class:: struct_time
 

Misc/NEWS.d/next/Library/2024-12-03-11-53-56.gh-issue-127527.hoM7oD.rst

@@ -0,0 +1,2 @@
+Made minor improvements to the error handling in ``time.strftime`` and
+documented the behavior when an unknown directive is seen

Modules/timemodule.c

@@ -837,20 +837,38 @@ time_strftime1(time_char **outbuf, size_t *bufsize,
             return NULL;
         }
 #endif
-        if (buflen == 0 && *bufsize < 256 * fmtlen) {
-            *bufsize += *bufsize;
-            continue;
+        if ((*outbuf)[buflen] == '\0') {
+#ifdef HAVE_WCSFTIME
+            return PyUnicode_FromWideChar(*outbuf, buflen);
+#else
+            return PyUnicode_DecodeLocaleAndSize(*outbuf, buflen, "surrogateescape");
+#endif
+        }
+        if (buflen != 0) {
+            // I believe this is unreachable in glibc, musl, and BSD.
+            PyErr_SetString(PyExc_SystemError, "Unexpected behavior from strftime");
+            return NULL;
         }
-        /* If the buffer is 256 times as long as the format,
-           it's probably not failing for lack of room!
-           More likely, the format yields an empty result,
-           e.g. an empty format, or %Z when the timezone
-           is unknown. */
+        if (*bufsize >= 256 * fmtlen) {
+            // If the buffer is 256 times as long as the format, it's probably
+            // not failing for lack of room! More likely, `format_time` doesn't
+            // like the format string.
+            // I believe that this is unreachable in glibc, but both musl and
+            // BSD can end up here.
 #ifdef HAVE_WCSFTIME
-        return PyUnicode_FromWideChar(*outbuf, buflen);
+            // For backwards compatibility, return empty string instead of
+            // raising a ValueError.
+            return PyUnicode_FromStringAndSize(NULL, 0);
 #else
-        return PyUnicode_DecodeLocaleAndSize(*outbuf, buflen, "surrogateescape");
+            // Previously we raised ValueError("embedded null byte") here, so
+            // this is backwards compatible as long as we are concerned only
+            // about error type.
+            PyErr_SetString(PyExc_ValueError, "Invalid format string");
+            return NULL;
 #endif
+
+        }
+        *bufsize += *bufsize;
     }
 }