kohsuke / args4j

args4j
http://args4j.kohsuke.org/
MIT License
789 stars 189 forks source link

args4j prints actual values as default values (might contain sensitive information) #153

Open patric-r opened 7 years ago

patric-r commented 7 years ago

Imagine an argument bean which contains sensitive information (e.g. passwords): -password actualProvidedPassword If we call org.kohsuke.args4j.CmdLineParser.printUsage(OutputStream) after we called parseArgument() (because some error occurred, e.g. we provided an non existing argument -file), args4j outputs the actual provided values as "default" values:

 "-file" is not a valid option
 -password password                 : database password (default:
                                          actualProvidedPassword)

There are two issues with that behavior:

Is there a way to prevent this behavior (beside disabling default value output at all)?

olivergondza commented 7 years ago

I have just hit this bug as well. It seems that arg4j confuses configured defaults and injected values. Which, of course, only manifests when help is printed after failed parsing attempt.

devconsole commented 7 years ago

Ugly workaround: Write usage to a StringWriter before calling parseArgument().

CmdLineParser cmdLineParser = new CmdLineParser(this);

StringWriter usage = new StringWriter();
cmdLineParser.printUsage(usage, null);

try {
    cmdLineParser.parseArgument(args);
}
catch (CmdLineException e) {
    System.err.println(e.getMessage());
    System.err.println("java Program [options...] arguments...");
    System.err.println(usage);
    System.err.println();
    return;
}
devconsole commented 7 years ago

Proposed solution: save default values in OptionHandler constructor and use that saved value in OptionHandler.printDefaultValue().

private final List<T> defaultValues;

protected OptionHandler(CmdLineParser parser, OptionDef option, Setter<? super T> setter) {
    this.owner = parser;
    this.option = option;
    this.setter = setter;

    this.defaultValues = setter instanceof Getter 
            ? ((Getter<T>) setter).getValueList() : null;
}

// ...

public String printDefaultValue() {
    if (defaultValues == null || defaultValues.isEmpty())
        return null;

    StringBuilder buf = new StringBuilder();
    if (defaultValues.size() > 1) {
        for (T v : defaultValues) {
            buf.append(buf.length() == 0 ? '[' : ',');
            buf.append(print(v));
        }
        buf.append(']');
    }
    else {
        buf.append(print(defaultValues.get(0)));
    }

    return buf.toString();
}
radai-rosenblatt commented 6 years ago

i have a slightly less ugly (i hope) workaround:

    private static Arguments parse(String[] cmdLine) {
        Arguments result = new Arguments();
        CmdLineParser parser = new CmdLineParser(result);
        try {
            parser.parseArgument(cmdLine);
        } catch (CmdLineException e) {
            System.err.println(e.getMessage());
            printUsage(System.err);
            System.exit(1);
        }
        return result;
    }

    private static void printUsage(PrintStream to) {
        CmdLineParser freshParser = new CmdLineParser(new Arguments());
        freshParser.printUsage(to);
    }

but longer term, i like @devconsole 's proposal