Rework locale encoding C functions (#158680)

Add output_length to _Py_EncodeLocale(), _Py_EncodeUTF8(),
encode_current_locale() and encode_ascii(). So unicode_encode_locale() and
wstr_to_utf8() can now use the output_length, instead of having to compute
strlen().

* Rename functions:

  * _Py_DecodeUTF8Ex() => _Py_DecodeUTF8()
  * _Py_EncodeUTF8Ex() => _Py_EncodeUTF8()
  * _Py_EncodeLocaleEx() => _Py_EncodeLocale()
  * _Py_DecodeLocaleEx() => _Py_DecodeLocale()

* Add tests on Py_DecodeLocale() and Py_EncodeLocale() in test_codecs.
  Clarify also their documentation.
* _Py_EncodeLocale() and _Py_DecodeLocale() now sets all output parameters on
  error. Add more tests on these output parameters.
* Add encode_current_locale_impl() to simplify encode_current_locale().
* Remove reason parameter of _Py_EncodeLocale() and _Py_DecodeLocale() to
  simplify the code. Only the UTF-8 decoder used specific reasons.
* _Py_EncodeUTF8() now uses size_t instead of Py_ssize_t to iterate
  on the input string.
* On error, encode_current_locale_impl() now returns ENCODE_ERROR (new
  constant) instead of DECODE_ERROR.
* Mark _Py_EncodeLocaleRaw() as static (it's not used outside fileutils.c).

Author: vstinner

Doc/c-api/sys.rst

@@ -152,28 +152,29 @@ Operating System Utilities
       <c-preinit>` and so that the LC_CTYPE locale is properly configured: see
       the :c:func:`Py_PreInitialize` function.
 
-   Decode a byte string from the :term:`filesystem encoding and error handler`.
-   If the error handler is :ref:`surrogateescape error handler
-   <surrogateescape>`, undecodable bytes are decoded as characters in range
-   U+DC80..U+DCFF; and if a byte sequence can be decoded as a surrogate
-   character, the bytes are escaped using the surrogateescape error handler
-   instead of decoding them.
+   Decode a byte string from the :term:`filesystem encoding <filesystem
+   encoding and error handler>` with the :ref:`surrogateescape error handler
+   <surrogateescape>`.
+
+   Undecodable bytes are decoded as characters in range U+DC80..U+DCFF. If a
+   byte sequence can be decoded as a surrogate character, escape the bytes
+   using the surrogateescape error handler instead of decoding them.
 
    Return a pointer to a newly allocated wide character string, use
    :c:func:`PyMem_RawFree` to free the memory. If size is not ``NULL``, write
    the number of wide characters excluding the null character into ``*size``
 
-   Return ``NULL`` on decoding error or memory allocation error. If *size* is
-   not ``NULL``, ``*size`` is set to ``(size_t)-1`` on memory error or set to
-   ``(size_t)-2`` on decoding error.
+   On memory allocation failure, set *\*size* to ``(size_t)-1`` and return
+   ``NULL``.
+
+   On decode error, set *\*size* to ``(size_t)-2`` and return ``NULL``.
+   Decoding errors should never happen, unless there is a bug in the C
+   library.
 
    The :term:`filesystem encoding and error handler` are selected by
    :c:func:`PyConfig_Read`: see :c:member:`~PyConfig.filesystem_encoding` and
    :c:member:`~PyConfig.filesystem_errors` members of :c:type:`PyConfig`.
 
-   Decoding errors should never happen, unless there is a bug in the C
-   library.
-
    Use the :c:func:`Py_EncodeLocale` function to encode the character string
    back to a byte string.
 
@@ -195,17 +196,19 @@ Operating System Utilities
 
 .. c:function:: char* Py_EncodeLocale(const wchar_t *text, size_t *error_pos)
 
-   Encode a wide character string to the :term:`filesystem encoding and error
-   handler`. If the error handler is :ref:`surrogateescape error handler
-   <surrogateescape>`, surrogate characters in the range U+DC80..U+DCFF are
-   converted to bytes 0x80..0xFF.
+   Encode a wide character string to the :term:`filesystem encoding <filesystem
+   encoding and error handler>` with the :ref:`surrogateescape error handler
+   <surrogateescape>`. Surrogate characters in the range U+DC80..U+DCFF are
+   encoded to bytes 0x80..0xFF.
 
    Return a pointer to a newly allocated byte string, use :c:func:`PyMem_Free`
-   to free the memory. Return ``NULL`` on encoding error or memory allocation
-   error.
+   to free the memory.
+
+   On memory allocation failure, set *\*error_pos* to ``(size_t)-1`` and return
+   ``NULL``.
 
-   If error_pos is not ``NULL``, ``*error_pos`` is set to ``(size_t)-1`` on
-   success,  or set to the index of the invalid character on encoding error.
+   On encoding error, set *\*error_pos* to the index of the first unencodable
+   character and return ``NULL``.
 
    The :term:`filesystem encoding and error handler` are selected by
    :c:func:`PyConfig_Read`: see :c:member:`~PyConfig.filesystem_encoding` and

Include/internal/pycore_fileutils.h

@@ -24,27 +24,22 @@ extern "C" {
 PyAPI_FUNC(_Py_error_handler) _Py_GetErrorHandler(const char *errors);
 
 // Export for '_testinternalcapi' shared extension
-PyAPI_FUNC(int) _Py_DecodeLocaleEx(
+PyAPI_FUNC(int) _Py_DecodeLocale(
     const char *arg,
     wchar_t **wstr,
     size_t *wlen,
-    const char **reason,
     int current_locale,
     _Py_error_handler errors);
 
 // Export for '_testinternalcapi' shared extension
-PyAPI_FUNC(int) _Py_EncodeLocaleEx(
+PyAPI_FUNC(int) _Py_EncodeLocale(
     const wchar_t *text,
     char **str,
+    size_t *output_length,
     size_t *error_pos,
-    const char **reason,
     int current_locale,
     _Py_error_handler errors);
 
-extern char* _Py_EncodeLocaleRaw(
-    const wchar_t *text,
-    size_t *error_pos);
-
 extern PyObject* _Py_device_encoding(int);
 
 #if defined(MS_WINDOWS) || defined(__APPLE__)
@@ -190,19 +185,23 @@ extern int _Py_open_osfhandle(void *handle, int flags);
      ? _PyStatus_ERR("cannot decode " NAME) \
      : _PyStatus_NO_MEMORY()
 
-extern int _Py_DecodeUTF8Ex(
+#define _Py_CODEC_MEMORY_ERROR -1
+#define _Py_CODEC_DECODE_ERROR -2
+#define _Py_CODEC_ENCODE_ERROR -2
+#define _Py_CODEC_UNSUPPORTED_ERROR_HANDLER -3
+
+extern int _Py_DecodeUTF8(
     const char *arg,
     Py_ssize_t arglen,
     wchar_t **wstr,
     size_t *wlen,
-    const char **reason,
     _Py_error_handler errors);
 
-extern int _Py_EncodeUTF8Ex(
+extern int _Py_EncodeUTF8(
     const wchar_t *text,
     char **str,
+    size_t *output_length,
     size_t *error_pos,
-    const char **reason,
     int raw_malloc,
     _Py_error_handler errors);
 

Lib/test/test_codecs.py

@@ -4057,34 +4057,67 @@ def test_pickle(self):
                     pickle.dumps(sr, proto)
 
 
+@unittest.skipIf(_testlimitedcapi is None, 'need _testlimitedcapi module')
 @unittest.skipIf(_testinternalcapi is None, 'need _testinternalcapi module')
 class LocaleCodecTest(unittest.TestCase):
     """
-    Test indirectly _Py_DecodeUTF8Ex() and _Py_EncodeUTF8Ex().
+    Test public Py_EncodeLocale() and Py_DecodeLocale() C API.
+
+    Test internal _Py_EncodeLocale() and _Py_DecodeLocale() C API.
+
+    Test indirectly _Py_DecodeUTF8() and _Py_EncodeUTF8().
     """
     ENCODING = sys.getfilesystemencoding()
     STRINGS = ("ascii", "ulatin1:\xa7\xe9",
                "u255:\xff",
                "UCS:\xe9\u20ac\U0010ffff",
-               "surrogates:\uDC80\uDCFF")
+               "surrogates:\uDC80\uDCFF",
+               "embed\0char")
     BYTES_STRINGS = (b"blatin1:\xa7\xe9", b"b255:\xff")
-    SURROGATES = "\uDC80\uDCFF"
 
-    def encode(self, text, errors="strict"):
-        return _testinternalcapi.EncodeLocaleEx(text, 0, errors)
+    def encode_locale_surrogateescape(self, text):
+        # Test public Py_EncodeLocale() C API:
+        # use the "surrogateescape" error handler
+        return _testlimitedcapi.encode_locale(text)
+
+    def encode_locale(self, text, errors="strict"):
+        # Test internal _Py_EncodeLocale() C API
+        return _testinternalcapi.encode_locale(text, 0, errors)
 
     def check_encode_strings(self, errors):
         for text in self.STRINGS:
             with self.subTest(text=text):
                 try:
                     expected = text.encode(self.ENCODING, errors)
+                    if b"\0" in expected:
+                        # Py_EncodeLocale() and _Py_EncodeLocale()
+                        # truncate the input string at the first NUL character
+                        expected = expected.partition(b'\0')[0]
                 except UnicodeEncodeError:
+                    for error_pos in range(len(text)):
+                        try:
+                            text[error_pos].encode(self.ENCODING, errors)
+                        except UnicodeEncodeError:
+                            break
+                    else:
+                        self.fail("failed to compute error_pos")
+
+                    if errors == "surrogateescape":
+                        with self.assertRaises(RuntimeError) as cm:
+                            self.encode_locale_surrogateescape(text)
+                        errmsg = f"encode error: pos={error_pos}"
+                        self.assertEqual(str(cm.exception), errmsg)
+
                     with self.assertRaises(RuntimeError) as cm:
-                        self.encode(text, errors)
-                    errmsg = str(cm.exception)
-                    self.assertRegex(errmsg, r"encode error: pos=[0-9]+, reason=")
+                        self.encode_locale(text, errors)
+                    errmsg = f"encode error: pos={error_pos}"
+                    self.assertEqual(str(cm.exception), errmsg)
                 else:
-                    encoded = self.encode(text, errors)
+                    if errors in ("strict", "surrogateescape"):
+                        encoded = self.encode_locale_surrogateescape(text)
+                        self.assertEqual(encoded, expected)
+
+                    encoded = self.encode_locale(text, errors)
                     self.assertEqual(encoded, expected)
 
     def test_encode_strict(self):
@@ -4095,7 +4128,7 @@ def test_encode_surrogateescape(self):
 
     def test_encode_surrogatepass(self):
         try:
-            self.encode('', 'surrogatepass')
+            self.encode_locale('', 'surrogatepass')
         except ValueError as exc:
             if str(exc) == 'unsupported error handler':
                 self.skipTest(f"{self.ENCODING!r} encoder doesn't support "
@@ -4107,11 +4140,17 @@ def test_encode_surrogatepass(self):
 
     def test_encode_unsupported_error_handler(self):
         with self.assertRaises(ValueError) as cm:
-            self.encode('', 'backslashreplace')
+            self.encode_locale('', 'backslashreplace')
         self.assertEqual(str(cm.exception), 'unsupported error handler')
 
-    def decode(self, encoded, errors="strict"):
-        return _testinternalcapi.DecodeLocaleEx(encoded, 0, errors)
+    def decode_locale(self, encoded, errors="strict"):
+        # Test internal _Py_DecodeLocale() C API
+        return _testinternalcapi.decode_locale(encoded, 0, errors)
+
+    def decode_locale_surrogateescape(self, encoded):
+        # Test the public Py_DecodeLocale() C API:
+        # use the "surrogateescape" error handler
+        return _testlimitedcapi.decode_locale(encoded)
 
     def check_decode_strings(self, errors):
         is_utf8 = (self.ENCODING == "utf-8")
@@ -4138,13 +4177,37 @@ def check_decode_strings(self, errors):
             with self.subTest(encoded=encoded):
                 try:
                     expected = encoded.decode(self.ENCODING, errors)
+                    if "\0" in expected:
+                        # Py_DecodeLocale() and _Py_DecodeLocale() truncate
+                        # the input string at the first NUL byte
+                        expected = expected.partition('\0')[0]
                 except UnicodeDecodeError:
+                    for error_pos in range(len(encoded) - 1, -1, -1):
+                        try:
+                            encoded[:error_pos].decode(self.ENCODING, errors)
+                        except UnicodeDecodeError:
+                            pass
+                        else:
+                            break
+                    else:
+                        self.fail("failed to compute error_pos")
+
+                    if errors == "surrogateescape":
+                        with self.assertRaises(RuntimeError) as cm:
+                            self.decode_locale_surrogateescape(encoded)
+                        errmsg = f"Py_DecodeLocale failed: error_pos={error_pos}"
+                        self.assertEqual(str(cm.exception), errmsg)
+
                     with self.assertRaises(RuntimeError) as cm:
-                        self.decode(encoded, errors)
-                    errmsg = str(cm.exception)
-                    self.assertStartsWith(errmsg, "decode error: ")
+                        self.decode_locale(encoded, errors)
+                    errmsg = f"decode error: pos={error_pos}"
+                    self.assertEqual(str(cm.exception), errmsg)
                 else:
-                    decoded = self.decode(encoded, errors)
+                    if errors == ("strict", "surrogateescape"):
+                        decoded = self.decode_locale_surrogateescape(encoded)
+                        self.assertEqual(decoded, expected)
+
+                    decoded = self.decode_locale(encoded, errors)
                     self.assertEqual(decoded, expected)
 
     def test_decode_strict(self):
@@ -4155,7 +4218,7 @@ def test_decode_surrogateescape(self):
 
     def test_decode_surrogatepass(self):
         try:
-            self.decode(b'', 'surrogatepass')
+            self.decode_locale(b'', 'surrogatepass')
         except ValueError as exc:
             if str(exc) == 'unsupported error handler':
                 self.skipTest(f"{self.ENCODING!r} decoder doesn't support "
@@ -4167,7 +4230,7 @@ def test_decode_surrogatepass(self):
 
     def test_decode_unsupported_error_handler(self):
         with self.assertRaises(ValueError) as cm:
-            self.decode(b'', 'backslashreplace')
+            self.decode_locale(b'', 'backslashreplace')
         self.assertEqual(str(cm.exception), 'unsupported error handler')