Skip to content

Commit af10eaf

Browse files
remkopremkop
committed
[#2087] Mask parameters in trace log when echo=false for interactive options
1 parent 197a502 commit af10eaf

File tree

3 files changed

+18
-16
lines changed

3 files changed

+18
-16
lines changed

‎RELEASE-NOTES.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@ Artifacts in this release are signed by Remko Popma (6601 E5C0 8DCC BB96).
2222

2323

2424
## <a name="4.7.5-fixes"></a> Fixed issues
25+
* [#2083][#2084] Enhancement: Java 22 update: improve logic for detecting if the output stream is connected to a terminal. Thanks to [Liam Miller-Cushon](https://github.com/cushon) for the pull request.
26+
* [#2087] Enhancement: Mask parameters in trace log when `echo=false` for `interactive` options and positional parameters. Thanks to [szzsolt](https://github.com/szzsolt) for raising this.
2527
* [#2060] Bugfix: Fix positional parameters bug with late-resolved arity variable. Thanks to [daisukeoto](https://github.com/daisukeoto) for raising this.
2628
* [#2074][#2075] Bugfix: Don't generate auto-complete for hidden attributes in `picocli.shell.jline3.PicoCommand`. Thanks to [clebertsuconic](https://github.com/clebertsuconic) for the pull request.
27-
* [#2083][#2084] Enhancement: Java 22 update: improve logic for detecting if the output stream is connected to a terminal. Thanks to [Liam Miller-Cushon](https://github.com/cushon) for the pull request.
2829
* [#2059] Bugfix: ArgGroup with `exclusive=false` and `multiplicity=1` should require at least one option; fix regression and refine solution introduced in [#1848][#2030]. Thanks to [Utkarsh Mittal](https://github.com/utmittal) for raising this.
2930
* [#2080] DOC: Improve GraalVM documentation: add `graalvm-native-image-plugin`. Thanks to [Bhavik Patel](https://github.com/bhavikp19) for the pull request.
3031
* [#2045] DOC: Commit html files with LF line-endings. Thanks to [Fridrich Strba](https://github.com/fridrich) for the pull request.

‎src/main/java/picocli/CommandLine.java

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13346,6 +13346,8 @@ String toString(String separator) {
1334613346
* Helper class responsible for processing command line arguments.
1334713347
*/
1334813348
private class Interpreter {
13349+
/** Value displayed in trace logs for options with echo=false. */
13350+
private static final String MASKED_VALUE = "*****(masked)"; // see #2087
1334913351
private final Map<Class<?>, ITypeConverter<?>> converterRegistry = new HashMap<Class<?>, ITypeConverter<?>>();
1335013352
private boolean isHelpRequested;
1335113353
private int position;
@@ -13707,10 +13709,11 @@ private boolean applyDefault(IDefaultValueProvider defaultValueProvider, ArgSpec
1370713709
String defaultValue = fromProvider == null ? arg.defaultValue() : arg.interpolate(fromProvider);
1370813710
String provider = defaultValueProvider == null ? "" : (" from " + defaultValueProvider.toString());
1370913711

13712+
String displayDefaultVal = arg.echo() ? defaultValue : MASKED_VALUE;
1371013713
Tracer tracer = CommandLine.tracer();
1371113714
if (defaultValue != null && !ArgSpec.NULL_VALUE.equals(defaultValue)) {
1371213715
if (tracer.isDebug()) {
13713-
tracer.debug("Applying defaultValue (%s)%s to %s on %s", defaultValue, provider, arg, arg.scopeString());}
13716+
tracer.debug("Applying defaultValue (%s)%s to %s on %s", displayDefaultVal, provider, arg, arg.scopeString());}
1371413717
Range arity = arg.arity().min(Math.max(1, arg.arity().min));
1371513718
applyOption(arg, false, LookBehind.SEPARATE, false, arity, stack(defaultValue), new HashSet<ArgSpec>(), arg.toString);
1371613719
arg.valueIsDefaultValue = true;
@@ -13727,7 +13730,7 @@ private boolean applyDefault(IDefaultValueProvider defaultValueProvider, ArgSpec
1372713730
if (ArgSpec.NULL_VALUE.equals(arg.originalDefaultValue)) {
1372813731
defaultValue = null;
1372913732
if (tracer.isDebug()) {
13730-
tracer.debug("Applying defaultValue (%s)%s to %s on %s", defaultValue, provider, arg, arg.scopeString());}
13733+
tracer.debug("Applying defaultValue (%s)%s to %s on %s", displayDefaultVal, provider, arg, arg.scopeString());}
1373113734
arg.setValue(defaultValue);
1373213735
arg.valueIsDefaultValue = true;
1373313736
return true;
@@ -14275,13 +14278,8 @@ private int applyValueToSingleValuedField(ArgSpec argSpec,
1427514278
} else {
1427614279
consumed = 1;
1427714280
if (interactiveValue != null) {
14278-
if (argSpec.echo()) {
14279-
initValueMessage = "Setting %s to %3$s (interactive value) for %4$s on %5$s";
14280-
overwriteValueMessage = "Overwriting %s value with %3$s (interactive value) for %s on %5$s";
14281-
} else {
14282-
initValueMessage = "Setting %s to *** (masked interactive value) for %4$s on %5$s";
14283-
overwriteValueMessage = "Overwriting %s value with *** (masked interactive value) for %s on %5$s";
14284-
}
14281+
initValueMessage = "Setting %s to %3$s (interactive value) for %4$s on %5$s";
14282+
overwriteValueMessage = "Overwriting %s value with %3$s (interactive value) for %s on %5$s";
1428514283
}
1428614284
// #1642 Negatable options should negate explicit values
1428714285
if ((cls == Boolean.class || cls == Boolean.TYPE) && arity.min >= 1) {
@@ -14321,7 +14319,8 @@ private int applyValueToSingleValuedField(ArgSpec argSpec,
1432114319
if (argSpec.typeInfo().isOptional()) {
1432214320
newValue = getOptionalOfNullable(newValue);
1432314321
}
14324-
if (tracer.isInfo()) { tracer.info(traceMessage, argSpec.toString(), String.valueOf(oldValue), String.valueOf(newValue), argDescription, argSpec.scopeString()); }
14322+
String displayVal = !argSpec.interactive() || argSpec.echo() ? String.valueOf(newValue) : MASKED_VALUE;
14323+
if (tracer.isInfo()) { tracer.info(traceMessage, argSpec.toString(), String.valueOf(oldValue), displayVal, argDescription, argSpec.scopeString()); }
1432514324
int pos = getPosition(argSpec);
1432614325
argSpec.setValue(newValue);
1432714326
parseResultBuilder.addOriginalStringValue(argSpec, actualValue);// #279 track empty string value if no command line argument was consumed
@@ -14429,7 +14428,8 @@ private void consumeOneMapArgument(ArgSpec argSpec,
1442914428
String rawMapValue = keyValue.length == 1 ? argSpec.mapFallbackValue() : keyValue[1];
1443014429
Object mapValue = tryConvert(argSpec, index, valueConverter, rawMapValue, 1);
1443114430
result.put(mapKey, mapValue);
14432-
if (tracer.isInfo()) { tracer.info("Putting [%s : %s] in %s<%s, %s> %s for %s on %s", String.valueOf(mapKey), String.valueOf(mapValue),
14431+
String displayVal = !argSpec.interactive() || argSpec.echo() ? String.valueOf(mapValue) : MASKED_VALUE;
14432+
if (tracer.isInfo()) { tracer.info("Putting [%s : %s] in %s<%s, %s> %s for %s on %s", String.valueOf(mapKey), displayVal,
1443314433
result.getClass().getSimpleName(), classes[0].getSimpleName(), classes[1].getSimpleName(), argSpec.toString(), argDescription, argSpec.scopeString()); }
1443414434
parseResultBuilder.addStringValue(argSpec, keyValue[0]);
1443514435
parseResultBuilder.addStringValue(argSpec, rawMapValue);
@@ -14700,7 +14700,8 @@ private int consumeOneArgument(ArgSpec argSpec,
1470014700
Object stronglyTypedValue = tryConvert(argSpec, index, converter, value, 0);
1470114701
result.add(stronglyTypedValue);
1470214702
if (tracer().isInfo()) {
14703-
tracer().info("Adding [%s] to %s for %s on %s", String.valueOf(result.get(result.size() - 1)), argSpec.toString(), argDescription, argSpec.scopeString());
14703+
String displayVal = !argSpec.interactive() || argSpec.echo() ? String.valueOf(stronglyTypedValue) : MASKED_VALUE;
14704+
tracer().info("Adding [%s] to %s for %s on %s", displayVal, argSpec.toString(), argDescription, argSpec.scopeString());
1470414705
}
1470514706
parseResultBuilder.addStringValue(argSpec, value);
1470614707
}
@@ -14720,7 +14721,7 @@ private boolean canConsumeOneArgument(ArgSpec argSpec, LookBehind lookBehind, bo
1472014721
tryConvert(argSpec, -1, converter, value, 0);
1472114722
}
1472214723
return true;
14723-
} catch (PicocliException ex) {
14724+
} catch (PicocliException ex) { // Note: we don't mask hidden (echo=false) options to facilitate debugging
1472414725
tracer().debug("%s cannot be assigned to %s: type conversion fails: %s.", arg, argDescription, ex.getMessage());
1472514726
return false;
1472614727
}

‎src/test/java/picocli/InteractiveArgTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ class App {
6363
String trace = errBaos.toString();
6464
assertThat(trace, containsString("User entered 10 characters"));
6565
assertThat(trace, containsString(
66-
"Setting " + specX.toString() + " to *** (masked interactive value)"));
66+
"Setting " + specX.toString() + " to *****(masked) (interactive value)"));
6767
assertThat(trace, not(containsString("1234567890")));
6868

6969
cmd.parseArgs("-z", "678");
@@ -607,7 +607,7 @@ class App {
607607
String trace = errBaos.toString();
608608
assertThat(trace, containsString("User entered 10 characters"));
609609
assertThat(trace, containsString(
610-
"Setting " + specX.toString() + " to *** (masked interactive value)"));
610+
"Setting " + specX.toString() + " to *****(masked) (interactive value)"));
611611
assertThat(trace, not(containsString("1234567890")));
612612
} finally {
613613
System.setOut(out);

0 commit comments

Comments
 (0)