php-school / cli-menu

🖥 Build beautiful PHP CLI menus. Simple yet Powerful. Expressive DSL.
http://www.phpschool.io
MIT License
1.94k stars 107 forks source link

Pressing backspace in an empty text input crashes the menu system #266

Open tbannister opened 1 year ago

tbannister commented 1 year ago

To duplicate: Open a new text input using v4.3.0 of cli-menu and press the backspace key until there the placeholder text has been deleted, then press it once more. Expected result: Nothing happens. Actual result: Fatal error: Uncaught TypeError: Argument 2 passed to PhpSchool\CliMenu\Input\InputIO::drawInput() must be of the type string, bool given, called in /.../vendor/php-school/cli-menu/src/Input/InputIO.php on line 80 and defined in /.../vendor/php-school/cli-menu/src/Input/InputIO.php on line 205 Also, after the crash the terminal window stops echoing input. Environment: CentOS 7.9, running PHP 7.4.20

Sample code: '$result = $menu->askText() ->setPromptText($prompt.' :') ->ask();'

I can replicate this behavior with the text input sample code in the examples directory.

AydinHassan commented 1 year ago

Certainly sounds like a bug, feel free to send a PR with a fix, otherwise I'm not quite sure when we'll get around to fixing. Regarding the echoing not working, we could maybe catch any errors or register a shutdown function to do similar to what \PhpSchool\CliMenu\CliMenu::tearDownTerminal does. What do you think?

tbannister commented 1 year ago

I've been looking into the root cause, and I've found it, before PHP 8, substr returns false in certain circumstances. The type of the return value from the substr call on line 78 of InputIO.php isn't checked. A relatively simple solution should work, something like: 'if (!is_string($inputValue)) { $inputValue = '' }'

And, yes it would probably be a good idea to have a shutdown function that restores the terminal.

AydinHassan commented 1 year ago

Sounds good to me :)

tbannister commented 1 year ago

267

tbannister commented 1 year ago

Hmm. Actually, this is already non-issue on the dev-master branch, rather than the most recently published version. It's already been fixed there.

AydinHassan commented 1 year ago

Ok I've reverted it. I'll prepare a new release next week with the existing fix.

dvlpr91 commented 1 year ago

In version 4.3 this problem still exists.

AydinHassan commented 1 year ago

It's fixed in master, just not released