Bug: cpp-terminal crashes if two utf8 are inserted simultaneously

[TobiasWallner]

I have a german keyboard with the 'ö' and 'ä' keys. In utf8 both are 2 bytes long. When I pressed them at the same time accidentally the cpp-terminal crashed. It crashed to such an extend that my terminal closed. I then tried the same not in my application but the kilo editor from the examples. The same thing happened and the kilo editor crashed as well and my terminal closed again. I tried it on the windows cmd terminal and power-shell - same result. But I think it has nothing to do with the terminal, since I can just enter both keys at the same time in the terminal command prompt without trouble .

[flagarde]

@TobiasWallner cpp-terminal is still not full utf8... Could you give me the code you are using? Normally if you press a key that is converted to two or more bytes it should be skip by the software without appearing for now..

Kilo is a bit long code and C to debug.. maybe your code is smaller ^^

[TobiasWallner]

Ahahaha, nah my code is definetly larger. I may find some time to get a minimal example to crash too. Will post it later.

[flagarde]

@TobiasWallner even larger if it's more C++ oriented it would be easier to understand

[certik]

@TobiasWallner thanks for reporting this! Interesting bug. I wonder if somehow the two utf inputs collide. I tried it with a Czech keyboard (ěščřžýýáíé) in the kilo editor, but it doesn't seem to show any of the characters, but also didn't crash. I then tried the multiline prompt, but that didn't work for me at all (https://github.com/jupyter-xeus/cpp-terminal/issues/260). I then tried ./minimal, and that works and I can't reproduce this either.

[TobiasWallner]

I have generated a minimal program where the bug appears for me. You can see it on a branch (utf8_bug_minimal) from my fork of cpp-terminal as an examples program. Here is the code: https://github.com/TobiasWallner/cpp-terminal/blob/utf8-bug-minimal/examples/utf8_bug_minimal.cpp

[TobiasWallner]

I only tried it on windows but with GCC, Clang and MSVC. The clang build was the most robust and I had to press 'ö' and 'ä' at the same time a view times before it crashed. GCC and MSVC crashed on the first or second try.

Have not tried it on linux though.

But other key combinations that just use the ASCII representation do not trigger the terminal to crash.

[TobiasWallner]

Also when the terminal crashed, the shell and everything is gone. But when I look in my task manager there is still a running prozess from the just crashed terminal.

[flagarde]

@certik The behaviour you have it's the one expected for now (ěščřžýýáíé) and all utf8 characters should be eaten and not appear on screen (for now). @TobiasWallner The behaviour you see is not expected.

I can have a look but maybe #255 should be merged before to avoid merging problem.

[TobiasWallner]

That is odd, because in my other main application I have no problems reading and displaying utf8 characters with your library when they are being pressed one at a time.

[TobiasWallner]

Also, when I enter utf8 characterd in the kilo editor, it displays them correctly, but there seems to be a bug where the cursor displayed on the screen is being advanced to far. for utf8 characters it advances the cursor by the number of bytes of the utf8 character instead of the number of utf8 complete characters.

[certik]

Probably there are some bugs on Windows regarding utf8.

[flagarde]

Yes windows has always some bug with utf8 even after so many years of trying to do it properly. It's hard to know if it's a library problem or windows problems. I think we should focus on very basic tests for utf8. can you give green light for merging #255 I will then try to code the right stufs to deal with utf8 on terminal input.

Windows is very misleading about UNICODE (utf16) and ANSII which can be turn to utf8 nowdays (maybe with some bugs)

[flagarde]

@TobiasWallner kilo is quite a big program and if I understand correctly was adapted from C code to this library.. Then I modify quite a lot how the library is working so maybe the C/C++ mixing create some problem especially if the program assure or character is one byte etc...

It surely need some polishing but I'm still focusing on the library itselft

[flagarde]

