Several bugs with pynput.keyboard uinput backend

[ntczkjfg]

Description
I came across several bugs in the uinput backend of pynput.keyboard while trying to run code like this:

from pynput import keyboard

kb = keyboard.Controller()

# Simulate Ctrl+V press
with kb.pressed(keyboard.Key.ctrl):
    kb.tap('v')

All of the bugs are related to the required_modifiers variable received by the Controller class's _handle function, from the _to_vk_and_modifiers function. In particular, two eventually trace back to when self._char_table is defined in the __init__ function of the Layout class. We have:

        self._char_table = {
            as_char(key): (
                vk,
                set()
                    | {Key.shift} if i & 1 else set()
                    | {Key.alt_gr} if i & 2 else set())
            for vk, keys in self._vk_table.items()
            for i, key in enumerate(keys)
            if key is not None and as_char(key) is not None}

I found this pretty difficult to parse - but here is some selected values for vk, keys, i, and key, when it's building self._char_table for 'v':

vk = 47, keys = <normal: 'v', shifted: 'V', alternative: 'v', shifted alternative: 'V'>
        i = 0, key = 'v'
        i = 1, key = 'V'
        i = 2, key = 'v'
        i = 3, key = 'V'

Now for the actual bugs:

Bug 1: Incorrect modifiers set for shifted alternative characters, due to order of operations precedence issue

When i == 3, we have bool(i & 1) == True and bool(i & 2) == True - and I believe the code would therefore like the set created to be {Key.shift, Key.alt_gr}. However, there's an order of operations issue going on, and it only comes out as {Key.shift}. This can be fixed by adding additional parenthesis as such:

        self._char_table = {
            as_char(key): (
                vk,
                set()
                    | ({Key.shift} if i & 1 else set())
                    | ({Key.alt_gr} if i & 2 else set()))
            for vk, keys in self._vk_table.items()
            for i, key in enumerate(keys)
            if key is not None and as_char(key) is not None}

Test case: x = (set() | {'x'} if True else set() | {'y'} if True else set()) will yield x == {'x'} instead of the desired x == {'x', 'y'}. I believe Python interprets it as x = (set() | {'x'} if True else (set() | {'y'} if True else set())). The fix is equivalent to x = (set() | ({'x'} if True else set()) | ({'y'} if True else set())).

Bug 2: Earlier dict entries are overwritten by later ones, favoring unnecessary modifiers

Because a given key in keys can repeat (we have two copies of both 'v' and 'V' for the V key from above, for example), earlier entries in self._char_table get overwritten by later ones with more modifiers. So the code tries to set the following entries to self._char_table, in this order:

key: value
'v': set()
'V': {Key.shift}
'v': {Key.alt_gr}
'V': {Key.shift, Key.alt_gr} # Using the above fix to bug # 1

