BUG: NrrdIO reads and writes floats locale-independently

All NRRD float parsing funnels through airSingleSscanf() and all float
formatting (header fields and ASCII pixels) through airSinglePrintf().
Both honored the ambient LC_NUMERIC locale: under a decimal-comma locale
(e.g. de_DE.UTF-8) "0.878906" parsed as 0, and the writer emitted
"0,878906", corrupting NRRD spacings, directions, origins, and
ASCII-encoded pixel values in both directions.

Parse with strtod_l() and print with the printf_l() family (glibc/musl
lack printf_l; there printf honors the uselocale() thread-local locale,
so swap it around the print) against one cached "C" locale shared via
privateAir.h. Overflow (ERANGE with +/-HUGE_VAL) and an unobtainable
"C" locale fail the operation instead of yielding a wrong value; ERANGE
underflow is a valid subnormal or zero and is accepted.

With NrrdIO locale-independent by construction, the NumericLocale RAII
guards in itk::NrrdImageIO are unnecessary and are removed; that RAII
mechanism is a no-op on macOS 13 and earlier libc, whose non-_l
strtod/printf ignore the thread-local locale.

itkNrrdLocaleTest now loops over four decimal-comma locales, adds a
write-under-comma-locale round trip, and warns loudly when no comma
locale is installed.

Mirrors the same change on the teem upstream branch
locale-independent-numeric-parse.

Author: hjmjohnson

Modules/IO/NRRD/src/itkNrrdImageIO.cxx

@@ -22,7 +22,6 @@
 #include "itkMetaDataObject.h"
 #include "itkIOCommon.h"
 #include "itkFloatingPointExceptions.h"
-#include "itkNumericLocale.h"
 #include "itkNumberToString.h"
 
 #include <cstdio>
@@ -599,12 +598,6 @@ NrrdImageIO::ReadImageInformation()
       FloatingPointExceptions::Disable();
     }
 
-    // Set LC_NUMERIC to "C" locale to ensure locale-independent parsing
-    // of floating-point values in NRRD headers (spacing, origin, direction, etc.).
-    // This prevents issues in locales that use comma as decimal separator.
-    // Using thread-safe NumericLocale from ITKCommon.
-    NumericLocale cLocale;
-
     // this is the mechanism by which we tell nrrdLoad to read
     // just the header, and none of the data
     nrrdIoStateSet(nio, nrrdIoStateSkipData, 1);
@@ -1138,11 +1131,6 @@ NrrdImageIO::Read(void * buffer)
   FloatingPointExceptions::Disable();
 #endif
 
-  // Set LC_NUMERIC to "C" locale to ensure locale-independent parsing
-  // of floating-point values in NRRD headers.
-  // Using thread-safe NumericLocale from ITKCommon.
-  NumericLocale cLocale;
-
   // Read in the nrrd.  Yes, this means that the header is being read
   // twice: once by NrrdImageIO::ReadImageInformation, and once here
   if (nrrdLoad(nrrd, this->GetFileName(), nullptr) != 0)
@@ -1460,11 +1448,6 @@ NrrdImageIO::Write(const void * buffer)
       break;
   }
 
-  // Set LC_NUMERIC to "C" locale to ensure locale-independent formatting
-  // of floating-point values when writing NRRD headers.
-  // Using thread-safe NumericLocale from ITKCommon.
-  NumericLocale cLocale;
-
   // Write the nrrd to file.
   if (nrrdSave(this->GetFileName(), nrrd, nio))
   {

Modules/IO/NRRD/test/itkNrrdLocaleTest.cxx

@@ -123,11 +123,18 @@ itkNrrdLocaleTest(int argc, char * argv[])
     }
   }
 
