Skip to content

Add useful message when no input from terminal #29369

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

Merged
merged 2 commits into from
Apr 11, 2018

Conversation

jasontedor
Copy link
Member

Today when a user runs a CLI tool with standard input closed and no tty attached, the result from reading is null and this usually leads to a null pointer exception when we try to parse this input. This arises for example when the user runs the plugin installer through a Docker container without leaving standard input open and attaching a tty (docker exec bin/elasticsearch-plugin install). When we try to read that the user accepts the plugin requiring additional security permissions we will get back null. This commit addresses this for all cases by throwing an illegal state exception. The solution for the user is leave standard input open and attach a tty (or, for some tools, use batch mode).

Relates #29359, relates #29365

Today when a user runs a CLI tool with standard input closed and no tty
attached, the result from reading is null and this usually leads to a
null pointer exception when we try to parse this input. This arises for
example when the user runs the plugin installer through a Docker
container without leaving standard input open and attaching a tty
(docker exec <container ID> bin/elasticsearch-plugin install). When we
try to read that the user accepts the plugin requiring additional
security permissions we will get back null. This commit addresses this
for all cases by throwing an illegal state exception. The solution for
the user is leave standard input open and attach a tty (or, for some
tools, use batch mode).
@jasontedor jasontedor added review :Core/Infra/Core Core issues without another label v7.0.0 v6.3.0 labels Apr 4, 2018
@jasontedor
Copy link
Member Author

Now, a user would see:

08:54:53 3d [jason@totoro:~/src/elastic/elasticsearch-6.x] c59ff00e22(+6/-2)+ ± sudo docker exec 9acae177f0f7 bin/elasticsearch-plugin install ingest-geoip
-> Downloading ingest-geoip from elastic
[=================================================] 100%?? 
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@     WARNING: plugin requires additional permissions     @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
* java.lang.RuntimePermission accessDeclaredMembers
* java.lang.reflect.ReflectPermission suppressAccessChecks
See http://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html
for descriptions of what these permissions allow and the associated risks.

Exception in thread "main" java.lang.IllegalStateException: standard input is closed or there is no tty attached
	at org.elasticsearch.cli.Terminal$SystemTerminal.readText(Terminal.java:168)
	at org.elasticsearch.plugins.PluginSecurity.prompt(PluginSecurity.java:86)
	at org.elasticsearch.plugins.PluginSecurity.confirmPolicyExceptions(PluginSecurity.java:70)
	at org.elasticsearch.plugins.InstallPluginCommand.installPlugin(InstallPluginCommand.java:712)
	at org.elasticsearch.plugins.InstallPluginCommand.install(InstallPluginCommand.java:623)
	at org.elasticsearch.plugins.InstallPluginCommand.execute(InstallPluginCommand.java:223)
	at org.elasticsearch.plugins.InstallPluginCommand.execute(InstallPluginCommand.java:212)
	at org.elasticsearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:86)
	at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:124)
	at org.elasticsearch.cli.MultiCommand.execute(MultiCommand.java:75)
	at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:124)
	at org.elasticsearch.cli.Command.main(Command.java:90)
	at org.elasticsearch.plugins.PluginCli.main(PluginCli.java:48)
08:55:10 3d [jason@totoro:~/src/elastic/elasticsearch-6.x] c59ff00e22(+6/-2)+ 1 ±

@jasontedor jasontedor requested a review from rjernst April 5, 2018 01:02
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jasontedor jasontedor merged commit 663a52a into elastic:master Apr 11, 2018
@jasontedor jasontedor deleted the closed-standard-input-npe branch April 11, 2018 01:51
jasontedor added a commit that referenced this pull request Apr 11, 2018
Today when a user runs a CLI tool with standard input closed and no tty
attached, the result from reading is null and this usually leads to a
null pointer exception when we try to parse this input. This arises for
example when the user runs the plugin installer through a Docker
container without leaving standard input open and attaching a tty
(docker exec <container ID> bin/elasticsearch-plugin install). When we
try to read that the user accepts the plugin requiring additional
security permissions we will get back null. This commit addresses this
for all cases by throwing an illegal state exception. The solution for
the user is leave standard input open and attach a tty (or, for some
tools, use batch mode).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants