Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Outputs invalid JSON int values (erroneously uses locale formatting) #722

Closed
davidljung opened this issue Oct 29, 2019 · 5 comments
Closed
Labels

Comments

@davidljung
Copy link

When I pass in a Jsonnet string like:
{ type: "int", limited: true, min: 0, max: 2000, params: [ "RS34" ] }

and execute it using the C++ API via evaluateSnippet(), the resulting string is:

{ "limited": true, "max": 2,000, "min": 0, "params": [ "RS34" ], "type": "int" }

which isn't valid JSON (notice the extra comma for the thousands position for the 2000).

This is likely the result of using a std::ostream for formatting integers, without correctly setting the numpunct to empty (resulting in the comma for the current locale being used). This is US English locale - I suspect for a Euro locale it might result in "2.000"!

@sbarzowski
Copy link
Collaborator

Thank you very much for reporting this. This is exactly the kind of dependence of the environment that we are trying to avoid with Jsonnet, so I would like to fix it asap. Unfortunately, I wasn't able to reproduce the problem.

Could you provide us with the following information:

  • Your OS
  • Jsonnet version that you're using
  • How did you install Jsonnet? Have you compiled it, used a precompiled version or perhaps fetched it from some public repository?
  • The exact locale that you're using (output of locale command)

On my end I tried setting the following envvars:

export LC_ALL=en_US.UTF-8  
export LANG=en_US.UTF-8

And I ran your example, i.e.:

jsonnet -e '{ type: "int", limited: true, min: 0, max: 2000, params: [ "RS34" ] }'

However, the output didn't exhibit the problem you describe. I did that on OS X 10.13.6 with current master and older v0.11.2 version of Jsonnet.

@davidljung
Copy link
Author

OS: Ubuntu 18.04 x86 64
Version: v0.14.0 771845d (just cloned master today from GitHub)
Installation: I actually just ran make from the repo root, then linked directly into my app (didn't install on system)

I'm using the en_US.UTF-8 locale, but I don't think that is directly relevant.
My understanding of C++ locale management isn't 100%, but I believe that the default global C++ locale (as far as std::locate code is concerned) defaults to some kind of classic locale ("C"? "POSIX"? unsure). This 'global' (from the process perspective) locale is used as a prototype when things like iostreams are created.

However, our app explicitly sets the process global locale according to user selected language & region (it is a multilingual app) using code similar to:

  std::locale glocale("en_US.UTF-8"); 
  std::locale::global(glocale);

My guess is that prior to that, the locale is "C" or some default that may not include any numeric punctuation (the C locale can be changed). That is also likely the reason you don't see the problem with the command-line app - it doesn't change process-global the locale. In C++ a call to std::setlocale("") sets the locale to the one defined by the environment (e.g. LC_ALL etc).

Jsonnet, as a 3rd-party library that is generating strings that are for machine consumption (JSON) rather than for output to humans, it should explicitly set some appropriate locale on the streams it is using to generate output, rather than relying on whatever locale settings are in the process global locale. That is, if using a stream to serialize an int to a string, the locale associated with the stream (using imbue()) should be appropriate for JSON (no numeric punctuation etc).
Similarly for other types. For example, if Jsonnet relies on the default bool serialization to "true" and "false" and someone installs once for French that renders ("vrai" and "faux"), then that will show up in the JSON output unless Jsonnet sets the locale on the stream explicitly.

Hope that helps!
Appreciate Jsonnet!

Work-around: for anyone using non-default locale in their C++ code, if you can tolerate globally disabling number punctuation (but still need other locale adaptations), you can disable it via:

mylocale = std::locale(mylocale, new std::numpunct<char>());

@davidljung
Copy link
Author

Here is some minimal code to reproduce it (at least on my system):

#include <iostream>
#include <locale>
#include "include/libjsonnet++.h"

int main()
{
    std::string templatedJSONString { "{ type: \"int\", limited: true, min: 0, max: 2000, params: [ \"RS34\" ] }" };
    std::locale glocale("en_US.UTF-8"); 
    std::locale::global(glocale);

    jsonnet::Jsonnet jsonnet {};
    jsonnet.init();

    std::string expanded {};
    if (!jsonnet.evaluateSnippet("", templatedJSONString, &expanded)) {
        // error
        std::cerr << "Error parsing Jsonnet: "+jsonnet.lastError();
        exit(-1);
    }
    std::cout << expanded << std::endl;
    return 0;
}

@sbarzowski
Copy link
Collaborator

Ah, so apparently the problem does not occur with the command, but when using Jsonnet as a library and there is a global locale setting , it affects Jsonnet output (which it obviously shouldn't).

I was able to reproduce the problem using your example, so I can probably fix it soon.

Thank you very much for the detailed information!

sbarzowski added a commit to sbarzowski/jsonnet that referenced this issue Nov 3, 2019
@sbarzowski sbarzowski added the bug label Nov 23, 2019
sbarzowski added a commit to sbarzowski/jsonnet that referenced this issue Feb 2, 2020
johnbartholomew pushed a commit to johnbartholomew/jsonnet that referenced this issue Feb 29, 2024
johnbartholomew pushed a commit to johnbartholomew/jsonnet that referenced this issue Mar 2, 2024
@johnbartholomew
Copy link
Collaborator

Thanks for the bug report! This should now be fixed on master and included in the next release. (PR #724)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants