diff --git a/Modules/IO/NRRD/test/CMakeLists.txt b/Modules/IO/NRRD/test/CMakeLists.txt index 004e86d1ad7..32cb1eb372a 100644 --- a/Modules/IO/NRRD/test/CMakeLists.txt +++ b/Modules/IO/NRRD/test/CMakeLists.txt @@ -356,3 +356,8 @@ itk_add_test( itkNrrdMetaDataTest ${ITK_TEST_OUTPUT_DIR} ) + +# GTest tests +set(ITKIONRRDGTests itkNrrdLocaleGTest.cxx) + +creategoogletestdriver(ITKIONRRD "${ITKIONRRD-Test_LIBRARIES}" "${ITKIONRRDGTests}") diff --git a/Modules/IO/NRRD/test/itkNrrdLocaleGTest.cxx b/Modules/IO/NRRD/test/itkNrrdLocaleGTest.cxx new file mode 100644 index 00000000000..a80fcfaa535 --- /dev/null +++ b/Modules/IO/NRRD/test/itkNrrdLocaleGTest.cxx @@ -0,0 +1,176 @@ +/*========================================================================= + * + * Copyright NumFOCUS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + *=========================================================================*/ + +#include +#include "itkImage.h" +#include "itkImageFileReader.h" +#include "itkImageFileWriter.h" +#include "itkNrrdImageIO.h" +#include "itksys/SystemTools.hxx" + +#include "itkGTest.h" + +#define _STRING(s) #s +#define TOSTRING(s) std::string(_STRING(s)) + +namespace +{ + +constexpr unsigned int Dimension = 3; +using PixelType = float; +using ImageType = itk::Image; + +class NrrdLocaleTest : public ::testing::Test +{ +public: + void + SetUp() override + { + itksys::SystemTools::ChangeDirectory(TOSTRING(ITK_TEST_OUTPUT_DIR)); + } + + ImageType::Pointer + CreateTestImage() + { + auto image = ImageType::New(); + + ImageType::SizeType size; + size.Fill(16); + + ImageType::IndexType start; + start.Fill(0); + + ImageType::RegionType region(start, size); + image->SetRegions(region); + image->Allocate(); + image->FillBuffer(42.0f); + + // Set spacing with fractional values that would be misparsed + // in locales using comma as decimal separator + ImageType::SpacingType spacing; + spacing[0] = 3.5; // Would be parsed correctly even with truncation + spacing[1] = 0.878906; // Would become 0.0 in de_DE locale, causing error + spacing[2] = 2.2; // Would be parsed incorrectly + + image->SetSpacing(spacing); + + // Set origin with fractional values + ImageType::PointType origin; + origin[0] = 1.5; + origin[1] = 2.75; + origin[2] = 0.5; + image->SetOrigin(origin); + + return image; + } +}; + +TEST_F(NrrdLocaleTest, ReadWriteWithCLocale) +{ + auto image = CreateTestImage(); + const auto expectedSpacing = image->GetSpacing(); + const auto expectedOrigin = image->GetOrigin(); + + const std::string filename = "locale_test.nrrd"; + + // Write the image with C locale + setlocale(LC_NUMERIC, "C"); + + auto writer = itk::ImageFileWriter::New(); + writer->SetFileName(filename); + writer->SetInput(image); + writer->SetImageIO(itk::NrrdImageIO::New()); + EXPECT_NO_THROW(writer->Update()); + + // Read with C locale + auto reader = itk::ImageFileReader::New(); + reader->SetFileName(filename); + reader->SetImageIO(itk::NrrdImageIO::New()); + EXPECT_NO_THROW(reader->Update()); + + const auto readImage = reader->GetOutput(); + const auto readSpacing = readImage->GetSpacing(); + const auto readOrigin = readImage->GetOrigin(); + + // Verify spacing + for (unsigned int i = 0; i < Dimension; ++i) + { + EXPECT_NEAR(readSpacing[i], expectedSpacing[i], 1e-6) << "Spacing mismatch in C locale at index " << i; + } + + // Verify origin + for (unsigned int i = 0; i < Dimension; ++i) + { + EXPECT_NEAR(readOrigin[i], expectedOrigin[i], 1e-6) << "Origin mismatch in C locale at index " << i; + } +} + +TEST_F(NrrdLocaleTest, ReadWithGermanLocale) +{ + auto image = CreateTestImage(); + const auto expectedSpacing = image->GetSpacing(); + const auto expectedOrigin = image->GetOrigin(); + + const std::string filename = "locale_test_de.nrrd"; + + // Write the image with C locale + setlocale(LC_NUMERIC, "C"); + + auto writer = itk::ImageFileWriter::New(); + writer->SetFileName(filename); + writer->SetInput(image); + writer->SetImageIO(itk::NrrdImageIO::New()); + EXPECT_NO_THROW(writer->Update()); + + // Try to set German locale; skip test if not available + if (setlocale(LC_NUMERIC, "de_DE.UTF-8") == nullptr) + { + GTEST_SKIP() << "de_DE.UTF-8 locale not available, skipping locale-specific test"; + } + + // Read with de_DE locale - this is the critical test + auto reader = itk::ImageFileReader::New(); + reader->SetFileName(filename); + reader->SetImageIO(itk::NrrdImageIO::New()); + EXPECT_NO_THROW(reader->Update()); + + const auto readImage = reader->GetOutput(); + const auto readSpacing = readImage->GetSpacing(); + const auto readOrigin = readImage->GetOrigin(); + + // Verify spacing - this would fail without locale-independent parsing + for (unsigned int i = 0; i < Dimension; ++i) + { + EXPECT_NEAR(readSpacing[i], expectedSpacing[i], 1e-6) + << "Spacing mismatch in de_DE locale at index " << i + << ". This indicates locale-dependent parsing is still occurring!"; + } + + // Verify origin + for (unsigned int i = 0; i < Dimension; ++i) + { + EXPECT_NEAR(readOrigin[i], expectedOrigin[i], 1e-6) + << "Origin mismatch in de_DE locale at index " << i + << ". This indicates locale-dependent parsing is still occurring!"; + } + + // Restore C locale + setlocale(LC_NUMERIC, "C"); +} + +} // namespace diff --git a/Modules/ThirdParty/NrrdIO/src/NrrdIO/CMakeLists.txt b/Modules/ThirdParty/NrrdIO/src/NrrdIO/CMakeLists.txt index c70ca0f7e70..db5ea50705d 100644 --- a/Modules/ThirdParty/NrrdIO/src/NrrdIO/CMakeLists.txt +++ b/Modules/ThirdParty/NrrdIO/src/NrrdIO/CMakeLists.txt @@ -8,7 +8,7 @@ INCLUDE_REGULAR_EXPRESSION("^.*.h$") # package. See http://teem.sourceforge.net for more information. # -SET(nrrdio_SRCS 754.c mop.c array.c parseAir.c dio.c sane.c endianAir.c +SET(nrrdio_SRCS 754.c mop.c array.c parseAir.cxx dio.c sane.c endianAir.c string.c enum.c miscAir.c biffbiff.c biffmsg.c accessors.c defaultsNrrd.c enumsNrrd.c arraysNrrd.c methodsNrrd.c reorder.c axis.c simple.c comment.c keyvalue.c endianNrrd.c parseNrrd.c gzio.c read.c write.c format.c diff --git a/Modules/ThirdParty/NrrdIO/src/NrrdIO/parseAir.c b/Modules/ThirdParty/NrrdIO/src/NrrdIO/parseAir.cxx similarity index 85% rename from Modules/ThirdParty/NrrdIO/src/NrrdIO/parseAir.c rename to Modules/ThirdParty/NrrdIO/src/NrrdIO/parseAir.cxx index a14baeb1e61..6a514005762 100644 --- a/Modules/ThirdParty/NrrdIO/src/NrrdIO/parseAir.c +++ b/Modules/ThirdParty/NrrdIO/src/NrrdIO/parseAir.cxx @@ -23,7 +23,14 @@ 3. This notice may not be removed or altered from any source distribution. */ +#include +#include +#include +#include + +extern "C" { #include "NrrdIO.h" +} static const char * _airBoolStr[] = { @@ -105,10 +112,29 @@ airSingleSscanf(const char *str, const char *fmt, void *ptr) { val = AIR_POS_INF; } else { - /* nothing special matched; pass it off to sscanf() */ - ret = sscanf(str, fmt, ptr); + /* nothing special matched; use C++ stream with classic locale */ + std::istringstream iss(str); + iss.imbue(std::locale::classic()); + + if (!strncmp(fmt, "%l", 2)) { + /* we were given a double pointer */ + double dval; + if (!(iss >> dval)) { + free(tmp); + return 0; + } + *((double *)(ptr)) = dval; + } else { + /* we were given a float pointer */ + float fval; + if (!(iss >> fval)) { + free(tmp); + return 0; + } + *((float *)(ptr)) = fval; + } free(tmp); - return ret; + return 1; } /* else we matched "nan", "-inf", or "inf", and set val accordingly */ if (!strncmp(fmt, "%l", 2)) { @@ -138,8 +164,36 @@ airSingleSscanf(const char *str, const char *fmt, void *ptr) { *((size_t *)(ptr)) = tsz; return 1; } else { - /* not a float, double, or size_t, let sscanf handle it */ - return sscanf(str, fmt, ptr); + /* not a float, double, or size_t, use C++ stream with classic locale */ + std::istringstream iss(str); + iss.imbue(std::locale::classic()); + + if (!strcmp(fmt, "%d")) { + int ival; + if (!(iss >> ival)) return 0; + *((int *)(ptr)) = ival; + return 1; + } else if (!strcmp(fmt, "%u")) { + unsigned int uival; + if (!(iss >> uival)) return 0; + *((unsigned int *)(ptr)) = uival; + return 1; + } else if (!strcmp(fmt, "%ld")) { + long lival; + if (!(iss >> lival)) return 0; + *((long *)(ptr)) = lival; + return 1; + } else if (!strcmp(fmt, "%lu")) { + unsigned long ulval; + if (!(iss >> ulval)) return 0; + *((unsigned long *)(ptr)) = ulval; + return 1; + } + /* fallback to sscanf for unknown formats - should never reach here! */ + std::string error_msg = std::string("airSingleSscanf: Unsupported format '") + + fmt + "' for string '" + str + + "' - fallback to locale-dependent sscanf would be used!"; + throw std::runtime_error(error_msg); } } diff --git a/Modules/ThirdParty/NrrdIO/src/NrrdIO/parseNrrd.c b/Modules/ThirdParty/NrrdIO/src/NrrdIO/parseNrrd.c index 01ded2644de..cee63586251 100644 --- a/Modules/ThirdParty/NrrdIO/src/NrrdIO/parseNrrd.c +++ b/Modules/ThirdParty/NrrdIO/src/NrrdIO/parseNrrd.c @@ -206,7 +206,7 @@ _nrrdReadNrrdParse_type(FILE *file, Nrrd *nrrd, } #define _PARSE_ONE_VAL(FIELD, CONV, TYPE) \ - if (1 != sscanf(info, CONV, &(FIELD))) { \ + if (1 != airSingleSscanf(info, CONV, &(FIELD))) { \ biffMaybeAddf(useBiff, NRRD, "%s: couldn't parse " TYPE \ " from \"%s\"", me, info); \ return 1; \ @@ -1299,24 +1299,54 @@ _nrrdReadNrrdParse_data_file(FILE *ffile, Nrrd *nrrd, sspn = strspn(nums, _nrrdFieldSep); nums[0] = 0; /* terminate so that format is now in info */ nums += sspn; - if (!( 3 == sscanf(nums, "%d %d %d",&(nio->dataFNMin), - &(nio->dataFNMax), &(nio->dataFNStep)) )) { - biffMaybeAddf(useBiff, NRRD, - "%s: couldn't parse three ints (min, max, step) after " - "data filename template", me); - airMopError(mop); return 1; - } - if ( 4 == sscanf(nums, "%d %d %d %u", &(nio->dataFNMin), - &(nio->dataFNMax), &(nio->dataFNStep), - &(nio->dataFileDim)) ) { - if (!AIR_IN_CL(1, nio->dataFileDim, nrrd->dim)) { + /* Parse three integers one at a time for locale independence */ + { + char *numsCopy = airStrdup(nums); + char *ptr = numsCopy; + int parsed = 0; + if (1 == airSingleSscanf(ptr, "%d", &(nio->dataFNMin))) { + while (*ptr && !isspace(*ptr)) ptr++; + while (*ptr && isspace(*ptr)) ptr++; + if (1 == airSingleSscanf(ptr, "%d", &(nio->dataFNMax))) { + while (*ptr && !isspace(*ptr)) ptr++; + while (*ptr && isspace(*ptr)) ptr++; + if (1 == airSingleSscanf(ptr, "%d", &(nio->dataFNStep))) { + parsed = 3; + } + } + } + airFree(numsCopy); + if (3 != parsed) { biffMaybeAddf(useBiff, NRRD, - "%s: datafile dimension %u outside valid range [1,%u]", - me, nio->dataFileDim, nrrd->dim); + "%s: couldn't parse three ints (min, max, step) after " + "data filename template", me); airMopError(mop); return 1; } - } else { - nio->dataFileDim = nrrd->dim-1; + } + /* Try to parse optional fourth integer */ + { + char *numsCopy = airStrdup(nums); + char *ptr = numsCopy; + int parsed = 0; + /* Skip past the three already parsed integers */ + for (int i = 0; i < 3 && *ptr; i++) { + while (*ptr && !isspace(*ptr)) ptr++; + while (*ptr && isspace(*ptr)) ptr++; + } + if (*ptr && 1 == airSingleSscanf(ptr, "%u", &(nio->dataFileDim))) { + parsed = 4; + if (!AIR_IN_CL(1, nio->dataFileDim, nrrd->dim)) { + biffMaybeAddf(useBiff, NRRD, + "%s: datafile dimension %u outside valid range [1,%u]", + me, nio->dataFileDim, nrrd->dim); + airFree(numsCopy); + airMopError(mop); return 1; + } + } + airFree(numsCopy); + if (4 != parsed) { + nio->dataFileDim = nrrd->dim-1; + } } if (0 == nio->dataFNStep) { biffMaybeAddf(useBiff, NRRD, @@ -1353,7 +1383,7 @@ _nrrdReadNrrdParse_data_file(FILE *ffile, Nrrd *nrrd, } info += strlen(NRRD_LIST_FLAG); if (info[0]) { - if (1 == sscanf(info, "%u", &(nio->dataFileDim))) { + if (1 == airSingleSscanf(info, "%u", &(nio->dataFileDim))) { if (!AIR_IN_CL(1, nio->dataFileDim, nrrd->dim)) { biffMaybeAddf(useBiff, NRRD, "%s: datafile dimension %u outside " "valid range [1,%u]",