From 6565352d06ecdc0189bd6c30dbefd0d3c5573efa Mon Sep 17 00:00:00 2001 From: Christoph Hasse Date: Thu, 28 Mar 2024 21:06:27 -0600 Subject: [PATCH] fix(lens): canon lens detecton should be independent of locale --- CMakeLists.txt | 8 ++++++++ src/canonmn_int.cpp | 38 ++++++++++++++++++++++++++++++++------ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cd339a6213..01a704b112 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -139,6 +139,14 @@ if(EXIV2_BUILD_EXIV2_COMMAND) WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/tests COMMAND cmake -E env EXIV2_BINDIR=${CMAKE_RUNTIME_OUTPUT_DIRECTORY} ${Python3_EXECUTABLE} runner.py --verbose lens_tests ) + # Repeat lens test with a DE locale that uses "," as float delimiter + # to check that all float conversions are done independently of locale + # See GitHub issue #2746 + add_test( + NAME lensTestsLocaleDE + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/tests + COMMAND cmake -E env LC_ALL=de_DE.UTF-8 EXIV2_BINDIR=${CMAKE_RUNTIME_OUTPUT_DIRECTORY} ${Python3_EXECUTABLE} runner.py --verbose lens_tests + ) add_test( NAME tiffTests WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/tests diff --git a/src/canonmn_int.cpp b/src/canonmn_int.cpp index 3de6946f6f..fe2bc4b646 100644 --- a/src/canonmn_int.cpp +++ b/src/canonmn_int.cpp @@ -23,7 +23,6 @@ #include #include #include - // ***************************************************************************** // class member definitions namespace Exiv2::Internal { @@ -2851,6 +2850,33 @@ std::ostream& printCsLensFFFF(std::ostream& os, const Value& value, const ExifDa return EXV_PRINT_TAG(canonCsLensType)(os, value, metadata); } +/** + * @brief convert string to float w/o considering locale + * + * Using std:stof to convert strings to float takes into account the locale + * and thus leads to wrong results when converting e.g. "5.6" with a DE locale + * which expects "," as decimal instead of ".". See GitHub issue #2746 + * + * Use std::from_chars once that's properly supported by compilers. + * + * @param str string to convert + * @return float value of string + */ +float string_to_float(std::string const& str) { + float val{}; + std::stringstream ss; + std::locale c_locale("C"); + ss.imbue(c_locale); + ss << str; + ss >> val; + + if (ss.fail()) { + throw Error(ErrorCode::kerErrorMessage, std::string("canonmn_int.cpp:string_to_float failed for: ") + str); + } + + return val; +} + std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, const ExifData* metadata) { if (!metadata || value.typeId() != unsignedShort || value.count() == 0) return os << value; @@ -2914,13 +2940,13 @@ std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, co throw Error(ErrorCode::kerErrorMessage, std::string("Lens regex didn't match for: ") + std::string(label)); } - auto tc = base_match[5].length() > 0 ? std::stof(base_match[5].str()) : 1.f; + auto tc = base_match[5].length() > 0 ? string_to_float(base_match[5].str()) : 1.f; - auto flMax = static_cast(std::stof(base_match[2].str()) * tc); - int flMin = base_match[1].length() > 0 ? static_cast(std::stof(base_match[1].str()) * tc) : flMax; + auto flMax = static_cast(string_to_float(base_match[2].str()) * tc); + int flMin = base_match[1].length() > 0 ? static_cast(string_to_float(base_match[1].str()) * tc) : flMax; - auto aperMaxTele = std::stof(base_match[4].str()) * tc; - auto aperMaxShort = base_match[3].length() > 0 ? std::stof(base_match[3].str()) * tc : aperMaxTele; + auto aperMaxTele = string_to_float(base_match[4].str()) * tc; + auto aperMaxShort = base_match[3].length() > 0 ? string_to_float(base_match[3].str()) * tc : aperMaxTele; if (flMin != exifFlMin || flMax != exifFlMax || exifAperMax < (aperMaxShort - .1 * tc) || exifAperMax > (aperMaxTele + .1 * tc)) {