From d7ddbf366197605642f725cce6165dfb179a114e Mon Sep 17 00:00:00 2001 From: Petri Lehtinen Date: Sun, 2 Oct 2011 21:27:53 +0300 Subject: [PATCH] Make real number encoding and decoding work under all locales The decimal point '.' is changed to locale's decimal point before/after JSON conversion to make C standard library's locale-specific string conversion functions work correctly. All the tests now call setlocale(LC_ALL, "") on startup to use the locale set in the environment. Fixes GH-32. --- configure.ac | 7 +++ doc/conformance.rst | 22 +++++++ src/Makefile.am | 1 + src/dump.c | 20 +------ src/jansson_config.h.in | 4 ++ src/jansson_config.h.win32 | 4 ++ src/jansson_private.h | 5 ++ src/load.c | 7 +-- src/strconv.c | 108 +++++++++++++++++++++++++++++++++++ test/bin/json_process.c | 12 ++++ test/suites/api/test_array.c | 4 +- test/suites/api/test_copy.c | 3 +- test/suites/api/test_dump.c | 3 +- test/suites/api/test_dump_callback.c | 4 +- test/suites/api/test_equal.c | 3 +- test/suites/api/test_load.c | 4 +- test/suites/api/test_loadb.c | 4 +- test/suites/api/test_memory_funcs.c | 4 +- test/suites/api/test_number.c | 4 +- test/suites/api/test_object.c | 4 +- test/suites/api/test_pack.c | 4 +- test/suites/api/test_simple.c | 4 +- test/suites/api/test_unpack.c | 4 +- test/suites/api/util.h | 19 ++++++ 24 files changed, 199 insertions(+), 59 deletions(-) create mode 100644 src/strconv.c diff --git a/configure.ac b/configure.ac index 64e22d7..c138937 100644 --- a/configure.ac +++ b/configure.ac @@ -14,6 +14,7 @@ AM_CONDITIONAL([GCC], [test x$GCC = xyes]) # Checks for libraries. # Checks for header files. +AC_CHECK_HEADERS([locale.h]) # Checks for typedefs, structures, and compiler characteristics. AC_TYPE_INT32_T @@ -34,6 +35,12 @@ esac AC_SUBST([json_inline]) # Checks for library functions. +AC_CHECK_FUNCS([setlocale localeconv]) +case "$ac_cv_header_locale_h$ac_cv_func_localeconv" in + yesyes) json_have_localeconv=1;; + *) json_have_localeconv=0;; +esac +AC_SUBST([json_have_localeconv]) AC_CONFIG_FILES([ jansson.pc diff --git a/doc/conformance.rst b/doc/conformance.rst index 34d60bd..3446e26 100644 --- a/doc/conformance.rst +++ b/doc/conformance.rst @@ -110,3 +110,25 @@ types, ``long double``, etc. Obviously, shorter types like ``short``, are implicitly handled via the ordinary C type coercion rules (subject to overflow semantics). Also, no support or hooks are provided for any supplemental "bignum" type add-on packages. + + +Locale +====== + +Jansson works fine under any locale. + +However, if the host program is multithreaded and uses ``setlocale()`` +to switch the locale in one thread while Jansson is currently encoding +or decoding JSON data in another thread, the result may be wrong or +the program may even crash. + +Jansson uses locale specific functions for certain string conversions +in the encoder and decoder, and then converts the locale specific +values to/from the JSON representation. This fails if the locale +changes between the string conversion and the locale-to-JSON +conversion. This can only happen in multithreaded programs that use +``setlocale()``, that switches the locale for all running threads, not +only the thread that calls ``setlocale()``. + +If your program uses ``setlocale()`` as described above, consider +using the thread-safe ``uselocale()`` instead. diff --git a/src/Makefile.am b/src/Makefile.am index 7da78b8..c79502f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -12,6 +12,7 @@ libjansson_la_SOURCES = \ pack_unpack.c \ strbuffer.c \ strbuffer.h \ + strconv.c \ utf.c \ utf.h \ value.c diff --git a/src/dump.c b/src/dump.c index 7bc3456..d87d5c7 100644 --- a/src/dump.c +++ b/src/dump.c @@ -196,26 +196,12 @@ static int do_dump(const json_t *json, size_t flags, int depth, { char buffer[MAX_REAL_STR_LENGTH]; int size; + double value = json_real_value(json); - size = snprintf(buffer, MAX_REAL_STR_LENGTH, "%.17g", - json_real_value(json)); - if(size >= MAX_REAL_STR_LENGTH) + size = jsonp_dtostr(buffer, MAX_REAL_STR_LENGTH, value); + if(size < 0) return -1; - /* Make sure there's a dot or 'e' in the output. Otherwise - a real is converted to an integer when decoding */ - if(strchr(buffer, '.') == NULL && - strchr(buffer, 'e') == NULL) - { - if(size + 2 >= MAX_REAL_STR_LENGTH) { - /* No space to append ".0" */ - return -1; - } - buffer[size] = '.'; - buffer[size + 1] = '0'; - size += 2; - } - return dump(buffer, size, data); } diff --git a/src/jansson_config.h.in b/src/jansson_config.h.in index 7f519cd..52e3d57 100644 --- a/src/jansson_config.h.in +++ b/src/jansson_config.h.in @@ -31,4 +31,8 @@ JSON_INTEGER_IS_LONG_LONG is defined to 1, otherwise to 0. */ #define JSON_INTEGER_IS_LONG_LONG @json_have_long_long@ +/* If locale.h and localeconv() are available, define to 1, + otherwise to 0. */ +#define JSON_HAVE_LOCALECONV @json_have_localeconv@ + #endif diff --git a/src/jansson_config.h.win32 b/src/jansson_config.h.win32 index 86c351d..a188f76 100644 --- a/src/jansson_config.h.win32 +++ b/src/jansson_config.h.win32 @@ -31,4 +31,8 @@ JSON_INTEGER_IS_LONG_LONG is defined to 1, otherwise to 0. */ #define JSON_INTEGER_IS_LONG_LONG 1 +/* If locale.h and localeconv() are available, define to 1, + otherwise to 0. */ +#define JSON_HAVE_LOCALECONV 1 + #endif diff --git a/src/jansson_private.h b/src/jansson_private.h index 452a4f0..dbe9760 100644 --- a/src/jansson_private.h +++ b/src/jansson_private.h @@ -11,6 +11,7 @@ #include #include "jansson.h" #include "hashtable.h" +#include "strbuffer.h" #define container_of(ptr_, type_, member_) \ ((type_ *)((char *)ptr_ - offsetof(type_, member_))) @@ -83,6 +84,10 @@ void jsonp_error_set(json_error_t *error, int line, int column, void jsonp_error_vset(json_error_t *error, int line, int column, size_t position, const char *msg, va_list ap); +/* Locale independent string<->double conversions */ +int jsonp_strtod(strbuffer_t *strbuffer, double *out); +int jsonp_dtostr(char *buffer, size_t size, double value); + /* Wrappers for custom memory functions */ void* jsonp_malloc(size_t size); void jsonp_free(void *ptr); diff --git a/src/load.c b/src/load.c index 9dc971a..36fc9e9 100644 --- a/src/load.c +++ b/src/load.c @@ -524,12 +524,7 @@ static int lex_scan_number(lex_t *lex, int c, json_error_t *error) lex_unget_unsave(lex, c); - saved_text = strbuffer_value(&lex->saved_text); - errno = 0; - value = strtod(saved_text, &end); - assert(end == saved_text + lex->saved_text.length); - - if(errno == ERANGE && value != 0) { + if(jsonp_strtod(&lex->saved_text, &value)) { error_set(error, lex, "real number overflow"); goto out; } diff --git a/src/strconv.c b/src/strconv.c new file mode 100644 index 0000000..2487060 --- /dev/null +++ b/src/strconv.c @@ -0,0 +1,108 @@ +#include +#include +#include +#include +#include "jansson_private.h" +#include "strbuffer.h" + +#if JSON_HAVE_LOCALECONV +#include + +/* + - This code assumes that the decimal separator is exactly one + character. + + - If setlocale() is called by another thread between the call to + localeconv() and the call to sprintf() or strtod(), the result may + be wrong. setlocale() is not thread-safe and should not be used + this way. Multi-threaded programs should use uselocale() instead. +*/ + +static void to_locale(strbuffer_t *strbuffer) +{ + const char *point; + char *pos; + + point = localeconv()->decimal_point; + if(*point == '.') { + /* No conversion needed */ + return; + } + + pos = strstr(strbuffer->value, "."); + if(pos) + *pos = *point; +} + +static void from_locale(char *buffer) +{ + const char *point; + char *pos; + + point = localeconv()->decimal_point; + if(*point == '.') { + /* No conversion needed */ + return; + } + + pos = strstr(buffer, point); + if(pos) + *pos = '.'; +} +#endif + +int jsonp_strtod(strbuffer_t *strbuffer, double *out) +{ + double value; + char *end; + +#if JSON_HAVE_LOCALECONV + to_locale(strbuffer); +#endif + + errno = 0; + value = strtod(strbuffer->value, &end); + assert(end == strbuffer->value + strbuffer->length); + + if(errno == ERANGE && value != 0) { + /* Overflow */ + return -1; + } + + *out = value; + return 0; +} + +int jsonp_dtostr(char *buffer, size_t size, double value) +{ + int ret; + size_t length; + + ret = snprintf(buffer, size, "%.17g", value); + if(ret < 0) + return -1; + + length = (size_t)ret; + if(length >= size) + return -1; + +#if JSON_HAVE_LOCALECONV + from_locale(buffer); +#endif + + /* Make sure there's a dot or 'e' in the output. Otherwise + a real is converted to an integer when decoding */ + if(strchr(buffer, '.') == NULL && + strchr(buffer, 'e') == NULL) + { + if(length + 2 >= size) { + /* No space to append ".0" */ + return -1; + } + buffer[length] = '.'; + buffer[length + 1] = '0'; + length += 2; + } + + return (int)length; +} diff --git a/test/bin/json_process.c b/test/bin/json_process.c index 1ed0c57..66c34d1 100644 --- a/test/bin/json_process.c +++ b/test/bin/json_process.c @@ -5,12 +5,20 @@ * it under the terms of the MIT license. See LICENSE for details. */ +#ifdef HAVE_CONFIG_H +#include +#endif + #include #include #include #include #include +#if HAVE_LOCALE_H +#include +#endif + static int getenv_int(const char *name) { char *value, *end; @@ -55,6 +63,10 @@ int main(int argc, char *argv[]) json_t *json; json_error_t error; +#if HAVE_SETLOCALE + setlocale(LC_ALL, ""); +#endif + if(argc != 1) { fprintf(stderr, "usage: %s\n", argv[0]); return 2; diff --git a/test/suites/api/test_array.c b/test/suites/api/test_array.c index 18b4652..872d52a 100644 --- a/test/suites/api/test_array.c +++ b/test/suites/api/test_array.c @@ -387,7 +387,7 @@ static void test_circular() } -int main() +static void run_tests() { test_misc(); test_insert(); @@ -395,6 +395,4 @@ int main() test_clear(); test_extend(); test_circular(); - - return 0; } diff --git a/test/suites/api/test_copy.c b/test/suites/api/test_copy.c index 6310f69..716e671 100644 --- a/test/suites/api/test_copy.c +++ b/test/suites/api/test_copy.c @@ -307,7 +307,7 @@ static void test_deep_copy_object(void) json_decref(copy); } -int main() +static void run_tests() { test_copy_simple(); test_deep_copy_simple(); @@ -315,5 +315,4 @@ int main() test_deep_copy_array(); test_copy_object(); test_deep_copy_object(); - return 0; } diff --git a/test/suites/api/test_dump.c b/test/suites/api/test_dump.c index 17d588b..97544ba 100644 --- a/test/suites/api/test_dump.c +++ b/test/suites/api/test_dump.c @@ -133,10 +133,9 @@ static void encode_other_than_array_or_object() } -int main() +static void run_tests() { encode_twice(); circular_references(); encode_other_than_array_or_object(); - return 0; } diff --git a/test/suites/api/test_dump_callback.c b/test/suites/api/test_dump_callback.c index cb71e1b..83e9045 100644 --- a/test/suites/api/test_dump_callback.c +++ b/test/suites/api/test_dump_callback.c @@ -26,7 +26,7 @@ static int my_writer(const char *buffer, size_t len, void *data) { return 0; } -int main(void) +static void run_tests() { struct my_sink s; json_t *json; @@ -78,6 +78,4 @@ int main(void) json_decref(json); free(dumped_to_string); free(s.buf); - return EXIT_SUCCESS; } - diff --git a/test/suites/api/test_equal.c b/test/suites/api/test_equal.c index ba7ab43..d7e2f61 100644 --- a/test/suites/api/test_equal.c +++ b/test/suites/api/test_equal.c @@ -180,11 +180,10 @@ static void test_equal_complex() /* TODO: There's no negative test case here */ } -int main() +static void run_tests() { test_equal_simple(); test_equal_array(); test_equal_object(); test_equal_complex(); - return 0; } diff --git a/test/suites/api/test_load.c b/test/suites/api/test_load.c index 2a60eb3..d60869d 100644 --- a/test/suites/api/test_load.c +++ b/test/suites/api/test_load.c @@ -50,11 +50,9 @@ static void disable_eof_check() json_decref(json); } -int main() +static void run_tests() { file_not_found(); reject_duplicates(); disable_eof_check(); - - return 0; } diff --git a/test/suites/api/test_loadb.c b/test/suites/api/test_loadb.c index 27ea575..60ebf4c 100644 --- a/test/suites/api/test_loadb.c +++ b/test/suites/api/test_loadb.c @@ -9,7 +9,7 @@ #include #include "util.h" -int main() +static void run_tests() { json_t *json; json_error_t error; @@ -33,6 +33,4 @@ int main() if(strcmp(error.text, "']' expected near end of file") != 0) { fail("json_loadb returned an invalid error message for an unclosed top-level array"); } - - return 0; } diff --git a/test/suites/api/test_memory_funcs.c b/test/suites/api/test_memory_funcs.c index 1a6681f..abecac6 100644 --- a/test/suites/api/test_memory_funcs.c +++ b/test/suites/api/test_memory_funcs.c @@ -75,10 +75,8 @@ static void test_secure_funcs(void) create_and_free_complex_object(); } -int main() +static void run_tests() { test_simple(); test_secure_funcs(); - - return 0; } diff --git a/test/suites/api/test_number.c b/test/suites/api/test_number.c index ff0741e..5b9de12 100644 --- a/test/suites/api/test_number.c +++ b/test/suites/api/test_number.c @@ -8,7 +8,7 @@ #include #include "util.h" -int main() +static void run_tests() { json_t *integer, *real; int i; @@ -39,6 +39,4 @@ int main() json_decref(integer); json_decref(real); - - return 0; } diff --git a/test/suites/api/test_object.c b/test/suites/api/test_object.c index 0499f76..59f85b9 100644 --- a/test/suites/api/test_object.c +++ b/test/suites/api/test_object.c @@ -437,7 +437,7 @@ static void test_preserve_order() json_decref(object); } -int main() +static void run_tests() { test_misc(); test_clear(); @@ -446,6 +446,4 @@ int main() test_set_nocheck(); test_iterators(); test_preserve_order(); - - return 0; } diff --git a/test/suites/api/test_pack.c b/test/suites/api/test_pack.c index ccab051..0af94b7 100644 --- a/test/suites/api/test_pack.c +++ b/test/suites/api/test_pack.c @@ -11,7 +11,7 @@ #include #include "util.h" -int main() +static void run_tests() { json_t *value; int i; @@ -227,6 +227,4 @@ int main() if(json_pack_ex(&error, 0, "{s:s}", "foo", "\xff\xff")) fail("json_pack failed to catch invalid UTF-8 in a string"); check_error("Invalid UTF-8 string", "", 1, 4, 4); - - return 0; } diff --git a/test/suites/api/test_simple.c b/test/suites/api/test_simple.c index 8c71329..bf18fc7 100644 --- a/test/suites/api/test_simple.c +++ b/test/suites/api/test_simple.c @@ -10,7 +10,7 @@ #include "util.h" /* Call the simple functions not covered by other tests of the public API */ -int main() +static void run_tests() { json_t *value; @@ -180,6 +180,4 @@ int main() json_incref(value); if(value->refcount != (size_t)-1) fail("refcounting null works incorrectly"); - - return 0; } diff --git a/test/suites/api/test_unpack.c b/test/suites/api/test_unpack.c index 9426104..b259b5f 100644 --- a/test/suites/api/test_unpack.c +++ b/test/suites/api/test_unpack.c @@ -11,7 +11,7 @@ #include #include "util.h" -int main() +static void run_tests() { json_t *j, *j2; int i1, i2, i3; @@ -336,6 +336,4 @@ int main() fail("json_unpack nested array with strict validation failed"); check_error("1 array item(s) left unpacked", "", 1, 5, 5); json_decref(j); - - return 0; } diff --git a/test/suites/api/util.h b/test/suites/api/util.h index 83be721..3e1027a 100644 --- a/test/suites/api/util.h +++ b/test/suites/api/util.h @@ -8,8 +8,16 @@ #ifndef UTIL_H #define UTIL_H +#ifdef HAVE_CONFIG_H +#include +#endif + #include #include +#if HAVE_LOCALE_H +#include +#endif + #include #define failhdr fprintf(stderr, "%s:%s:%d: ", __FILE__, __FUNCTION__, __LINE__) @@ -52,4 +60,15 @@ } \ } while(0) + +static void run_tests(); + +int main() { +#ifdef HAVE_SETLOCALE + setlocale(LC_ALL, ""); +#endif + run_tests(); + return 0; +} + #endif -- 2.1.4