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

Fixed processing of non-ASCII symbols missing in CP_THREAD_ACP but present in current KB layout #910

Merged
merged 1 commit into from
Dec 30, 2020
Merged

Fixed processing of non-ASCII symbols missing in CP_THREAD_ACP but present in current KB layout #910

merged 1 commit into from
Dec 30, 2020

Conversation

eugenegff
Copy link

Hi there, my name is Eugene Golushkov, I had problems with Barrier and being the programmer decided to contribute fix.

I'm Ukrainian, this language uses https://en.wikipedia.org/wiki/Cyrillic_script, Unicode range 0x400 - 0x4FF, Windows cp1251

My setup consists of

  1. Windows 10 PC, Barrier Server, ENG + UKR keyboard layouts.
  2. macOS Catalina, Barrier Client, ENG + UKR keyboard layouts.
  3. Windows 10 Tablet, Barrier Client, ENG + UKR keyboard layouts.

Problem description:
When current keyboard layout is ENG everything works like a charm. But if I move mouse cursor to clients (2),(3) having UKR keyboard layout selected on win server (1), then win client (3) ignores key strokes, and macos client (2) enters Latin characters with diacritics instead.

Expected result:

  1. Produce Cyrillic characters on clients if possible
  2. Or fallback to English characters
  3. But never produce Latin characters with diacritics that are absent in both English and Ukrainian

Solution:
I enabled Debug1 log level on macos client (2) and pressed 'A' key that is near CapsLock, in ENG KB layout. Got expected log:
DEBUG1: recv key down id=0x00000061, mask=0x2000, button=0x001e
then I switched to UKR layout on (1) and pressed the same physical key, that was expected to produce U+0444 CYRILLIC SMALL LETTER EF, but got such log:
DEBUG1: recv key down id=0x000000f4, mask=0x2000, button=0x001e
where 0xF4 is code for CYRILLIC SMALL LETTER EF in Windows ANSI codepage cp1251.

It became obvious that Barrier was not able to restore unicode char from ANSI, probably because CP_THREAD_ACP that it tried to use for this purpose is not in any way connected to current HKL. This was even as (1) had "Current language for non-Unicode programs" = Ukrainian. The fix was easy - keyboard driver already produces UTF16 chars that could be obtained by ToUnicode function. ToAscii is actually just simple wrapper around ToUnicode that further converts UTF16 chars to ANSI, and by avoiding this unnecessary step we will fix the problem. Barrier inter-machine protocol is Unicode based (KeyID).

Results:

  1. I built Barrier with fix, without GUI and installer, and substituted binaries into existing Barrier setup. Everything still works on clients (2), (3) with ENG keyboard layout selected on win server (1)
  2. windows client (3) that previously ignored keystrokes - now enters Cyrillic characters with UKR keyboard layout selected on win server (1), and language indicator on (3) synchronizes with language indicator on (1) after the first pressed key.
  3. macos client that previously entered Latin characters with diacritics - now see in log proper key down id 0x00000444, but entered proper English chars - there are still space for improvement, quite possibly I will look there later.

@eugenegff
Copy link
Author

eugenegff commented Oct 14, 2020

I found the reason of ANSI codepages mismatch.

My Windows 10 was installed as English, and Ukrainian was set as "Current language for non-Unicode programs" later. And I don't bother to install Ukrainian Language Pack or somehow change the locale - just added the Ukrainian keyboard layout to otherwise English Windows.

So ToAscii translated UTF16 from KB layout to ANSI using RtlUnicodeToMultiByteN that uses CP_ACP codepage aka "Current language for non-Unicode programs", in my case cp1251 Cyrillic
And then CP_THREAD_ACP was used to translate ANSI to UTF16. CP_THREAD_ACP is set by SetThreadLocale, and while I'm not sure which one of LOCALE_SYSTEM_DEFAULT and LOCALE_USER_DEFAULT was used by Barrier - but on my system all "The settings for the current user, welcome screen (system accounts) and new user accounts" are "English (United States)", therefore CP_THREAD_ACP is cp1252 Western.

Anyway, proposed fix is the ultimate fix for the problem, as it eliminates any dependency on ANSI codepages, by keeping data in Unicode all the time.

@fthdgn
Copy link

fthdgn commented Oct 15, 2020

This PR fixes #527. I tried it and all keys type correct Turkish characters on the MacOS.

My PC too is English but I use Turkish-Q keyboard.

@hurrycaner
Copy link

@eugenegff could you share the release links so i can test it myself since the PR had no response in 13 days?

@eugenegff
Copy link
Author

@eugenegff could you share the release links so i can test it myself since the PR had no response in 13 days?

I did not build full release, as I have no QT, etc. The way I tested it - I configured in CMake build without GUI, installer and tests, and copied built binaries into installed Barrier instance. Here are release binaries built by MSVC 2019 for x64 https://1drv.ms/u/s!AnyscjGR32vZhPxtPPb2RbgB8lIEpw?e=UXYhAC

@eugenegff
Copy link
Author

This PR fixes #527. I tried it and all keys type correct Turkish characters on the MacOS.

My PC too is English but I use Turkish-Q keyboard.

And probably Turkish as Language for non-Unicode programs, meaning CP_ACP = cp1254, while CP_THREAD_ACP = cp1252

@p12tic
Copy link
Member

p12tic commented Oct 26, 2020

This looks great, thank you! I'll still need to look into the PR in more depth, but during cursory review I haven't seen any problems.

@eugenegff
Copy link
Author

This looks great, thank you! I'll still need to look into the PR in more depth, but during cursory review I haven't seen any problems.

Maybe I can help somehow with this pull request? This fix for Windows Barrier server is a prerequisite for further investigation of problem with non-ASCII characters in macOS Barrier client, that I deliberately do not start till the current fix is landed.

@wikiwalk
Copy link

Is there a way to download a distribution from this branch?
I have the problem (Windows 10 client and server, no accented chars from FR and DE keyboards work on the client)
I did try replacing the 3 executables kindly provided by @eugenegff
Unfortunately the server does not start (failed to launch, error: process immediately stopped).

The non-ASCII symbols missing in CP_THREAD_ACP but present in current KB
layout were processed incorrectly. Do not rely on ANSI => UTF16
conversion, obtain UTF16 directly from KB layout driver. BTW, ToAscii is
implemented via ToUnicode + RtlUnicodeToMultiByteN, so this is really
optimization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants