[Bug] Remote config thread crashes getting timezone on non-English Windows systems

[triplef]

[REQUIRED] Please fill in the following fields:

• Pre-built SDK from the website or open-source from this repo: open-source (self built)
• Firebase C++ SDK version: 11.2.0
• Problematic Firebase Component: Remote Config
• Other Firebase Components in use: Auth
• Platform you are using the C++ SDK on: Windows
• Platform you are targeting: desktop

[REQUIRED] Please describe the issue here:

We’re seeing the following crash on first launch when initializing remote config. This seems to happen only if the system language is not English (tested with German), and can be resolved by changing the time zone. Afterwards changing the timezone back works fine, possibly due to caching of remote config data.

I believe this is related to the changes in #1332 by @jonsimantov. The crash occurs in line 116 here:

firebase-cpp-sdk/app/src/locale.cc

Lines 114 to 116 in ab2fd00

	// Convert time zone name to wide string
	std::wstring_convert<std::codecvt_utf8<wchar_t>> to_utf16;
	std::wstring windows_tz_utf16 = to_utf16.from_bytes(windows_tz_utf8);

Crash log:

OS Version: Windows 10.0.19045 (3086)
Report Version: 104

Crashed Thread: 8596

Application Specific Information:
Fatal Error: Unhandled C++ Exception / 0x7ffe65b6cf19

Thread 8596 Crashed:
0   KERNELBASE.dll                  0x7ffe65b6cf19      RaiseException
1   VCRUNTIME140.dll                0x7ffe2948671f      _CxxThrowException (throw.cpp:74)
2   MyApp.exe                       0x7ff7611db2a1      std::_Throw_range_error (stdexcept:166)
3   MyApp.exe                       0x7ff7611dc598      std::wstring_convert<T>::from_bytes (xlocbuf:396)
4   MyApp.exe                       0x7ff761285361      [inlined] std::wstring_convert<T>::from_bytes (xlocbuf:366)
5   MyApp.exe                       0x7ff761285361      firebase::internal::GetTimezone (locale.cc:116)
6   MyApp.exe                       0x7ff7611f2edc      firebase::remote_config::internal::RemoteConfigREST::SetupRestRequest (rest.cc:136)
7   MyApp.exe                       0x7ff7611f26fa      firebase::remote_config::internal::RemoteConfigREST::Fetch (rest.cc:71)
8   MyApp.exe                       0x7ff76116f2c8      firebase::remote_config::internal::RemoteConfigInternal::FetchInternal (remote_config_desktop.cc:597)
9   MyApp.exe                       0x7ff76116f648      <lambda>::<T> (remote_config_desktop.cc:175)
10  MyApp.exe                       0x7ff761170261      [inlined] firebase::callback::CallbackVariadic<T>::RunInternal (callback.h:341)
11  MyApp.exe                       0x7ff761170261      firebase::callback::CallbackVariadic<T>::Run (callback.h:332)
12  MyApp.exe                       0x7ff7611623e7      firebase::scheduler::Scheduler::TriggerCallback (scheduler.cc:189)
13  MyApp.exe                       0x7ff761162232      firebase::scheduler::Scheduler::WorkerThreadRoutine (scheduler.cc:169)
14  MyApp.exe                       0x7ff76116ab8e      [inlined] std::invoke (type_traits:1574)
15  MyApp.exe                       0x7ff76116ab8e      std::thread::_Invoke<T> (thread:55)
16  ucrtbase.dll                    0x7ffe65731bb1      thread_start<T>
17  KERNEL32.DLL                    0x7ffe668e7613      BaseThreadInitThunk
18  ntdll.dll                       0x7ffe680826f0      RtlUserThreadStart

Steps to reproduce:

1. Set system language to e.g. German:
   • Open Windows 10 settings
   • Select "Time & language" section
   • Click "Add Language" and select "Deutsch (Deutschland)" / "German (Germany)"
   • Make German the primary language by moving it to the top of the list of preferred languages
   • Reboot
2. Initialize Firebase Remote Config

[jonsimantov]

Hmm, interesting. It looks like a range_error occurs. Could there be a character included in the UTF8 string that can't be specified in UTF16? If that's the case, I think changing from codecvt_utf8 to codecvt_utf8_utf16 could fix this. Do you know what the value of windows_tz_utf8 is? (You may be able to find this by seeing what the "TZ" environment variable is set to.)

[jonsimantov]

Also, one thing that would be helpful, on your machine set to German: Could you run the following command in the Windows shell and paste the output here?

tzutil /l

This will list all of the time zones that Windows knows about, so I can see what their localized names are and determine if that's what's causing the issue.

[triplef]

I was able to reproduce this in the debugger. Here is the output from get_tzname():

And here the std::string before the conversion:

The string should be "Mitteleuropäische Zeit", but as you can see the umlaut is not being interpreted correctly. I’m not entirely sure where it’s going wrong, but I believe the input is not actually UTF-8 encoded. Maybe this helps:

And here’s the output from tzutil /l:
tzutil-list.txt

[jonsimantov]

Aha, you're right, looks like it's actually Windows CP-1252-encoded and not UTF-8. It's difficult to find documentation on what encoding Windows uses for the TZ environment variable used by _tzset, so assuming it's UTF-8 seems like the error here. Let's see what happens if I switch it to encode manually.

Since you are self-building the SDK, I'll send you a branch to build from to test this out once I have it fixed. Thanks for your help!

[jonsimantov]

Also, weird that tzutil's time zone list doesn't actually have that time zone you listed. Is that the value of your TZ environment variable?

[jonsimantov]

@triplef Could you build the fix-for-windows-time-zone-name branch (#1375) and see if that fixes the issue?

[triplef]

Thanks @jonsimantov. I tried that branch and it doesn’t crash any more, but stepping through the code it looks like it’s still unable to get the timezone, and the strings don’t look correct yet:

[jonsimantov]

Thanks for following up. I'll add some tests to make sure the conversion is correct and will follow up shortly. BTW, what's the value of your TZ environment variable? Do you have any other environment variables that contain a time zone that matches any of the tzutil /l entries?

[jonsimantov]

Added a unit test and found the issue—signed char strikes again! So if you pull the latest contents of the branch, it should convert correctly now. Could you give it a try?

[triplef]

I tried the latest branch, but got_time_zone is still false:

I don’t seem to have a TZ environment variable set. To reproduce the bug I think you just need to change your system language to German and the time zone to this (probably also works with other languages/timezones containing non-ASCII characters):

[jonsimantov]

Another question - when got_time_zone ended up false, did it return the UTF-8 time zone name correctly as a fallback?

[jonsimantov]

OK, I think I finally got a proper fix in place. Rather than doing all of this conversion, I just spin up a thread and use SetThreadUILanguage to ensure the returned time zone name is compatible with the IANA conversion library.

Could you grab the latest version of the branch and give it a try?

[triplef]

Thanks @jonsimantov, works great now!

I actually just now remembered that I implemented the same a while ago, but instead of spawning another thread I just set the thread language temporarily – maybe that’s a simpler/faster option? Sorry I did’t remember that earlier...

[jonsimantov]

Thanks so much for the help, @triplef! The change has been merged in, and will be included in the upcoming release of the Firebase C++ SDK (11.3.0) and the Firebase Unity SDK (11.3.0). Closing this bug for now, but feel free to reopen if it happens again.