Mercurial > hg > release > thermostat-1.4
changeset 1797:7f2a3a806c77
Prevent infinite loops on short input for "setup -c".
Reviewed-by: aazores, neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-September/016049.html
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-September/016171.html
PR2626
author | Severin Gehwolf <sgehwolf@redhat.com> |
---|---|
date | Wed, 09 Sep 2015 10:00:43 +0200 |
parents | 539a7e34b6b3 |
children | e955837e95de |
files | setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/CLISetup.java setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReader.java setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReader.java setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/CLISetupTest.java setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReaderTest.java setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReaderTest.java |
diffstat | 6 files changed, 182 insertions(+), 7 deletions(-) [+] |
line wrap: on
line diff
--- a/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/CLISetup.java Wed Sep 16 10:45:57 2015 -0400 +++ b/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/CLISetup.java Wed Sep 09 10:00:43 2015 +0200 @@ -54,6 +54,7 @@ public class CLISetup { + private static final int EOF = -1; private static final Logger logger = LoggingUtils.getLogger(CLISetup.class); private static final Translate<LocaleResources> t = LocaleResources.createLocalizer(); private final ThermostatSetup thermostatSetup; @@ -165,6 +166,9 @@ int currTry = 0; do { input = readLine(in); + if (input == null) { + throw new IOException("Unexpected EOF while reading answer."); + } if (input.equals(localizedCancelToken)) { return false; } @@ -181,13 +185,17 @@ private String readLine(InputStream in) throws IOException { int c; StringBuilder builder = new StringBuilder(); - while ((c = in.read()) != -1) { + while ((c = in.read()) != EOF) { char token = (char) c; if (token == '\n') { break; } builder.append(token); } + if (c == EOF) { + // reached EOF before we read any other character. + return null; + } return builder.toString(); }
--- a/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReader.java Wed Sep 16 10:45:57 2015 -0400 +++ b/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReader.java Wed Sep 09 10:00:43 2015 +0200 @@ -50,6 +50,7 @@ class PasswordCredentialsReader { + private static final int MAX_TRIES = 100; private static final LocalizedString UNUSED = new LocalizedString("ignored"); private static final Translate<LocaleResources> t = LocaleResources.createLocalizer(); private final Console console; @@ -70,11 +71,19 @@ char[] password; char[] confirmation; boolean isValid = false; + int currTry = 0; do { StorageAuthInfoGetter getter = new StorageAuthInfoGetter(console, UNUSED, passwordPrompt); password = getter.getPassword(null); + if (password == null) { + throw new IOException("Unexpected EOF while reading password."); + } StorageAuthInfoGetter confirmGetter = new StorageAuthInfoGetter(console, UNUSED, confirmPasswordPrompt); confirmation = confirmGetter.getPassword(null); + if (confirmation == null) { + throw new IOException("Unexpected EOF while reading password confirmation."); + } + currTry++; try { verifyPassword(password, confirmation); Arrays.fill(confirmation, '\0'); @@ -87,13 +96,21 @@ printError(LocaleResources.CLI_SETUP_PASSWORD_MISMATCH); clearPasswords(password, confirmation); } - } while (!isValid); + } while (!isValid && currTry < MAX_TRIES); + // If we have reached maximum tries then we might still be invalid + if (!isValid) { + throw new IOException("Tried " + MAX_TRIES + " times and got invalid input each time."); + } return password; } private void clearPasswords(char[] password, char[] confirmation) { - Arrays.fill(password, '\0'); - Arrays.fill(confirmation, '\0'); + if (password != null) { + Arrays.fill(password, '\0'); + } + if (confirmation != null) { + Arrays.fill(confirmation, '\0'); + } password = null; confirmation = null; } @@ -104,7 +121,6 @@ } private void verifyPassword(char[] password, char[] confirmation) throws InvalidPasswordException, PasswordMismatchException { - checkPasswordsMatch(password, confirmation); UserCredsValidator validator = new UserCredsValidator(); try { validator.validatePassword(password); @@ -112,6 +128,8 @@ } catch (IllegalArgumentException e) { throw new InvalidPasswordException(); } + // passwords are now non-null and not empty + checkPasswordsMatch(password, confirmation); } void checkPasswordsMatch(char[] first, char[] second) throws PasswordMismatchException {
--- a/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReader.java Wed Sep 16 10:45:57 2015 -0400 +++ b/setup/command/src/main/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReader.java Wed Sep 09 10:00:43 2015 +0200 @@ -48,6 +48,7 @@ class UsernameCredentialsReader { + private static final int MAX_TRIES = 100; private static final LocalizedString UNUSED = new LocalizedString("ignored"); private static final Translate<LocaleResources> t = LocaleResources.createLocalizer(); private final Console console; @@ -65,8 +66,13 @@ boolean isValid = false; UserCredsValidator validator = new UserCredsValidator(); String username = null; - while (!isValid) { + int currTry = 0; + while (!isValid && currTry < MAX_TRIES) { username = getter.getUserName(null); + if (username == null) { + throw new IOException("Unexpected EOF while reading username."); + } + currTry++; try { validator.validateUsername(username); isValid = true; @@ -75,6 +81,10 @@ // continue loop } } + // If we have reached maximum tries then we might still be invalid + if (!isValid) { + throw new IOException("Tried " + MAX_TRIES + " times and got invalid input each time."); + } return username; }
--- a/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/CLISetupTest.java Wed Sep 16 10:45:57 2015 -0400 +++ b/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/CLISetupTest.java Wed Sep 09 10:00:43 2015 +0200 @@ -136,7 +136,8 @@ cliSetup.run(); fail("Expected setup to cancel"); } catch (CommandException e) { - assertEquals("Setup cancelled on user request.", e.getMessage()); + IOException cause = (IOException)e.getCause(); + assertEquals("Unexpected EOF while reading answer.", cause.getMessage()); } } @@ -216,6 +217,36 @@ assertEquals("Expected no errors", "", new String(berr.toByteArray())); } + /* + * Verifies that if webapp is installed but only mongodb credentials are + * provided, setup fails and does not loop forever. + */ + @Test + public void setupDoesNotLoopOnShortInput() throws CommandException { + String input = "yes\nmongodb-user\nfoo\nfoo\n"; + doShortInputTest(input); + } + + @Test + public void setupDoesNotLoopOnShortInput2() throws CommandException { + String input = "yes\nmongodb-user\nfoo\nfoo\nsomeUser\n"; + doShortInputTest(input); + } + + private void doShortInputTest(String input) { + // simulate webapp being installed + when(thermostatSetup.isWebAppInstalled()).thenReturn(true); + + when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes())); + try { + cliSetup.run(); + fail("Expected exception due to invalid - too short - input"); + } catch (CommandException e) { + IOException cause = (IOException)e.getCause(); + assertTrue(cause.getMessage().startsWith("Unexpected EOF while reading")); + } + } + private CharArrayMatcher matchesPassword(char[] array) { return new CharArrayMatcher(array); }
--- a/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReaderTest.java Wed Sep 16 10:45:57 2015 -0400 +++ b/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/PasswordCredentialsReaderTest.java Wed Sep 09 10:00:43 2015 +0200 @@ -46,6 +46,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -100,4 +101,75 @@ assertEquals("Expected no errors", "", new String(berr.toByteArray())); assertEquals("tell me the password: \nrepeat the password: \n", new String(bout.toByteArray())); } + + @Test + public void canGetPasswordNoEOLAfterConfirmation() throws IOException { + String input = "bar\nbar"; + when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes())); + char[] password = credsReader.readPassword(); + assertArrayEquals(new char[] { 'b', 'a', 'r' }, password); + assertEquals("Expected no errors", "", new String(berr.toByteArray())); + assertEquals("tell me the password: \nrepeat the password: \n", new String(bout.toByteArray())); + } + + /* + * Insufficient input should not loop forever. + */ + @Test + public void testShortReadNoConfirmation() throws IOException { + String input = "bar\n"; // expected password + confirmation + when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes())); + try { + credsReader.readPassword(); + fail("should not reach here"); + } catch (IOException e) { + assertEquals("Unexpected EOF while reading password confirmation.", e.getMessage()); + } + } + + + @Test + public void testShortReadNothing() throws IOException { + String input = ""; // expected password + confirmation + when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes())); + try { + credsReader.readPassword(); + fail("should not reach here"); + } catch (IOException e) { + assertEquals("Unexpected EOF while reading password.", e.getMessage()); + } + } + + @Test + public void testShortReadNoMatch() throws IOException { + String input = "foo\nbar"; // no EOL after bar. next iteration is null password + when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes())); + try { + credsReader.readPassword(); + fail("should not reach here"); + } catch (IOException e) { + assertEquals("Unexpected EOF while reading password.", e.getMessage()); + } + } + + @Test + public void noMatchAfter100TriesBreaksLoop() { + String input = build101NoMatchingPasswords("mypassword"); + when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes())); + try { + credsReader.readPassword(); + fail("should not reach here"); + } catch (IOException e) { + assertEquals("Tried 100 times and got invalid input each time.", e.getMessage()); + } + } + + private String build101NoMatchingPasswords(String password) { + StringBuilder builder = new StringBuilder(); + for (int i = 0; i < 101; i++) { + builder.append(password + "\n"); + builder.append(password + "-no-match\n"); + } + return builder.toString(); + } }
--- a/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReaderTest.java Wed Sep 16 10:45:57 2015 -0400 +++ b/setup/command/src/test/java/com/redhat/thermostat/setup/command/internal/cli/UsernameCredentialsReaderTest.java Wed Sep 09 10:00:43 2015 +0200 @@ -37,6 +37,7 @@ package com.redhat.thermostat.setup.command.internal.cli; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -88,4 +89,39 @@ assertEquals("Chosen username '' invalid!\n", new String(berr.toByteArray())); assertEquals("tell me the username: \ntell me the username: try-second-time\n", new String(bout.toByteArray())); } + + /* + * Insufficient input should not loop forever. + */ + @Test + public void testShortRead() throws IOException { + String input = ""; + when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes())); + try { + credsReader.read(); + fail("should not reach here"); + } catch (IOException e) { + assertEquals("Unexpected EOF while reading username.", e.getMessage()); + } + } + + @Test + public void tooManyInvalidInputsBreaksLoop() throws IOException { + String input = build101EmptyInputs(); + when(console.getInput()).thenReturn(new ByteArrayInputStream(input.getBytes())); + try { + credsReader.read(); + fail("should not reach here"); + } catch (IOException e) { + assertEquals("Tried 100 times and got invalid input each time.", e.getMessage()); + } + } + + private String build101EmptyInputs() { + StringBuilder builder = new StringBuilder(); + for (int i = 0; i < 101; i++) { + builder.append("\n"); + } + return builder.toString(); + } }