Maybe these big programs could be moved in a bin folder while examples could be used only for small program to emphasis features ? Or to have some basic program for testing. This library is difficult to auto-test so it would be nice to have very simple exmaples that user could use to event test his terminal or to check i everything is fine in his OS like colors or cin

[TobiasWallner]

@flagarde regarding reading utf8. In my university project I already have a state machine that can read utf8 symbols from byte by byte inputs. After this wednesday, when the project is over (from the university side) I will upload it on github, in there is also my utf8 character and utf8 string implementation, that may be useful for you.

[flagarde]

@TobiasWallner nice would be very useful for sure thx !

[flagarde]

@certik @TobiasWallner I found a fix (Windows is so buggy featured)

[TobiasWallner]

wow, nice, awesome, did not expect that, especially so quickly.

[flagarde]

@TobiasWallner Seems to work but i'm not sure maybe there is cornercases. I will submit tomorrow and you can have a try if you want ^^

[flagarde]

@TobiasWallner Should be fix please have a try on #261

[TobiasWallner]

will try, thanks

[flagarde]

@TobiasWallner I added your example code https://github.com/TobiasWallner/cpp-terminal/blob/utf8-bug-minimal/examples/utf8_bug_minimal.cpp to the repo. Hope you don't mind. You can then test it directly

[TobiasWallner]

I checked out your Utf8 bug fix. It fixed the problem with the crashing terminal.

[TobiasWallner]

I tried to implement it in my own project and a lot of keys seemed to not work anymore like any utf8, enter and backspace. I guess the utf8 are not working because you now return the utf8 directly as one symbol and not byte by byte, event by event any more. Could not figure out why enter and backspace did not work maybe my project got too out of date with cpp-terminal? Unfortunatelly I will not have the time to look at it further this week.

[TobiasWallner]

as pointed out in an earlier comment, I would like to show you my implementation of a utf8::char and utf8::string in my current project. I will probably turn them into their own repo and library. If you find them usefull, feel free to use them.

https://github.com/TobiasWallner/Lime/tree/main/modules/utf8_string

[flagarde]

I tried to implement it in my own project and a lot of keys seemed to not work anymore like any utf8, enter and backspace. I guess the utf8 are not working because you now return the utf8 directly as one symbol and not byte by byte, event by event any more. Could not figure out why enter and backspace did not work maybe my project got too out of date with cpp-terminal? Unfortunatelly I will not have the time to look at it further this week.

What do you mean by not working anymore ?

[flagarde]

@TobiasWallner This is working for me on linux :

#include <cpp-terminal/event.hpp>
#include <cpp-terminal/input.hpp>
#include <cpp-terminal/key.hpp>
#include <cpp-terminal/terminal.hpp>

int main()
{
  try
  {
    // set options for the console
    Term::terminal.setOptions(Term::Option::NoClearScreen, Term::Option::NoSignalKeys, Term::Option::Cursor, Term::Option::Raw);
    // initial render of the whole screen
    Term::terminal << "CTRL + Q to end" << std::endl;
    bool main_loop_continue = true;
    while(main_loop_continue)
    {
      auto event = Term::read_event();
      switch(event.type())
      {
        case Term::Event::Type::Empty:
        {
          Term::terminal << "Event: Empty" << std::endl;
          break;
        }
        case Term::Event::Type::Key:
        {
          Term::Key keyEvent = event;
          if(keyEvent==Term::Key::BACKSPACE) Term::terminal << "Event: Key (backspace)" << std::endl;
          else if(keyEvent==Term::Key::TAB) Term::terminal << "Event: Key (tab)" << std::endl;
          else if(keyEvent.iscntrl()) Term::terminal << "Event: Key (control key)" << std::endl;
          else Term::terminal << "Event: Key (" << keyEvent.str() << ")" << std::endl;
          if(keyEvent == Term::Key::CTRL_Q) main_loop_continue = false;
          break;
        }
        case Term::Event::Type::Screen:
        {
          Term::terminal << "Event: Screen" << std::endl;
          break;
        }
        case Term::Event::Type::Cursor:
        {
          Term::terminal << "Event: Cursor" << std::endl;
          break;
        }
        case Term::Event::Type::CopyPaste:
        {
          Term::terminal << "Event: CopyPaste (" << static_cast<std::string>(event) << ")" << std::endl;
          break;
        }
        default:
        {
          Term::terminal << "Event: Unknown" << std::endl;
          break;
        }
      }
    }
  }
  catch(...)
  {
    Term::terminal << "There was an exception!" << std::endl;
  }
  return 0;
}

