GEODE-1347: do not echo back password and clear history file * rename toHistoryLoggable to redact
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/f77739be Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/f77739be Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/f77739be Branch: refs/heads/feature/GEODE-1255 Commit: f77739be9bf843509ecb6c9023675829f0223314 Parents: 32a6987 Author: Jinmei Liao <jil...@pivotal.io> Authored: Wed May 4 10:08:58 2016 -0700 Committer: Jinmei Liao <jil...@pivotal.io> Committed: Thu May 5 07:46:00 2016 -0700 ---------------------------------------------------------------------- .../management/internal/cli/Launcher.java | 9 ++- .../internal/cli/commands/ShellCommands.java | 59 ++++++++++---------- .../management/internal/cli/shell/Gfsh.java | 41 ++++++++------ .../internal/cli/shell/GfshConfig.java | 11 ++++ .../internal/cli/shell/jline/GfshHistory.java | 12 ++-- .../cli/shell/GfshHistoryJUnitTest.java | 28 +++++++--- 6 files changed, 96 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f77739be/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/Launcher.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/Launcher.java b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/Launcher.java index fe806f6..5d57db6 100755 --- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/Launcher.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/Launcher.java @@ -22,14 +22,16 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import org.springframework.shell.core.ExitShellRequest; - import com.gemstone.gemfire.internal.GemFireVersion; import com.gemstone.gemfire.internal.PureJavaMode; import com.gemstone.gemfire.management.internal.cli.i18n.CliStrings; import com.gemstone.gemfire.management.internal.cli.parser.SyntaxConstants; import com.gemstone.gemfire.management.internal.cli.shell.Gfsh; import com.gemstone.gemfire.management.internal.cli.shell.GfshConfig; +import com.gemstone.gemfire.management.internal.cli.shell.jline.GfshHistory; + +import org.springframework.shell.core.ExitShellRequest; + import joptsimple.OptionException; import joptsimple.OptionParser; import joptsimple.OptionSet; @@ -220,7 +222,8 @@ public final class Launcher { // Execute all of the commands in the list, one at a time. for (int i = 0; i < commandsToExecute.size() && exitRequest == ExitShellRequest.NORMAL_EXIT; i++) { String command = commandsToExecute.get(i); - System.out.println(GfshParser.LINE_SEPARATOR + "(" + (i + 1) + ") Executing - " + command + // sanitize the output string to not show the password + System.out.println(GfshParser.LINE_SEPARATOR + "(" + (i + 1) + ") Executing - " + GfshHistory.redact(command) + GfshParser.LINE_SEPARATOR); if (!gfsh.executeScriptLine(command) || gfsh.getLastExecutionStatus() != 0) { exitRequest = ExitShellRequest.FATAL_EXIT; http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f77739be/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ShellCommands.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ShellCommands.java b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ShellCommands.java index 0af040a..a9712a1 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ShellCommands.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/ShellCommands.java @@ -16,6 +16,33 @@ */ package com.gemstone.gemfire.management.internal.cli.commands; +import java.io.BufferedReader; +import java.io.BufferedWriter; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileWriter; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.Writer; +import java.net.ConnectException; +import java.net.MalformedURLException; +import java.net.URL; +import java.security.KeyStore; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Properties; +import java.util.Set; +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.TrustManagerFactory; + import com.gemstone.gemfire.distributed.internal.DistributionConfig; import com.gemstone.gemfire.internal.ClassPathLoader; import com.gemstone.gemfire.internal.DSFIDFactory; @@ -52,39 +79,13 @@ import com.gemstone.gemfire.management.internal.web.domain.LinkIndex; import com.gemstone.gemfire.management.internal.web.http.support.SimpleHttpRequester; import com.gemstone.gemfire.management.internal.web.shell.HttpOperationInvoker; import com.gemstone.gemfire.management.internal.web.shell.RestHttpOperationInvoker; + import org.springframework.shell.core.CommandMarker; import org.springframework.shell.core.ExitShellRequest; import org.springframework.shell.core.annotation.CliAvailabilityIndicator; import org.springframework.shell.core.annotation.CliCommand; import org.springframework.shell.core.annotation.CliOption; -import javax.net.ssl.HttpsURLConnection; -import javax.net.ssl.KeyManagerFactory; -import javax.net.ssl.SSLContext; -import javax.net.ssl.TrustManagerFactory; -import java.io.BufferedReader; -import java.io.BufferedWriter; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileWriter; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.Writer; -import java.net.ConnectException; -import java.net.MalformedURLException; -import java.net.URL; -import java.security.KeyStore; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Properties; -import java.util.Set; - /** * * @since 7.0 @@ -868,7 +869,6 @@ private void configureHttpsURLConnection(Map<String, String> sslConfigProps) thr int historySizeWordLength = historySizeString.length(); GfshHistory gfshHistory = gfsh.getGfshHistory(); - //List<?> gfshHistoryList = gfshHistory.getHistoryList(); Iterator<?> it = gfshHistory.entries(); boolean flagForLineNumbers = (saveHistoryTo != null && saveHistoryTo .length() > 0) ? false : true; @@ -944,8 +944,7 @@ private void configureHttpsURLConnection(Map<String, String> sslConfigProps) thr Result executeClearHistory(){ try{ Gfsh gfsh = Gfsh.getCurrentInstance(); - GfshHistory gfshHistory = gfsh.getGfshHistory(); - gfshHistory.clear(); + gfsh.clearHistory(); }catch(Exception e){ LogWrapper.getInstance().info(CliUtil.stackTraceAsString(e) ); return ResultBuilder.createGemFireErrorResult("Exception occured while clearing history " + e.getMessage()); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f77739be/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/Gfsh.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/Gfsh.java b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/Gfsh.java index e5bcf06..9688441 100755 --- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/Gfsh.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/Gfsh.java @@ -36,19 +36,6 @@ import java.util.logging.Level; import java.util.logging.LogManager; import java.util.logging.Logger; -import jline.Terminal; - -import jline.console.ConsoleReader; -import org.springframework.shell.core.AbstractShell; -import org.springframework.shell.core.CommandMarker; -import org.springframework.shell.core.Converter; -import org.springframework.shell.core.ExecutionStrategy; -import org.springframework.shell.core.ExitShellRequest; -import org.springframework.shell.core.JLineLogHandler; -import org.springframework.shell.core.JLineShell; -import org.springframework.shell.core.Parser; -import org.springframework.shell.event.ShellStatus.Status; - import com.gemstone.gemfire.internal.Banner; import com.gemstone.gemfire.internal.GemFireVersion; import com.gemstone.gemfire.internal.lang.ClassUtils; @@ -73,6 +60,19 @@ import com.gemstone.gemfire.management.internal.cli.shell.jline.GfshUnsupportedT import com.gemstone.gemfire.management.internal.cli.shell.unsafe.GfshSignalHandler; import com.gemstone.gemfire.management.internal.cli.util.CommentSkipHelper; +import org.springframework.shell.core.AbstractShell; +import org.springframework.shell.core.CommandMarker; +import org.springframework.shell.core.Converter; +import org.springframework.shell.core.ExecutionStrategy; +import org.springframework.shell.core.ExitShellRequest; +import org.springframework.shell.core.JLineLogHandler; +import org.springframework.shell.core.JLineShell; +import org.springframework.shell.core.Parser; +import org.springframework.shell.event.ShellStatus.Status; + +import jline.Terminal; +import jline.console.ConsoleReader; + /** * Extends an interactive shell provided by <a * href="https://github.com/SpringSource/spring-shell">Spring Shell</a> library. @@ -607,11 +607,11 @@ public class Gfsh extends JLineShell { String originalString = expandedPropCommandsMap.get(processedLine); if (originalString != null) { // In history log the original command string & expanded line as a comment - super.logCommandToOutput(GfshHistory.toHistoryLoggable(originalString)); - super.logCommandToOutput(GfshHistory.toHistoryLoggable("// Post substitution")); - super.logCommandToOutput(GfshHistory.toHistoryLoggable("//" + processedLine)); + super.logCommandToOutput(GfshHistory.redact(originalString)); + super.logCommandToOutput(GfshHistory.redact("// Post substitution")); + super.logCommandToOutput(GfshHistory.redact("//" + processedLine)); } else { - super.logCommandToOutput(GfshHistory.toHistoryLoggable(processedLine)); + super.logCommandToOutput(GfshHistory.redact(processedLine)); } } @@ -1030,6 +1030,13 @@ public class Gfsh extends JLineShell { return gfshConfig.getHistoryFileName(); } + public void clearHistory() { + gfshHistory.clear(); + if(!gfshConfig.deleteHistoryFile()){ + printAsWarning("Gfsh history file is not deleted"); + } + } + public String getLogFilePath() { return gfshConfig.getLogFilePath(); } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f77739be/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/GfshConfig.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/GfshConfig.java b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/GfshConfig.java index 9ee0284..11a767a 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/GfshConfig.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/GfshConfig.java @@ -60,6 +60,17 @@ public class GfshConfig { this(HISTORY_FILE.getAbsolutePath(), DEFAULT_PROMPT, MAX_HISTORY_SIZE, null, null, null, null, null); } + public boolean deleteHistoryFile(){ + if(historyFileName==null) + return true; + + File file = new File(historyFileName); + if(!file.exists()) + return true; + + return file.delete(); + } + public GfshConfig(String historyFileName, String defaultPrompt, int historySize, String logDir, Level logLevel, Integer logLimit, Integer logCount, String initFileName) { http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f77739be/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/jline/GfshHistory.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/jline/GfshHistory.java b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/jline/GfshHistory.java index 7bfc937..2175692 100644 --- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/jline/GfshHistory.java +++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/shell/jline/GfshHistory.java @@ -16,15 +16,13 @@ */ package com.gemstone.gemfire.management.internal.cli.shell.jline; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import com.gemstone.gemfire.management.internal.cli.parser.preprocessor.PreprocessorUtils; import jline.console.history.MemoryHistory; -import java.io.File; -import java.io.IOException; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - /** * Overrides jline.History to add History without newline characters. * @@ -40,7 +38,7 @@ public class GfshHistory extends MemoryHistory { public void addToHistory(String buffer) { if (isAutoFlush()) { - super.add(toHistoryLoggable(buffer)); + super.add(redact(buffer)); } } @@ -52,7 +50,7 @@ public class GfshHistory extends MemoryHistory { this.autoFlush = autoFlush; } - public static String toHistoryLoggable(String buffer) { + public static String redact(String buffer) { String trimmed = PreprocessorUtils.trim(buffer, false).getString(); Matcher matcher = passwordRe.matcher(trimmed); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/f77739be/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/shell/GfshHistoryJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/shell/GfshHistoryJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/shell/GfshHistoryJUnitTest.java index 3ad9ce8..b2081d2 100644 --- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/shell/GfshHistoryJUnitTest.java +++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/shell/GfshHistoryJUnitTest.java @@ -16,7 +16,15 @@ */ package com.gemstone.gemfire.management.internal.cli.shell; +import static org.junit.Assert.*; + +import java.io.File; +import java.lang.reflect.Field; +import java.nio.file.Files; +import java.util.List; + import com.gemstone.gemfire.test.junit.categories.UnitTest; + import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -24,13 +32,6 @@ import org.junit.Test; import org.junit.experimental.categories.Category; import org.junit.rules.TemporaryFolder; -import java.io.File; -import java.lang.reflect.Field; -import java.nio.file.Files; -import java.util.List; - -import static org.junit.Assert.assertEquals; - @Category(UnitTest.class) public class GfshHistoryJUnitTest { @@ -83,4 +84,17 @@ public class GfshHistoryJUnitTest { assertEquals("// [failed] connect --password=***** --password = ***** --password= ***** --password =***** --password-param=***** --other-password-param= *****", lines.get(1)); } + + @Test + public void testClearHistory() throws Exception{ + Gfsh gfsh = Gfsh.getInstance(false, new String[] {}, gfshConfig); + gfsh.executeScriptLine("connect --fake-param=foo"); + List<String> lines = Files.readAllLines(gfshHistoryFile.toPath()); + assertEquals(2, lines.size()); + + // clear the history + gfsh.clearHistory(); + assertEquals(gfsh.getGfshHistory().size(), 0); + assertFalse(gfshHistoryFile.exists()); + } }