mpx / lua-cjson

Lua CJSON is a fast JSON encoding/parsing module for Lua
https://kyne.au/~mark/software/lua-cjson.php
MIT License
933 stars 478 forks source link

Writing floats is locale dependent #70

Open smurfix opened 4 years ago

smurfix commented 4 years ago

Hi, CJSON is not locale independent. Specifically, if the current locale wants us to encode the decimal point as a comma, as in Germany, CJSON will happily do so.

This makes the other side rather unhappy. Worse, if the problem occurs in a list it's not even recognized by the receiving parser.

Please fix!

mpx commented 4 years ago

Afaik, this bug was fixed in 1.0.4.

Can you please provide some more details about the platform you are using, and how you are compiling Lua CJSON?

There are 2 ways of compiling floating point support:

This is documented in the manual under Built-in floating point conversion.

smurfix commented 4 years ago

I'm using the Debian package for 2.1 which doesn't use specific preprocessor arguments. Apparently the program I'm linking with manages to change the current locale after your fpconv_init gets called.

I'll ask the Debian maintainer to build with USE_INTERNAL_FPCONV, but cjson still needs to support usage with a non-constant locale setting IMHO.

smurfix commented 4 years ago

Also, what about locales that don't even use "our" digits? Please consider unconditionally switching to USE_INTERNAL_FPCONV and/or using that as the default.

mpx commented 4 years ago

While the hack to detect/adapt the decimal separator for different locales is ugly, I'm not aware of any specific locales where it doesn't work (happy to hear otherwise).

Changing the locale after the program has started is typically a bad strategy. setlocale isn't thread-safe, and various code might make assumptions based on the previous configuration.

I agree it would be better to have a single implementation. However, there are tradeoffs:

It's an awkward tradeoff. I wanted Lua CJSON to just work out of the box, which means using the platform routines. The internal dtoa has advantages but must be build correctly for the platform and the codebase.

If Debian wants to use the internal routines it will need to correctly set the endianness, and probably always enable/link pthreads (since it may be used in a multi-threaded application).

mpx commented 4 years ago

Another option: Can you call fpconv_init after the locale has been changed? fpconv_init isn't thread safe, but neither is setlocale, so it shouldn't be any worse.

mpx commented 3 years ago

Another option..

You could use compiler defines to autodetect the correct endianness for USE_INTERNAL_FPCONV by adapting something like this: https://stackoverflow.com/a/58642785

It might be reasonable to add something similar to CJSON as well, but it would need testing across a wide range of compilers/platforms first.