daniele77 / cli

A library for interactive command line interfaces in modern C++
Boost Software License 1.0
1.24k stars 140 forks source link

bugfix: revert 0a4d8608 signed char on data reception #219

Closed jtavares closed 10 months ago

jtavares commented 10 months ago

Commit 0a4d8608 caused received characters to be forcibly converted from unsigned -> signed on some platforms, and so incoming byte values >127 (such as telnet escape sequences) were getting converted to negative values. The problem is that the enum containing the escape sequence constants has wider underlying type (int) that can store the large unsigned value (such as, 0xFF). This caused telnet escape sequence comparisons to fail (such as: -128 != 255), which means that telnet escape sequence processing / filtering was not being performed.

The end result was that telnet connections close immediately upon connection (for busybox telnet client, at least).

This change standardizes on int as the reception/transmission char type. This allows for in-band bytes to take the values 0-255, and -1 to be used for EOF (out-of-band).

jtavares commented 10 months ago

...whoops, this fixed my problem but causes build failures on x86-64. Will re-work in a more generic way.

jtavares commented 10 months ago

@daniele77 This PR is ready for your re-consideration

daniele77 commented 10 months ago

Hi @jtavares, thank you very much for your contribution. I've tested your PR and unfortunately, on my architecture (Linux on x86 machine) the examples crashed at every telnet connection. I was wondering whether changing the method TelnetSession::OnDataReceived in this way could solve the issue on the architecture you're working on:

void OnDataReceived(const std::string& _data) override
{
    for (int c: _data)
        Consume(c);
}

Please, let me know.

daniele77 commented 10 months ago

I hope I have fixed the problem with the commit: 2921758 Please, check if this solves your problem and let me know. Thanks.

jtavares commented 10 months ago

I tested commit 2921758 and it appears to solve my disconnect problem. Thank you. I apologize I was not able to get back to my branch in time to fix it up. I will close the PR.