[TobiasWallner]

I mean that for example in my current application I have the following switch statement to prozess key events:

switch(character_32){
        case Term::Key::CTRL_Q : this->quit(); break;
        case Term::Key::ESC : this->toggle_command_line(); break;
        case Term::Key::ENTER : this->activeEditor->enter(); break; 
        case Term::Key::BACKSPACE : this->activeEditor->Delete(); break;
        case Term::Key::DEL: this->activeEditor->erase(); break;
        ...

and on the master branch on the commit: 49e3160358237a3af25e7c7c3e70e593812cd0fe if I hit the enter key, then for example the line: (case Term::Key::ENTER : this->activeEditor->enter(); break; ) in the switch statement gets triggered and executed.

If I now switch to your new Utf8 branch and I rebuild and launch my application and hit the enter key on my keyboard, said line no longer gets invoked.

[flagarde]

@TobiasWallner this is very strange maybe it's a problem how you convert to character_32 ? because the enter should be be change as it's a one byte utf8 so it should not be changed

[TobiasWallner]

const auto character_32 = static_cast<int32_t>(keyEvent); But i will debug it now and look what is going on.

[flagarde]

@TobiasWallner very strange indeed because enter is a one byte utf8 so it should not be parsed by the fix... Maybe auto is doing some strange stuf I never use auto. IMHO it's a pythonisation that c++ standard should have avoid

[TobiasWallner]

Ok, so it seems like a press on enter will not generate an event using this function: auto event = Term::read_event(); I added break points around them and tried different key, from the alphabete (which worked just fine) and enter which did not trigger an event

[TobiasWallner]

I also tried it in the kilo editor. Enter will not work anymore, as well as ,;.:_-#'+*?

[flagarde]

@TobiasWallner I'm not able to reproduce the bug with win11 and kilo

Maybe you need to merge all the commits

[TobiasWallner]

I did as you told and merged all the commits, but it is still not working for me:

Here is a link to a video displaying that keys are not working. First I show you that I am on the master branch the commit I am on and that all keys are working in the kilo edior. Then I merge utf8 and show that some keys are not working anymore.

http://gofile.me/78Gyd/tliTQifIE

[flagarde]

@TobiasWallner Could you give me some info about your os Windows version etc ?

[flagarde]

@TobiasWallner Coudl you check if the terminal on your system in not turned into 'legacy mode' ? https://learn.microsoft.com/en-us/windows/console/legacymode This it's a thing we are not protected too for now with the library. I need to investigate this ""feature"" from windows because of course it seems there is no direct possibility to check programatically if this is turned on or off

[TobiasWallner]

@TobiasWallner Could you give me some info about your os Windows version etc ?

Microsoft Windows 10 Version 22H2 (OS Build 19045.2965)

[TobiasWallner]

@TobiasWallner Coudl you check if the terminal on your system in not turned into 'legacy mode' ? https://learn.microsoft.com/en-us/windows/console/legacymode This it's a thing we are not protected too for now with the library. I need to investigate this ""feature"" from windows because of course it seems there is no direct possibility to check programatically if this is turned on or off

I just checked, it is not in legacy mode

[TobiasWallner]

I will ckeck later if and how something happens before an event gets generated from cpp-terminal

[TobiasWallner]

just, checked, seems to be fixed by #261

[flagarde]

Let's merge the PR then and close this issue 🥳