NS_tsnprintf() does not handle %s correctly on Windows
While testing the ESR68-based updater on Windows, Kathy and I found a Windows toolchain problem. We tested using our own 64-bit build (rbm nightly build process) on a Windows 10 system.
We discovered that file paths generated using format strings are corrupted. Specifically, calls to NS_tsnprintf()
do not work correctly; apparently, because that is a "wide" function a %s
when used in the format string is supposed to mean "expect the arg to be of type WCHAR *". Instead, the args are processed as C-style strings which means they get truncated after the first character (at least when using characters that fit within the first 256 Unicode codepoints).
NS_tsnprintf()
is a macro that is defined in toolkit/mozapps/update/common/updatedefines.h (all of the code that uses it is related to the updater). We first noticed that problem when we saw a failure inside updater.cpp's WriteToFile()
function. The following code from that function fails because it tries to move filename
to a bad path.
#if defined(XP_WIN)
NS_tchar dstfilename[MAXPATHLEN] = {NS_T('\0')};
NS_tsnprintf(dstfilename, sizeof(dstfilename) / sizeof(dstfilename[0]),
NS_T("%s\\%s"), gPatchDirPath, aFilename);
if (MoveFileExW(filename, dstfilename, MOVEFILE_REPLACE_EXISTING) == 0) {
return false;
}
#endif
The computed path is C\u
(the first character from gPatchDirPath
followed by the \ and then the first character of aFilename
).
On Windows, NS_tsnprintf()
is defined as mywcsprintf
which is an inline function that uses _vsnwprintf()
. We have not traced this bug deeper than that point, but we did verify that the problems disappear if we replace all occurrences of %s
with %S
in the NS_tsnprintf()
format strings. That is a very ugly fix though, and it would be wrong on macOS and Linux.