godotengine / godotengine/godot

UTF-8 Strings are incorrectly parsed as latin1.

Open
#100,641 9 comments 5 reactions 0 assignees View on GitHub
discussion enhancement topic:core
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.