From 48548f6c3d92ad3668ba21567e37e52e6eec9d5a Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 24 Feb 2017 20:20:17 -0800 Subject: [PATCH] CLI: Fix prompting for yes/no to handle console returning null (#23320) Console.readText may return null in certain cases. This commit fixes a bug in Terminal.promptYesNo which assumed a non-null return value. It also adds a test for this, and modifies mock terminal to be able to handle null input values. --- .../java/org/elasticsearch/cli/Terminal.java | 5 ++-- .../org/elasticsearch/cli/TerminalTests.java | 2 ++ .../org/elasticsearch/cli/MockTerminal.java | 28 +++++++++++++------ 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cli/Terminal.java b/core/src/main/java/org/elasticsearch/cli/Terminal.java index cd7fc76e681..d42e3475dc4 100644 --- a/core/src/main/java/org/elasticsearch/cli/Terminal.java +++ b/core/src/main/java/org/elasticsearch/cli/Terminal.java @@ -100,10 +100,11 @@ public abstract class Terminal { public final boolean promptYesNo(String prompt, boolean defaultYes) { String answerPrompt = defaultYes ? " [Y/n]" : " [y/N]"; while (true) { - String answer = readText(prompt + answerPrompt).toLowerCase(Locale.ROOT); - if (answer.isEmpty()) { + String answer = readText(prompt + answerPrompt); + if (answer == null || answer.isEmpty()) { return defaultYes; } + answer = answer.toLowerCase(Locale.ROOT); boolean answerYes = answer.equals("y"); if (answerYes == false && answer.equals("n") == false) { println("Did not understand answer '" + answer + "'"); diff --git a/core/src/test/java/org/elasticsearch/cli/TerminalTests.java b/core/src/test/java/org/elasticsearch/cli/TerminalTests.java index 795780b4890..3b409c2add6 100644 --- a/core/src/test/java/org/elasticsearch/cli/TerminalTests.java +++ b/core/src/test/java/org/elasticsearch/cli/TerminalTests.java @@ -52,6 +52,8 @@ public class TerminalTests extends ESTestCase { assertTrue(terminal.promptYesNo("Answer?", true)); terminal.addTextInput(""); assertFalse(terminal.promptYesNo("Answer?", false)); + terminal.addTextInput(null); + assertFalse(terminal.promptYesNo("Answer?", false)); } public void testPromptYesNoReprompt() throws Exception { diff --git a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java index bd8bd493cea..a547e4033dd 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/MockTerminal.java @@ -25,7 +25,9 @@ import java.io.PrintWriter; import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.Deque; +import java.util.List; /** * A terminal for tests which captures all output, and @@ -35,8 +37,18 @@ public class MockTerminal extends Terminal { private final ByteArrayOutputStream buffer = new ByteArrayOutputStream(); private final PrintWriter writer = new PrintWriter(new OutputStreamWriter(buffer, StandardCharsets.UTF_8)); - private final Deque textInput = new ArrayDeque<>(); - private final Deque secretInput = new ArrayDeque<>(); + + // A deque would be a perfect data structure for the FIFO queue of input values needed here. However, + // to support the valid return value of readText being null (defined by Console), we need to be able + // to store nulls. However, java the java Deque api does not allow nulls because it uses null as + // a special return value from certain methods like peek(). So instead of deque, we use an array list here, + // and keep track of the last position which was read. It means that we will hold onto all input + // setup for the mock terminal during its lifetime, but this is normally a very small amount of data + // so in reality it will not matter. + private final List textInput = new ArrayList<>(); + private int textIndex = 0; + private final List secretInput = new ArrayList<>(); + private int secretIndex = 0; public MockTerminal() { super("\n"); // always *nix newlines for tests @@ -44,18 +56,18 @@ public class MockTerminal extends Terminal { @Override public String readText(String prompt) { - if (textInput.isEmpty()) { + if (textIndex >= textInput.size()) { throw new IllegalStateException("No text input configured for prompt [" + prompt + "]"); } - return textInput.removeFirst(); + return textInput.get(textIndex++); } @Override public char[] readSecret(String prompt) { - if (secretInput.isEmpty()) { + if (secretIndex >= secretInput.size()) { throw new IllegalStateException("No secret input configured for prompt [" + prompt + "]"); } - return secretInput.removeFirst().toCharArray(); + return secretInput.get(secretIndex++).toCharArray(); } @Override @@ -65,12 +77,12 @@ public class MockTerminal extends Terminal { /** Adds an an input that will be return from {@link #readText(String)}. Values are read in FIFO order. */ public void addTextInput(String input) { - textInput.addLast(input); + textInput.add(input); } /** Adds an an input that will be return from {@link #readText(String)}. Values are read in FIFO order. */ public void addSecretInput(String input) { - secretInput.addLast(input); + secretInput.add(input); } /** Returns all output written to this terminal. */