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

Incorrect parsing of double, float, decimal in Cultures without "." as decimal separator in JSON store #115

Open
ismdiego opened this issue Jul 22, 2021 · 2 comments

Comments

@ismdiego
Copy link

ismdiego commented Jul 22, 2021

If you use a JSON store, you will face this problem with a config file like this:

{
   "num1": 2.5
}

If you execute this trivial code:

IAppSettings settings = new ConfigurationBuilder<IAppSettings>()
    .UseJsonFile(fileName)
    .Build();
var num1 = settings.num1;

// ...
public interface IAppSettings
{
    float num1 { get; set; }
}

You will find out that num1 == 0 (!!!!!). This is caused by a bug in GetStringValue method in JsonFileConfigStore which is using ToString() to convert back the read value into a string. This ToString() takes Culture into account, and so it converts 2.5 to "2,5" (notice the comma) in Cultures with that decimal separator (Spain, as in my case). Afterwards, the ValueHandler ParseValue method fails in the TryParse part because of that, and returns the default value (0 in this case).

This is easily fixed in JsonFileConfigStore changing:

private string GetStringValue(JToken token)
{
    if (token == null) return null;

    return token.ToString();
}

to:

private string GetStringValue(JToken token)
{
    if (token == null) return null;

    return token.Value<string>();
}

Can you please integrate this into the library? It is a safe change IMHO. And this is a really nasty bug...

As a temporary workaround, if you put:

{
   "num1": "2.5"
}

Then the parser works well... and the value is converted to float

MichaelRobl added a commit to MichaelRobl/config that referenced this issue Sep 24, 2021
Fixed the problem for issue:
aloneguid#115
@martin-braun
Copy link

I experience the same problem. My culture locale is "de-DE" and fractioned values will fail to parse properly. @aloneguid would you please apply the PR and publish a new version?

aloneguid pushed a commit that referenced this issue Dec 9, 2021
Fixed the problem for issue:
#115
@martin-braun
Copy link

@aloneguid Thanks, it works, this can be closed.

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

No branches or pull requests

2 participants