troldal / OpenXLSX

A C++ library for reading, writing, creating and modifying Microsoft Excel® (.xlsx) files.
BSD 3-Clause "New" or "Revised" License
1.32k stars 312 forks source link

Broken with floats if locale decimal_point is not '.' #190

Open piru opened 1 year ago

piru commented 1 year ago

OpenXLSX fails when used with locales that use decimal_point other than .. This happens because of checking for hardcoded decimal separator at https://github.com/troldal/OpenXLSX/blob/29034465e569675f1677157b65b8263ccb1452f0/OpenXLSX/sources/XLCellValue.cpp#L249

CEXT-Dan commented 1 year ago

It’s my understanding that this is by design. If it were by locale, then other locales may see an invalid file format.

piru commented 1 year ago

OpenXLSX uses locale aware routines elsewhere to format floats. Hence it itself generates invalid formats. Either you consistently obey locale or don't. You can't mix.

CEXT-Dan commented 1 year ago

I don't think this is the case in the raw data, I believe this is stored in the STYLE, otherwise, if you were to send me a file to test, my tests would fail.

piru commented 1 year ago

Here are steps to reproduce:

  1. Apply this patch:

    index 44d6eee..102b571 100644
    --- a/Examples/Demo1.cpp
    +++ b/Examples/Demo1.cpp
    @@ -7,6 +7,8 @@ using namespace OpenXLSX;
    
    int main()
    {
    +    setlocale(LC_ALL, "");
    +
     cout << "********************************************************************************\n";
     cout << "DEMO PROGRAM #01: Basic Usage\n";
     cout << "********************************************************************************\n";
  2. Rebuild
  3. LC_NUMERIC=fi_FI.UTF-8 output/Demo1 (Note: Any locale that has non . decimal_point will do, fi_FI.UTF-8 is just one of them)

Result:

********************************************************************************
DEMO PROGRAM #01: Basic Usage
********************************************************************************
terminate called after throwing an instance of 'OpenXLSX::XLValueTypeError'
  what():  XLCellValue object does not contain the requested type.
Aborted
CEXT-Dan commented 1 year ago

I see, IMHO, the issue is in PUGI__SNPRINTF is using your locale when setting the data. The raw data itself should always be “.”

piru commented 1 year ago

This is a bit messy to fix cleanly on all platforms. On POSIX 2008 conforming system this is somewhat ok, as you can newlocale the "C" locale, and then either uselocale the "C" one when needed or even better use it directly with snprintf_l and wcstod_l/strtod_l, and finally cleaning up with freelocale.

Here is some discussion: https://stackoverflow.com/questions/13919817/how-to-parse-numbers-like-3-14-with-scanf-when-locale-expects-3-14