From 73a03326ceac24031b0573ebe1ce0246ea3bb3c8 Mon Sep 17 00:00:00 2001 From: David de Hilster Date: Sun, 11 Oct 2026 21:56:58 -0400 Subject: [PATCH] fix: run the engine in the "C" locale so Windows keeps UTF-8 whole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, Python sets LC_CTYPE to the user's code page (e.g. 1252) at startup. The engine tests UTF-8 a byte at a time with , and in cp1252 byte 0xA0 is a no-break space, so the continuation byte 0xA0 of "à" (C3 A0) or "전" (EC A0 84) read as whitespace: clean_str() (used by phrasetext() and friends) cut words mid-character and wrote invalid UTF-8. nlp.exe (C locale) and Linux (UTF-8 locale) were unaffected. Wrap the constructor, analyze() and compile() in a per-thread guard that switches to the "C" locale and restores the caller's on return (_configthreadlocale on Windows, uselocale elsewhere). Add tests/test_locale.py, which forces a single-byte locale, analyzes "Voilà" and "오전" with parse-en-us and checks the output is valid UTF-8 with both words whole and the caller's locale restored. Run it against every repaired Windows wheel in publish.yml. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/publish.yml | 4 ++ bindings.cpp | 79 ++++++++++++++++++++++++++++++++++- tests/test_locale.py | 70 +++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 tests/test_locale.py diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 7c82773..cc148bf 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -141,6 +141,10 @@ jobs: # NOTE: bump "78" if the vcpkg ICU major version changes. CIBW_REPAIR_WHEEL_COMMAND_WINDOWS: "delvewheel repair --no-mangle-all --add-dll icuin78.dll --add-path ${{ github.workspace }}/vcpkg/installed/x64-windows-release/bin -w {dest_dir} {wheel}" CIBW_CONFIG_SETTINGS: "cmake.args='-A x64;-DCMAKE_BUILD_TYPE=Release;-DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake'" + # Run against the repaired wheel under code page 1252, where the + # engine used to split UTF-8 characters at byte 0xA0. Linux CI has no + # single-byte locale, so this is the job that catches a regression. + CIBW_TEST_COMMAND_WINDOWS: "python -m unittest discover -s {project}/tests -p test_locale.py -v" - name: Upload artifact id: upload diff --git a/bindings.cpp b/bindings.cpp index 7ade743..17ec567 100644 --- a/bindings.cpp +++ b/bindings.cpp @@ -1,5 +1,14 @@ #include #include +#include +#include +#include +#ifndef _WIN32 +#include +#ifdef __APPLE__ +#include +#endif +#endif #include #include @@ -14,6 +23,72 @@ namespace nb = nanobind; using namespace nb::literals; +/** + * Run the engine in the "C" locale for the lifetime of this object. + * + * The engine walks UTF-8 text a byte at a time through + * (isspace, isalpha, toupper, ...), whose answers depend on the + * locale. nlp.exe runs in the default "C" locale, but Python sets + * LC_CTYPE from the user's environment at startup. On Windows that + * is a code page such as 1252, where 0xA0 is a no-break space, so the + * continuation byte 0xA0 of "à" or "전" reads as whitespace and + * clean_str() splits the character in half. + * + * The switch is per thread (_configthreadlocale on Windows, uselocale + * elsewhere) and the caller's locale is restored on the way out, so + * other Python threads never see it. + */ +class CLocaleGuard { +public: + CLocaleGuard() { +#ifdef _WIN32 + prevMode_ = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); + const char *cur = setlocale(LC_ALL, nullptr); + if (cur) + prevLocale_ = cur; + setlocale(LC_ALL, "C"); +#else + cLocale_ = newlocale(LC_ALL_MASK, "C", (locale_t)0); + if (cLocale_) + prevLocale_ = uselocale(cLocale_); +#endif + } + ~CLocaleGuard() { +#ifdef _WIN32 + if (!prevLocale_.empty()) + setlocale(LC_ALL, prevLocale_.c_str()); + _configthreadlocale(prevMode_); +#else + if (cLocale_) { + uselocale(prevLocale_); + freelocale(cLocale_); + } +#endif + } + CLocaleGuard(const CLocaleGuard &) = delete; + CLocaleGuard &operator=(const CLocaleGuard &) = delete; + +private: +#ifdef _WIN32 + int prevMode_ = 0; + std::string prevLocale_; +#else + locale_t cLocale_ = (locale_t)0; + locale_t prevLocale_ = (locale_t)0; +#endif +}; + +/** + * Construct the engine under CLocaleGuard: init loads the analyzer + * KBs, which goes through the same locale-sensitive byte tests. + */ +void +wrap_init(NLP_ENGINE *self, const std::string &workingFolder, + const bool silent) { + CLocaleGuard cLocale; + new (self) NLP_ENGINE(workingFolder, silent); +} + /** * Wrap the analyze() method to deal with C++ friction. * @@ -34,6 +109,7 @@ const std::string wrap_analyze(NLP_ENGINE &engine, const std::string &parser, const std::string &input, const bool develop, const bool compiled) { + CLocaleGuard cLocale; _TCHAR *_parser = _tcsdup(parser.c_str()); std::istringstream instream(input); std::ostringstream outstream; @@ -69,6 +145,7 @@ wrap_analyze(NLP_ENGINE &engine, const std::string &parser, void wrap_compile(NLP_ENGINE &engine, const std::string &analyzer, const bool develop, const bool kbOnly, const bool analyzerOnly) { + CLocaleGuard cLocale; _TCHAR *_analyzer = _tcsdup(analyzer.c_str()); engine.init(_analyzer, develop, /*silent*/false, /*compile*/(!kbOnly && !analyzerOnly), /*compiled*/false, @@ -102,7 +179,7 @@ NB_MODULE(bindings, m) { m.def("engine_version", &wrap_engine_version, "Return the bundled nlp-engine version string (e.g. '3.1.49')."); nb::class_(m, "NLP_ENGINE", "Instance of the NLP++ Engine.") - .def(nb::init(), + .def("__init__", &wrap_init, "workingFolder"_a = ".", "silent"_a = true) .def("analyze", &wrap_analyze, diff --git a/tests/test_locale.py b/tests/test_locale.py new file mode 100644 index 0000000..c8bf2ac --- /dev/null +++ b/tests/test_locale.py @@ -0,0 +1,70 @@ +"""The engine must not depend on the caller's C locale. + +The engine reads UTF-8 a byte at a time through . Under a +single-byte locale such as Windows code page 1252 (what Python sets on +Windows at startup), byte 0xA0 is a no-break space, so the continuation +byte 0xA0 in "à" (C3 A0) or "전" (EC A0 84) used to be taken for +whitespace: words were cut mid-character and the output was no longer +valid UTF-8. The binding now runs the engine in the "C" locale. +""" + +import locale +import unittest + +try: + import NLPPlus + HAVE_NLPPLUS = True +except Exception: # native binding not built in this environment + HAVE_NLPPLUS = False + +ANALYZER = "parse-en-us" # first pass is dicttokz; phrasetext() runs clean_str +TEXT = "Voilà, the form. 오전 9시.\n" + +# A locale in which isspace(0xA0) is true. +SINGLE_BYTE_LOCALES = [".1252", "English_United States.1252", + "en_US.ISO-8859-1", "en_US.ISO8859-1", "en_US"] + + +def set_single_byte_locale(): + for name in SINGLE_BYTE_LOCALES: + try: + locale.setlocale(locale.LC_CTYPE, name) + except locale.Error: + continue + if "utf" not in locale.setlocale(locale.LC_CTYPE).lower(): + return True + return False + + +@unittest.skipUnless(HAVE_NLPPLUS, "NLPPlus native binding not built") +class TestLocale(unittest.TestCase): + def setUp(self): + self.saved = locale.setlocale(locale.LC_CTYPE) + if not set_single_byte_locale(): + locale.setlocale(locale.LC_CTYPE, self.saved) + self.skipTest("no single-byte locale available") + self.ctype = locale.setlocale(locale.LC_CTYPE) + + def tearDown(self): + locale.setlocale(locale.LC_CTYPE, self.saved) + + def test_multibyte_words_survive_single_byte_locale(self): + with NLPPlus.Engine() as engine: + # Raised UnicodeDecodeError while the bug was present. + results = engine.analyze(TEXT, ANALYZER) + for path in results.outdir.iterdir(): + with self.subTest(file=path.name): + path.read_bytes().decode("utf-8") + # One line per clause, its text passed through clean_str(). + clauses = (results.outdir / "clause.txt").read_text(encoding="utf-8") + self.assertIn("Voilà, the form", clauses) + self.assertIn("오전 9시", clauses) + + def test_caller_locale_restored(self): + with NLPPlus.Engine() as engine: + engine.analyze(TEXT, ANALYZER) + self.assertEqual(locale.setlocale(locale.LC_CTYPE), self.ctype) + + +if __name__ == "__main__": + unittest.main()