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

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

Open
tbannister opened this issue Jan 5, 2023 · 8 comments
Open

Comments

@tbannister
Copy link

tbannister commented Jan 5, 2023

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

Sounds good to me :)

@tbannister
Copy link
Author

#267

@tbannister
Copy link
Author

tbannister commented Jan 5, 2023

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
Copy link
Member

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

@dvlpr91
Copy link

dvlpr91 commented Feb 8, 2023

In version 4.3 this problem still exists.

@AydinHassan
Copy link
Member

It's fixed in master, just not released

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

No branches or pull requests

3 participants