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

Usage of std::stof crashes on Windows when using some non-English languages/locales (i.e. German / de) #2221

Open
taenri opened this issue Feb 9, 2024 · 4 comments
Labels

Comments

@taenri
Copy link
Contributor

taenri commented Feb 9, 2024

Bug

If the machine's locale is set to a language like German (de) or French (fr), rendering SVG paths that contain floating point numbers will crash if the floating point numbers use dots instead of commas as the radix. This is because languages like German expect the usage of commas in floating-point numbers (i.e. ",482" instead of ".482"), and std::stof takes into account the current locale of the application. PathView utilizes std::stof in its parsing logic.

Fix recommendation: std::from_chars should be used instead as it will always use the C-default locale instead of whatever is currently set in the application. I have tested this locally and this seems to work as expected.

Environment info

As far as I know, this reproduces on Windows 11 environments in react-native-svg version 13.14.0, and the latest 14.1.0.

Steps To Reproduce

Issues without reproduction steps or code are likely to stall.

  1. Clone/build react-native-svg as normal for Windows environment.
  2. Force the locale to be German in C++ at the start of PathView::UpdateProperties via std::setlocale(LC_ALL, "de_DE");.
  3. Create a simple SVG path component with the following value for its d property: "m41.213 20.483 6.8889 3.1408m0.48227-3.8605-7.3712 0.71964 7.2985 1.2256"
  4. Build and run. Try to display SVG path from part 3.
  5. Observe that the program crashes due to invalid argument exception in PathView::ParseNumber during the std::stof call.

Describe what you expected to happen:

  1. Any SVG should render on Windows using German (de) locale and not crash. Specifically, SVGs using paths with floating-point values that use decimals for the radix instead of commas.

Short, Self Contained, Correct (Compilable), Example

N/A

If more information is needed, please let me know.

@taenri taenri changed the title Usage of std::stof crashes on Windows when using some non-English languages/locales (i.e. de or fr) Usage of std::stof crashes on Windows when using some non-English languages/locales (i.e. German / de) Feb 9, 2024
@tero-paananen
Copy link

Could usage be something like:

  float result = 0;
  const auto res = std::from_chars(num.data(), num.data() + num.size(), result);
  if (res.ec == std::errc::invalid_argument) {
    throw hresult_invalid_argument(L"Invalid number.");
  } else if (res.ec == std::errc::result_out_of_range) {
    throw hresult_invalid_argument(L"Invalid number, out of range.");
  }

I did not managed to change my Windows locale into de even calling this std::setlocale(LC_ALL, "de_DE")

@taenri
Copy link
Contributor Author

taenri commented Feb 21, 2024

Could usage be something like:

  float result = 0;
  const auto res = std::from_chars(num.data(), num.data() + num.size(), result);
  if (res.ec == std::errc::invalid_argument) {
    throw hresult_invalid_argument(L"Invalid number.");
  } else if (res.ec == std::errc::result_out_of_range) {
    throw hresult_invalid_argument(L"Invalid number, out of range.");
  }

I did not managed to change my Windows locale into de even calling this std::setlocale(LC_ALL, "de_DE")

Yes, usage like that should work!

This should be the correct way to set locale actually:
std::setlocale(LC_ALL, "de_DE.UTF-8");

Here is an isolated example that can repro the same crash:

#include <iostream>
#include <string>

int main()
{
    // std::setlocale(LC_ALL, "de_DE.UTF-8"); // Uncomment to repro crash
    std::string str = ".48227";
    float test = std::stof(str, nullptr);
    std::cout << test << "\n";
}

@bohdanprog
Copy link
Member

CC @marlenecota

@taenri
Copy link
Contributor Author

taenri commented Oct 9, 2024

@bohdanprog / @marlenecota - Any progress on this bug? We need to update RNSVG for other bug fixes, and re-making the patch we need for this bug is becoming cumbersome.

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