From 45aec91a6acf18aba7c03d213bfd85fddab5527e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Thu, 9 Mar 2017 10:36:32 +0100 Subject: [PATCH 1/2] Never prompt for password if it was passed as an argument (fixes #195) Took the liberty of also fixing one pre-existing test related to passwords that was marked as disabled until now. --- .../java/com/beust/jcommander/JCommander.java | 77 ++++++++++--- .../com/beust/jcommander/JCommanderTest.java | 108 ++++++++++++++---- 2 files changed, 143 insertions(+), 42 deletions(-) diff --git a/src/main/java/com/beust/jcommander/JCommander.java b/src/main/java/com/beust/jcommander/JCommander.java index ac28cca8..7d05d108 100644 --- a/src/main/java/com/beust/jcommander/JCommander.java +++ b/src/main/java/com/beust/jcommander/JCommander.java @@ -18,20 +18,42 @@ package com.beust.jcommander; -import com.beust.jcommander.FuzzyMap.IKey; -import com.beust.jcommander.converters.*; -import com.beust.jcommander.internal.*; - import java.io.BufferedReader; import java.io.IOException; -import java.lang.reflect.*; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Paths; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.EnumSet; +import java.util.Iterator; +import java.util.List; +import java.util.Locale; +import java.util.Map; import java.util.ResourceBundle; import java.util.concurrent.CopyOnWriteArrayList; +import com.beust.jcommander.FuzzyMap.IKey; +import com.beust.jcommander.converters.DefaultListConverter; +import com.beust.jcommander.converters.EnumConverter; +import com.beust.jcommander.converters.IParameterSplitter; +import com.beust.jcommander.converters.NoConverter; +import com.beust.jcommander.converters.StringConverter; +import com.beust.jcommander.internal.Console; +import com.beust.jcommander.internal.DefaultConsole; +import com.beust.jcommander.internal.DefaultConverterFactory; +import com.beust.jcommander.internal.JDK6Console; +import com.beust.jcommander.internal.Lists; +import com.beust.jcommander.internal.Maps; +import com.beust.jcommander.internal.Nullable; + /** * The main class for JCommander. It's responsible for parsing the object that contains * all the annotated fields, parse the command line and assign the fields with the correct @@ -642,12 +664,7 @@ private void parseValues(String[] args, boolean validate) { if (pd != null) { if (pd.getParameter().password()) { - // - // Password option, use the Console to retrieve the password - // - char[] password = readPassword(pd.getDescription(), pd.getParameter().echoInput()); - pd.addValue(new String(password)); - requiredFields.remove(pd.getParameterized()); + increment = processPassword(args, i, pd, validate); } else { if (pd.getParameter().variableArity()) { // @@ -760,6 +777,34 @@ public int processVariableArity(String optionName, String[] options) { private final IVariableArity DEFAULT_VARIABLE_ARITY = new DefaultVariableArity(); + private final int determineArity(String[] args, int index, ParameterDescription pd, IVariableArity va) { + List currentArgs = Lists.newArrayList(); + for (int j = index + 1; j < args.length; j++) { + currentArgs.add(args[j]); + } + return va.processVariableArity(pd.getParameter().names()[0], + currentArgs.toArray(new String[0])); + } + + /** + * @return the number of options that were processed. + */ + private int processPassword(String[] args, int index, ParameterDescription pd, boolean validate) { + final int passwordArity = determineArity(args, index, pd, DEFAULT_VARIABLE_ARITY); + if (passwordArity == 0) { + // password option with password not specified, use the Console to retrieve the password + char[] password = readPassword(pd.getDescription(), pd.getParameter().echoInput()); + pd.addValue(new String(password)); + requiredFields.remove(pd.getParameterized()); + return 1; + } else if (passwordArity == 1) { + // password option with password specified + return processFixedArity(args, index, pd, validate, List.class, 1); + } else { + throw new ParameterException("Password parameter must have at most 1 argument."); + } + } + /** * @return the number of options that were processed. */ @@ -772,13 +817,7 @@ private int processVariableArity(String[] args, int index, ParameterDescription va = (IVariableArity) arg; } - List currentArgs = Lists.newArrayList(); - for (int j = index + 1; j < args.length; j++) { - currentArgs.add(args[j]); - } - int arity = va.processVariableArity(pd.getParameter().names()[0], - currentArgs.toArray(new String[0])); - + int arity = determineArity(args, index, pd, va); int result = processFixedArity(args, index, pd, validate, List.class, arity); return result; } diff --git a/src/test/java/com/beust/jcommander/JCommanderTest.java b/src/test/java/com/beust/jcommander/JCommanderTest.java index 4c3feab8..c1befe15 100644 --- a/src/test/java/com/beust/jcommander/JCommanderTest.java +++ b/src/test/java/com/beust/jcommander/JCommanderTest.java @@ -18,8 +18,64 @@ package com.beust.jcommander; -import com.beust.jcommander.args.*; +import java.io.ByteArrayInputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.FileWriter; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStreamWriter; +import java.math.BigDecimal; +import java.nio.charset.Charset; +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.EnumSet; +import java.util.Iterator; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.ResourceBundle; +import java.util.TreeSet; + +import com.beust.jcommander.args.AlternateNamesForListArgs; +import com.beust.jcommander.args.Args1; +import com.beust.jcommander.args.Args1Setter; +import com.beust.jcommander.args.Args2; +import com.beust.jcommander.args.ArgsArityString; +import com.beust.jcommander.args.ArgsBooleanArity; +import com.beust.jcommander.args.ArgsBooleanArity0; +import com.beust.jcommander.args.ArgsConverter; +import com.beust.jcommander.args.ArgsEnum; import com.beust.jcommander.args.ArgsEnum.ChoiceType; +import com.beust.jcommander.args.ArgsEquals; +import com.beust.jcommander.args.ArgsHelp; +import com.beust.jcommander.args.ArgsI18N1; +import com.beust.jcommander.args.ArgsI18N2; +import com.beust.jcommander.args.ArgsI18N2New; +import com.beust.jcommander.args.ArgsInherited; +import com.beust.jcommander.args.ArgsList; +import com.beust.jcommander.args.ArgsLongCommandDescription; +import com.beust.jcommander.args.ArgsLongDescription; +import com.beust.jcommander.args.ArgsLongMainParameterDescription; +import com.beust.jcommander.args.ArgsMainParameter1; +import com.beust.jcommander.args.ArgsMaster; +import com.beust.jcommander.args.ArgsMultipleUnparsed; +import com.beust.jcommander.args.ArgsOutOfMemory; +import com.beust.jcommander.args.ArgsPrivate; +import com.beust.jcommander.args.ArgsRequired; +import com.beust.jcommander.args.ArgsSlave; +import com.beust.jcommander.args.ArgsSlaveBogus; +import com.beust.jcommander.args.ArgsValidate1; +import com.beust.jcommander.args.ArgsWithSet; +import com.beust.jcommander.args.Arity1; +import com.beust.jcommander.args.HiddenArgs; +import com.beust.jcommander.args.SeparatorColon; +import com.beust.jcommander.args.SeparatorEqual; +import com.beust.jcommander.args.SeparatorMixed; +import com.beust.jcommander.args.SlashSeparator; +import com.beust.jcommander.args.VariableArity; import com.beust.jcommander.command.CommandAdd; import com.beust.jcommander.command.CommandCommit; import com.beust.jcommander.command.CommandMain; @@ -29,14 +85,6 @@ import org.testng.annotations.DataProvider; import org.testng.annotations.Test; -import java.io.*; -import java.math.BigDecimal; -import java.nio.charset.Charset; -import java.text.ParseException; -import java.text.SimpleDateFormat; -import java.util.*; -import java.util.ResourceBundle; - @Test public class JCommanderTest { @@ -818,21 +866,35 @@ public void commandsWithSamePrefixAsOptionsShouldWork() { jc.parse("--configure"); } - // Tests: - // required unparsed parameter - @Test(enabled = false, - description = "For some reason, this test still asks the password on stdin") - public void askedRequiredPassword() { - class A { - @Parameter(names = {"--password", "-p"}, description = "Private key password", - password = true, required = true) - public String password; + public static class PasswordTestingArgs { + @Parameter(names = {"--password", "-p"}, description = "Private key password", + password = true, required = true) + public String password; - @Parameter(names = {"--port", "-o"}, description = "Port to bind server to", - required = true) - public int port; - } - A a = new A(); + @Parameter(names = {"--port", "-o"}, description = "Port to bind server to", + required = true) + public int port; + } + + @Test + public void passwordNotRequiredToAsk() { + PasswordTestingArgs a = new PasswordTestingArgs(); + final String expectedPassword = "somepassword"; + final int expectedPort = 7; + new JCommander(a, "--password", expectedPassword, "--port", String.valueOf(7)); + Assert.assertEquals(a.port, expectedPort); + Assert.assertEquals(a.password, expectedPassword); + } + + @Test(expectedExceptions = ParameterException.class) + public void passwordWithExcessiveArity() { + PasswordTestingArgs a = new PasswordTestingArgs(); + new JCommander(a, "--password", "somepassword", "someotherarg", "--port", String.valueOf(7)); + } + + @Test + public void passwordRequiredAsked() { + PasswordTestingArgs a = new PasswordTestingArgs(); InputStream stdin = System.in; try { System.setIn(new ByteArrayInputStream("password".getBytes())); From b446705367f9427d2f424d438c995532b52a9043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Petrovick=C3=BD?= Date: Thu, 9 Mar 2017 11:08:02 +0100 Subject: [PATCH 2/2] Cover all possible password cases --- .../com/beust/jcommander/JCommanderTest.java | 40 ------- .../com/beust/jcommander/PasswordTest.java | 111 ++++++++++++++++++ 2 files changed, 111 insertions(+), 40 deletions(-) create mode 100644 src/test/java/com/beust/jcommander/PasswordTest.java diff --git a/src/test/java/com/beust/jcommander/JCommanderTest.java b/src/test/java/com/beust/jcommander/JCommanderTest.java index c1befe15..65c8e8c4 100644 --- a/src/test/java/com/beust/jcommander/JCommanderTest.java +++ b/src/test/java/com/beust/jcommander/JCommanderTest.java @@ -866,46 +866,6 @@ public void commandsWithSamePrefixAsOptionsShouldWork() { jc.parse("--configure"); } - public static class PasswordTestingArgs { - @Parameter(names = {"--password", "-p"}, description = "Private key password", - password = true, required = true) - public String password; - - @Parameter(names = {"--port", "-o"}, description = "Port to bind server to", - required = true) - public int port; - } - - @Test - public void passwordNotRequiredToAsk() { - PasswordTestingArgs a = new PasswordTestingArgs(); - final String expectedPassword = "somepassword"; - final int expectedPort = 7; - new JCommander(a, "--password", expectedPassword, "--port", String.valueOf(7)); - Assert.assertEquals(a.port, expectedPort); - Assert.assertEquals(a.password, expectedPassword); - } - - @Test(expectedExceptions = ParameterException.class) - public void passwordWithExcessiveArity() { - PasswordTestingArgs a = new PasswordTestingArgs(); - new JCommander(a, "--password", "somepassword", "someotherarg", "--port", String.valueOf(7)); - } - - @Test - public void passwordRequiredAsked() { - PasswordTestingArgs a = new PasswordTestingArgs(); - InputStream stdin = System.in; - try { - System.setIn(new ByteArrayInputStream("password".getBytes())); - new JCommander(a, "--port", "7", "--password"); - Assert.assertEquals(a.port, 7); - Assert.assertEquals(a.password, "password"); - } finally { - System.setIn(stdin); - } - } - public void dynamicParameters() { class Command { @DynamicParameter(names = {"-P"}, description = "Additional command parameters") diff --git a/src/test/java/com/beust/jcommander/PasswordTest.java b/src/test/java/com/beust/jcommander/PasswordTest.java new file mode 100644 index 00000000..ff1792ed --- /dev/null +++ b/src/test/java/com/beust/jcommander/PasswordTest.java @@ -0,0 +1,111 @@ +package com.beust.jcommander; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; + +import org.testng.Assert; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class PasswordTest { + + @DataProvider(name = "args") + public Object[][] createArgs() { + return new Object[][] { + { new PasswordTestingArgs() }, + { new OptionalPasswordTestingArgs() }, + }; + } + + public interface Args { + + String getPassword(); + + int getPort(); + + } + + public static class PasswordTestingArgs implements PasswordTest.Args { + @Parameter(names = {"--password", "-p"}, description = "Private key password", + password = true, required = true) + public String password; + + @Parameter(names = {"--port", "-o"}, description = "Port to bind server to", + required = true) + public int port; + + @Override + public String getPassword() { + return password; + } + + @Override + public int getPort() { + return port; + } + } + + @Test(dataProvider = "args") + public void passwordNotAsked(Args a) { + String expectedPassword = "somepassword"; + int expectedPort = 7; + new JCommander(a, "--password", expectedPassword, "--port", String.valueOf(7)); + Assert.assertEquals(a.getPort(), expectedPort); + Assert.assertEquals(a.getPassword(), expectedPassword); + } + + @Test(dataProvider = "args", expectedExceptions = ParameterException.class) + public void passwordWithExcessiveArity(Args a) { + new JCommander(a, "--password", "somepassword", "someotherarg", "--port", String.valueOf(7)); + } + + @Test(dataProvider = "args") + public void passwordAsked(Args a) { + InputStream stdin = System.in; + String password = "password"; + int port = 7; + try { + System.setIn(new ByteArrayInputStream(password.getBytes())); + new JCommander(a, "--port", String.valueOf(port), "--password"); + Assert.assertEquals(a.getPort(), port); + Assert.assertEquals(a.getPassword(), password); + } finally { + System.setIn(stdin); + } + } + + public static class OptionalPasswordTestingArgs implements PasswordTest.Args { + @Parameter(names = {"--password", "-p"}, description = "Private key password", + password = true) + public String password; + + @Parameter(names = {"--port", "-o"}, description = "Port to bind server to", + required = true) + public int port; + + @Override + public String getPassword() { + return password; + } + + @Override + public int getPort() { + return port; + } + } + + @Test + public void passwordOptionalNotProvided() { + Args a = new OptionalPasswordTestingArgs(); + new JCommander(a, "--port", "7"); + Assert.assertEquals(a.getPort(), 7); + Assert.assertEquals(a.getPassword(), null); + } + + @Test(expectedExceptions = ParameterException.class) + public void passwordRequredNotProvided() { + Args a = new PasswordTestingArgs(); + new JCommander(a, "--port", "7"); + } + +}