godotengine / godotengine/godot
UTF-8 Strings are incorrectly parsed as latin1.
- Dominant language
- C++
- Stars
- 117k
- Forks
- 26.8k
- PR merge metrics
- PR metrics pending
Description
### Tested versions
All versions are affected. This is _old_ code.
### Issue description
The current implementation of Godot has a bug where UTF-8 encoded strings are parsed as `latin1`.
I'd like to hear what you all think is the best solution to this problem.
The `String(const char *)` constructor currently parses `char` strings using latin1 encoding:
https://github.com/godotengine/godot/blob/89001f91d21ebd08b59756841426f540b154b97d/core/string/ustring.h#L615-L617
(`parse_latin1` was [recently renamed](https://github.com/godotengine/godot/pull/100434) from `copy_from` because that's what the function does).
This constructor is used for many strings, including static C strings. The encoding of static C strings is controlled by `-fexec-charset` on GCC / Clang[^1] and `/execution-charset` on MSVC[^2]. In the Godot codebase, it defaults to UTF-8 on GCC / Clang, and is [explicitly set](https://github.com/godotengine/godot/blob/89001f91d21ebd08b59756841426f540b154b97d/platform/windows/detect.py#L448) to UTF-8 on Windows. `latin1` and `utf-8` are compatible for values below 128, and incompatible otherwise. Therefore, there is a mismatch between encodings that can be encountered for non-ascii strings. It is likely that there are mismatches in other, non-static string use-cases, because `latin1` encoded strings are a somewhat rare encounter (though ascii-only strings are pretty likely).
The mismatch has apparently [led to problems](https://github.com/godotengine/godot/pull/100434#discussion_r1885808663) a few times in the past (though I don't know how often). Most times, strings use ascii-only characters, in which range `latin1` and `utf-8` overlap.
### Possible Solutions
Here are some ideas that I came up with:
- Enforce ASCII-only for static C strings (and force the use of `u8"String"` for utf-8 strings).
- Using `-fexec-charset` (and force the use of `u8"String"` for utf-8 strings). This would be the best solution in my opinion, but it doesn't appear to be possible to pass anything else except `UTF-8` to `-fexec-charset` right now, at least in Clang.
- Using external linter tools or options. I don't know if such a tool exists.
- Use UTF-8 parsing for strings for the default constructors. This would be somewhat slower than `latin1` parsing (though accelerated by https://github.com/godotengine/godot/pull/99826). The exact difference would have to be measured and tested for significance.
- Use ASCII-only parsing for strings for the default constructors. Log errors if values > 127 are encountered. This should be negligibly slower than `parse_latin1`, and somewhat faster than `parse_utf8`.
Ideally, the default, implicit `String(char*)` constructor is removed, to avoid this problem in the future. Instead, every use of it should be replaced with an explicit call to `String::utf8` or similar (maybe with the exception of construction from string literals), so we can be sure intent was put behind choosing the encoding. This is a bigger task though and not realistic for the near future.
Solution 1 does not address possible encoding mismatches of non-static string constructor calls.
In either the second or third solution, users that actually _want to_ parse `latin1` have to be switched to use that function explicitly.
[^1]: https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html#index-fexec-charset
[^2]: https://learn.microsoft.com/en-us/cpp/build/reference/execution-charset-set-execution-character-set?view=msvc-170
Contributor guide
Research direction
Read the String(const char *) constructor in core/string/ustring.h and the UTF-8 execution-charset setting in platform/windows/detect.py. Review the linked pull request discussion and compare the proposed ASCII, UTF-8, and explicit-constructor approaches, including their performance implications. Done means a specific encoding strategy is agreed and its affected constructor uses are identified for implementation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- localization
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100