-  // Test 2: Read with de_DE locale (the problematic case)
-  // Try to set German locale; if not available, skip this part
-  if (setlocale(LC_NUMERIC, "de_DE.UTF-8") != nullptr)
+  // Test 2: read under a decimal-comma locale -- the only path exercising the
+  // locale-independent parse; warn loudly below when none is installed.
+  const char * const commaLocales[] = { "de_DE.UTF-8", "fr_FR.UTF-8", "nl_NL.UTF-8", "it_IT.UTF-8" };
+  unsigned int       localesTested = 0;
+  for (const char * const commaLocale : commaLocales)
   {
-    std::cout << "Testing with de_DE.UTF-8 locale..." << std::endl;
+    if (setlocale(LC_NUMERIC, commaLocale) == nullptr)
+    {
+      continue;
+    }
+    ++localesTested;
+    std::cout << "Testing with " << commaLocale << " locale..." << std::endl;
 
     using ReaderType = itk::ImageFileReader<ImageType>;
     auto reader = ReaderType::New();
@@ -140,15 +147,15 @@ itkNrrdLocaleTest(int argc, char * argv[])
     ImageType::SpacingType readSpacing = readImage->GetSpacing();
     ImageType::PointType   readOrigin = readImage->GetOrigin();
 
-    std::cout << "de_DE locale - Read spacing: " << readSpacing << std::endl;
-    std::cout << "de_DE locale - Read origin: " << readOrigin << std::endl;
+    std::cout << commaLocale << " - Read spacing: " << readSpacing << std::endl;
+    std::cout << commaLocale << " - Read origin: " << readOrigin << std::endl;
 
     // Verify spacing - this is the critical test
     for (unsigned int i = 0; i < Dimension; ++i)
     {
       if (itk::Math::Absolute(readSpacing[i] - spacing[i]) > 1e-6)
       {
-        std::cerr << "Spacing mismatch in de_DE locale at index " << i << std::endl;
+        std::cerr << "Spacing mismatch in " << commaLocale << " locale at index " << i << std::endl;
         std::cerr << "Expected: " << spacing[i] << ", Got: " << readSpacing[i] << std::endl;
         std::cerr << "This indicates locale-dependent parsing is still occurring!" << std::endl;
         return EXIT_FAILURE;
@@ -160,19 +167,57 @@ itkNrrdLocaleTest(int argc, char * argv[])
     {
       if (itk::Math::Absolute(readOrigin[i] - origin[i]) > 1e-6)
       {
-        std::cerr << "Origin mismatch in de_DE locale at index " << i << std::endl;
+        std::cerr << "Origin mismatch in " << commaLocale << " locale at index " << i << std::endl;
         std::cerr << "Expected: " << origin[i] << ", Got: " << readOrigin[i] << std::endl;
         std::cerr << "This indicates locale-dependent parsing is still occurring!" << std::endl;
         return EXIT_FAILURE;
       }
     }
+  }
+  setlocale(LC_NUMERIC, "C");
 
-    // Restore C locale
+  // Test 3: WRITE under a decimal-comma locale, read back under "C". A
+  // locale-dependent writer would emit "0,878906" and corrupt the file.
+  for (const char * const commaLocale : commaLocales)
+  {
+    if (setlocale(LC_NUMERIC, commaLocale) == nullptr)
+    {
+      continue;
+    }
+    const std::string commaFilename = std::string(argv[1]) + "/locale_test_written_under_comma.nrrd";
+    auto              commaWriter = WriterType::New();
+    commaWriter->SetFileName(commaFilename);
+    commaWriter->SetInput(image);
+    commaWriter->SetImageIO(itk::NrrdImageIO::New());
+    ITK_TRY_EXPECT_NO_EXCEPTION(commaWriter->Update());
     setlocale(LC_NUMERIC, "C");
+
+    using ReaderType = itk::ImageFileReader<ImageType>;
+    auto reader = ReaderType::New();
+    reader->SetFileName(commaFilename);
+    reader->SetImageIO(itk::NrrdImageIO::New());
+    ITK_TRY_EXPECT_NO_EXCEPTION(reader->Update());
+
+    const ImageType::SpacingType readSpacing = reader->GetOutput()->GetSpacing();
+    for (unsigned int i = 0; i < Dimension; ++i)
+    {
+      if (itk::Math::Absolute(readSpacing[i] - spacing[i]) > 1e-6)
+      {
+        std::cerr << "Spacing mismatch after writing under " << commaLocale << " at index " << i << std::endl;
+        std::cerr << "Expected: " << spacing[i] << ", Got: " << readSpacing[i] << std::endl;
+        std::cerr << "This indicates locale-dependent FORMATTING during write!" << std::endl;
+        return EXIT_FAILURE;
+      }
+    }
+    break; // one comma locale suffices for the write path
   }
-  else
+  setlocale(LC_NUMERIC, "C");
+
+  if (localesTested == 0)
   {
-    std::cout << "de_DE.UTF-8 locale not available, skipping locale-specific test" << std::endl;
+    std::cout << "WARNING: no decimal-comma locale installed; the locale-independent "
+              << "parse and format paths were NOT exercised on this runner. Install "
+              << "e.g. de_DE.UTF-8 to cover them." << std::endl;
   }
 
   std::cout << "Test finished successfully." << std::endl;

Modules/ThirdParty/NrrdIO/src/CMakeLists.txt

@@ -14,6 +14,10 @@ set(ZLIB_LIBRARY ITK::ITKZLIBModule)
 list(APPEND NRRDIO_COMPRESSION_LIBRARIES ITK::ITKZLIBModule)
 if(UNIX)
   list(APPEND NRRDIO_COMPRESSION_LIBRARIES m)
+  # parseAir.c uses pthread_once for its cached "C" locale; on glibc < 2.34
+  # pthread_once lives in libpthread, not libc.
+  find_package(Threads REQUIRED)
+  list(APPEND NRRDIO_COMPRESSION_LIBRARIES Threads::Threads)
   list(REMOVE_DUPLICATES NRRDIO_COMPRESSION_LIBRARIES)
 endif()
 

Modules/ThirdParty/NrrdIO/src/NrrdIO/itk_NrrdIO_mangle.h.in

@@ -113,6 +113,7 @@ number, from inclusion.
 #define airTeemReleaseDone @MANGLE_PREFIX@_airTeemReleaseDone
 #define airTeemVersion @MANGLE_PREFIX@_airTeemVersion
 #define airTeemVersionSprint @MANGLE_PREFIX@_airTeemVersionSprint
+#define air__CLocale @MANGLE_PREFIX@_air__CLocale
 #define air__SanityHelper @MANGLE_PREFIX@_air__SanityHelper
 #define biffAdd @MANGLE_PREFIX@_biffAdd
 #define biffAddf @MANGLE_PREFIX@_biffAddf

Modules/ThirdParty/NrrdIO/src/NrrdIO/miscAir.c

@@ -23,6 +23,25 @@
   3. This notice may not be removed or altered from any source distribution.
 */
 
+/* Locale-independent numeric printing for airSinglePrintf; the API selection
+   must precede the first system-header include (NrrdIO.h pulls in <stdio.h>).
+   BSD/macOS and Windows have printf-family *_l variants; glibc/musl lack
+   them, but their printf honors the uselocale() thread-local locale. */
+#if defined(__APPLE__) || defined(__FreeBSD__) || defined(__NetBSD__)                    \
+  || defined(__OpenBSD__) || defined(__DragonFly__)
+#  include <xlocale.h>
+#  define TEEM_HAS_PRINTF_L 1
+#elif defined(__linux__)
+#  ifndef _GNU_SOURCE
+#    define _GNU_SOURCE 1
+#  endif
+#  include <locale.h>
+#  define TEEM_HAS_USELOCALE 1
+#elif defined(_MSC_VER)
+#  include <locale.h>
+#  define TEEM_HAS_PRINTF_L_WIN 1
+#endif
+
 #include "NrrdIO.h"
 #include "privateAir.h"
 /* timer functions */
@@ -211,8 +230,27 @@ airSinglePrintf(FILE *file, char *str, size_t strSize, const char *_fmt, ...) {
   int ret, isF, isD;
   const char *_p0, *_p1, *_p2, *_p3, *_p4, *_p5;
   va_list ap;
+#if defined(TEEM_HAS_PRINTF_L) || defined(TEEM_HAS_USELOCALE)
+  locale_t _aspCloc = (locale_t)air__CLocale();
+#elif defined(TEEM_HAS_PRINTF_L_WIN)
+  _locale_t _aspCloc = (_locale_t)air__CLocale();
+#endif
+#if defined(TEEM_HAS_USELOCALE)
+  /* swap this thread to the "C" numeric locale for the whole function;
+     restored at the single exit below */
+  locale_t _aspOld = _aspCloc ? uselocale(_aspCloc) : (locale_t)0;
+#endif
 
   va_start(ap, _fmt);
+#if defined(TEEM_HAS_PRINTF_L) || defined(TEEM_HAS_USELOCALE)                            \
+  || defined(TEEM_HAS_PRINTF_L_WIN)
+  /* cannot print locale-independently without the cached "C" locale: fail
+     (printf error convention) rather than risk a wrong decimal separator */
+  if (!_aspCloc) {
+    va_end(ap);
+    return -1;
+  }
+#endif
 
   /* look for unmodified floating point conversion sequences */
   _p0 = strstr(_fmt, "%e");
@@ -228,10 +266,22 @@ airSinglePrintf(FILE *file, char *str, size_t strSize, const char *_fmt, ...) {
      "is 3-character conv. seq."
      (but for TeemV2 we run with that type implication for "%g" and "%lg") */
   if (isF || isD) {
+#if defined(TEEM_HAS_PRINTF_L)
+#define PRINT(F, S, C, V)                                                               \
+  ((F) /* */                                                                            \
+     ? fprintf_l((F), _aspCloc, (C), (V))                                               \
+     : snprintf_l((S), strSize, _aspCloc, (C), (V)))
+#elif defined(TEEM_HAS_PRINTF_L_WIN)
+#define PRINT(F, S, C, V)                                                               \
+  ((F) /* */                                                                            \
+     ? _fprintf_l((F), (C), _aspCloc, (V))                                              \
+     : _snprintf_l((S), strSize, (C), _aspCloc, (V)))
+#else
 #define PRINT(F, S, C, V)                                                               \
   ((F) /* */                                                                            \
      ? fprintf((F), (C), (V))                                                           \
      : snprintf((S), strSize, (C), (V)))
+#endif
     double val0;
     const char *_conv;
     size_t fmtSize;
@@ -342,9 +392,20 @@ airSinglePrintf(FILE *file, char *str, size_t strSize, const char *_fmt, ...) {
        more specific like "%.17g" (which NOTE does not get the above special treatment of
        IEEE754 special values!s), or, it isn't for any kind of floating point value.
        We can use the given `_fmt` as is (no local allocation of `fmt`) */
+#if defined(TEEM_HAS_PRINTF_L)
+    ret = file ? vfprintf_l(file, _aspCloc, _fmt, ap) /* */
+               : vsnprintf_l(str, strSize, _aspCloc, _fmt, ap);
+#elif defined(TEEM_HAS_PRINTF_L_WIN)
+    ret = file ? _vfprintf_l(file, _fmt, _aspCloc, ap) /* */
+               : _vsnprintf_l(str, strSize, _fmt, _aspCloc, ap);
+#else
     ret = file ? vfprintf(file, _fmt, ap) : vsnprintf(str, strSize, _fmt, ap);
+#endif
   }
 
+#if defined(TEEM_HAS_USELOCALE)
+  uselocale(_aspOld);
+#endif
   va_end(ap);
   return ret;
 }

Modules/ThirdParty/NrrdIO/src/NrrdIO/parseAir.c

@@ -23,7 +23,75 @@
   3. This notice may not be removed or altered from any source distribution.
 */
 
+/* Enable the POSIX-2008 thread-safe locale API (newlocale/strtod_l) where
+   available; must precede the first system-header include (NrrdIO.h pulls in
+   <stdlib.h>).  TEEM_HAS_STRTOD_L selects a locale-independent numeric parse
+   in airSingleSscanf(); platforms lacking it keep the historical sscanf(). */
+#if defined(__APPLE__) || defined(__FreeBSD__) || defined(__NetBSD__)                    \
+  || defined(__OpenBSD__) || defined(__DragonFly__)
+#  include <xlocale.h>
+#  include <pthread.h>
+#  define TEEM_HAS_STRTOD_L 1
+#elif defined(__linux__)
+/* __GLIBC__ comes from <features.h>, not a compiler predefine, so it is unset
+   at this pre-include point; key off __linux__ (glibc, and musl >= 1.2.1,
+   provide newlocale/strtod_l under _GNU_SOURCE). */
+#  ifndef _GNU_SOURCE
+#    define _GNU_SOURCE 1
+#  endif
+#  include <locale.h>
+#  include <pthread.h>
+#  define TEEM_HAS_STRTOD_L 1
+#elif defined(_MSC_VER)
+#  include <locale.h>
+#  include <intrin.h>
+#  define TEEM_HAS_STRTOD_L_WIN 1
+#endif
+
 #include "NrrdIO.h"
+#include "privateAir.h"
+
+#if defined(TEEM_HAS_STRTOD_L)
+/* Cached "C" locale for locale-independent strtod_l; created once. */
+static locale_t _airCLocale = (locale_t)0;
+static pthread_once_t _airCLocaleOnce = PTHREAD_ONCE_INIT;
+static void
+_airCLocaleInit(void) {
+  _airCLocale = newlocale(LC_NUMERIC_MASK, "C", (locale_t)0);
+}
+static locale_t
+_airGetCLocale(void) {
+  pthread_once(&_airCLocaleOnce, _airCLocaleInit);
+  return _airCLocale;
+}
+#elif defined(TEEM_HAS_STRTOD_L_WIN)
+/* Cached "C" locale published exactly once via an atomic pointer CAS, so
+   concurrent first use is race-free: the losing thread frees its duplicate. */
+static void *volatile _airCLocale = NULL;
+static _locale_t
+_airGetCLocale(void) {
+  _locale_t loc = (_locale_t)_airCLocale;
+  if (!loc) {
+    _locale_t created = _create_locale(LC_NUMERIC, "C");
+    void *prev = _InterlockedCompareExchangePointer(&_airCLocale, created, NULL);
+    if (prev) { _free_locale(created); loc = (_locale_t)prev; }
+    else      { loc = created; }
+  }
+  return loc;
+}
+#endif
+
+/* Cached "C" LC_NUMERIC locale as an opaque pointer (locale_t or _locale_t);
+   NULL when the platform lacks the thread-safe locale API or creation failed.
+   Shared with airSinglePrintf (miscAir.c) via privateAir.h. */
+void *
+air__CLocale(void) {
+#if defined(TEEM_HAS_STRTOD_L) || defined(TEEM_HAS_STRTOD_L_WIN)
+  return (void *)_airGetCLocale();
+#else
+  return NULL;
+#endif
+}
 
 /* clang-format off */
 static const char *
@@ -105,12 +173,45 @@ airSingleSscanf(const char *str, const char *fmt, void *ptr) {
     } else if (strstr(tmp, "inf")) {
       val = (double)AIR_POS_INF;
     } else {
-      /* nothing special matched; pass it off to sscanf() */
-      /* (save setlocale here) */
+      /* nothing special matched; parse one floating-point value with a
+         locale-independent "C" parse so NRRD's '.' decimal separator is
+         honored regardless of the process/thread LC_NUMERIC setting. */
+#if defined(TEEM_HAS_STRTOD_L) || defined(TEEM_HAS_STRTOD_L_WIN)
+      char *endptr = NULL;
+      double dval = 0.0;
+      ret = 0;
+      /* a null "C" locale means we cannot parse locale-independently:
+         fail the parse rather than risk a silently wrong value */
+#  if defined(TEEM_HAS_STRTOD_L)
+      locale_t cloc = _airGetCLocale();
+      if (cloc) {
+        errno = 0;
+        dval = strtod_l(str, &endptr, cloc);
+#  else
+      _locale_t cloc = _airGetCLocale();
+      if (cloc) {
+        errno = 0;
+        dval = _strtod_l(str, &endptr, cloc);
+#  endif
+        /* accept a conversion unless it overflowed (ERANGE with +/-HUGE_VAL);
+           ERANGE underflow is a valid subnormal or zero */
+        if (endptr != str
+            && !(ERANGE == errno && (HUGE_VAL == dval || -HUGE_VAL == dval))) {
+          ret = 1;
+        }
+      }
+      if (ret) {
+        if (fmt[1] == 'l') { *((double *)(ptr)) = dval; }
+        else               { *((float *)(ptr)) = AIR_FLOAT(dval); }
+      }
+      free(tmp);
+      return ret;
+#else
+      /* fallback: historical locale-dependent behavior, unchanged */
       ret = sscanf(str, fmt, ptr);
-      /* (return setlocale here) */
       free(tmp);
       return ret;
+#endif
     }
     /* else we matched "nan", "-inf", or "inf"; now set val accordingly */
     if (fmt[1] == 'l') {