And so even though we should have self._char_table['v'] == set() and self._char_table['V'] == {Key.shift}, we actually get self._char_table['v'] == {Key.alt_gr} and self._char_table['V'] == {Key.shift, Key.alt_gr} (again, with the fix to bug # 1 incorporated). My suggested fix to this is to iterate over keys backwards, so if a key repeats, then the one with fewer modifiers is always preferred:

        self._char_table = {
            as_char(key): (
                vk,
                set()
                    | ({Key.shift} if i & 1 else set())
                    | ({Key.alt_gr} if i & 2 else set()))
            for vk, keys in self._vk_table.items()
            for i, key in reversed(list(enumerate(keys)))
            if key is not None and as_char(key) is not None}

(incorporates fix to both bug #1 and bug #2)

With this fix we do get self._char_table['v'] == set() and also self._char_table['V'] == {Key.shift}.

Bug 3: Conditional check required_modifiers is not None will evaluate to True, even when it's the empty set

In the Controller class, in the _handle function, we get required_modifiers, which will be the set created above for a given key. Note above that the set will always be, at a minimum, set() - but in _handle, we have the check required_modifiers is not None - which will always evaluate to True for this. (required_modifiers can also be None, from some other branches of the self._to_vk_and_modifiers function, so a potential fix must account for both cases)

This is a problem because after that check, it runs code that releases any modifiers which are currently pressed, which are not part of required_modifiers. So the effect is that if a key doesn't require any modifiers, this code releases all modifiers anyways. This goes back to my original code sample, where I hold down ctrl and press v - this bug will make it release ctrl before pressing v, even within the with kb.pressed(keyboard.Key.ctrl): block, defeating the point of the block entirely.

The suggested fix here is simple: Just remove the is not None from the check. So the conditional becomes:

if is_press and required_modifiers:

Bug 4: Too many modifiers released

That said, let's say there are required modifiers - like what if we ran this code?

with kb.pressed(keyboard.Key.ctrl):
    kb.tap('V')

Then, we will have required_modifiers == {Key.shift}, correctly. But the code within that conditional from bug # 3 will release the ctrl key before pressing 'V'.

I'm less confident about this suggested fix, but I think it would make more sense if the only keys we could release in this case are Key.shift and Key.alt_gr - or maybe even just get rid of the to_release code all together. I might do it like this:

                to_release = {
                    getattr(evdev.ecodes, key.value._kernel_name)
                    for key in (modifiers - required_modifiers)} & {Key.shift, Key.alt_gr}

It might also be worth adding new code to specifically disallow releasing a key within that key's with kb.pressed block.

With those four changes, the uinput backend to pynput.keyboard works fine for me as a standard keyboard controller - albeit with just standard keyboard keys, not arbitrary unicode.

Platform and pynput version
Fedora Linux 42
KDE 6.4.1 (Wayland)
Pynput 1.8.1

[moses-palmer]

Thank you very much for your very thorough report!

Bug 1: Incorrect modifiers set for shifted alternative characters, due to order of operations precedence issue

This is, to be frank, quite embarrassing! I have incorporated you suggested fix in the fixup/uinput branch.

Bug 2: Earlier dict entries are overwritten by later ones, favoring unnecessary modifiers

I have incorporated this suggestion as well in the same branch.

Bug 3 and Bug 4

These ones I am not as certain about. I believe that the None check is to distinguish special keys---those with vk set---from other keys, and not change the modifier state in those cases. I vaguely remember the reason for the to_release and to_press sets being that I wanted pynput.keyboard.Controller.tap to actually type the character, but considering that other backends allow use such as your ctrl+V example above, this might be misguided.

[ntczkjfg]

Thank you for the response - I'm glad to hear about the inclusion of the first two fixes ^^

I would still defend the need to modify the behavior in the case for bugs 3 and 4 - otherwise, is there any way at all for this backend to send commands like ctrl+v, even without the use of .tap? While I still personally prefer both fixes, I could see defending leaving bug 3 as-is - but would limiting the modifiers that can be released to just Shift and Alt_gr, as suggested for bug 4, be a reasonable compromise? That seems in-line with the intention of the code, at least - that it sets those two modifiers right to ensure the character requested is the character typed. Allowing ctrl/super/alt/whatever else to be held down doesn't remove from that.

This is particularly relevant to me because I do need to output arbitrary unicode, which works fine with pynput on Windows and X11 (mostly) - but on Wayland pynput can't do that (yet?). So on it I'm using pynput for keyboard monitoring, then when I need to output I'm using wl-clipboard to store the current system clipboard, change the clipboard to what I need to output, sending a single ctrl+v event with pynput, then restoring the original system clipboard. I'm accomplishing that right now with modified pynput files using my fixes above - it would be nice if I could just use stock pynput on wayland though.

Honestly I just really hate trying to do automation stuff like this in wayland. I hope it gets easier before it becomes the only option. I had to give up on mouse control entirely - I could get clicking and scrolling to work, but the best I managed for motion was imprecise relative motion. I appreciate how much you've done to make this sort of thing as simple as it is on Windows and X11.