honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove string formatting from Terminal print methods 

This can be trappy and wrong formating strings can throws format exceptions and hide the real message.
This commit is contained in:
Tanguy Leroux 2016-02-02 12:01:39 +01:00
parent cf28d62fc4
commit 865bbc2096
11 changed files with 66 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
core/src/main/java/org/elasticsearch/bootstrap/BootstrapCLIParser.java
View File

@ -83,7 +83,9 @@ final class BootstrapCLIParser extends CliTool {
@Override
public ExitStatus execute(Settings settings, Environment env) throws Exception {
terminal.println("Version: %s, Build: %s/%s, JVM: %s", org.elasticsearch.Version.CURRENT, Build.CURRENT.shortHash(), Build.CURRENT.date(), JvmInfo.jvmInfo().version());
terminal.println("Version: " + org.elasticsearch.Version.CURRENT
+ ", Build: " + Build.CURRENT.shortHash() + "/" + Build.CURRENT.date()
+ ", JVM: " + JvmInfo.jvmInfo().version());
return ExitStatus.OK_AND_EXIT;
}
}

9
core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java
View File

@ -100,8 +100,9 @@ public abstract class CheckFileCommand extends CliTool.Command {
Set<PosixFilePermission> permissionsBeforeWrite = entry.getValue();
Set<PosixFilePermission> permissionsAfterWrite = Files.getPosixFilePermissions(entry.getKey());
if (!permissionsBeforeWrite.equals(permissionsAfterWrite)) {
terminal.printWarn("The file permissions of [%s] have changed from [%s] to [%s]",
entry.getKey(), PosixFilePermissions.toString(permissionsBeforeWrite), PosixFilePermissions.toString(permissionsAfterWrite));
terminal.printWarn("The file permissions of [" + entry.getKey() + "] have changed "
+ "from [" + PosixFilePermissions.toString(permissionsBeforeWrite) + "] "
+ "to [" + PosixFilePermissions.toString(permissionsAfterWrite) + "]");
terminal.printWarn("Please ensure that the user account running Elasticsearch has read access to this file!");
}
}
@ -115,7 +116,7 @@ public abstract class CheckFileCommand extends CliTool.Command {
String ownerBeforeWrite = entry.getValue();
String ownerAfterWrite = Files.getOwner(entry.getKey()).getName();
if (!ownerAfterWrite.equals(ownerBeforeWrite)) {
terminal.printWarn("WARN: Owner of file [%s] used to be [%s], but now is [%s]", entry.getKey(), ownerBeforeWrite, ownerAfterWrite);
terminal.printWarn("WARN: Owner of file [" + entry.getKey() + "] used to be [" + ownerBeforeWrite + "], but now is [" + ownerAfterWrite + "]");
}
}
@ -128,7 +129,7 @@ public abstract class CheckFileCommand extends CliTool.Command {
String groupBeforeWrite = entry.getValue();
String groupAfterWrite = Files.readAttributes(entry.getKey(), PosixFileAttributes.class).group().getName();
if (!groupAfterWrite.equals(groupBeforeWrite)) {
terminal.printWarn("WARN: Group of file [%s] used to be [%s], but now is [%s]", entry.getKey(), groupBeforeWrite, groupAfterWrite);
terminal.printWarn("WARN: Group of file [" + entry.getKey() + "] used to be [" + groupBeforeWrite + "], but now is [" + groupAfterWrite + "]");
}
}

4
core/src/main/java/org/elasticsearch/common/cli/CliTool.java
View File

@ -20,7 +20,6 @@
package org.elasticsearch.common.cli;
import org.apache.commons.cli.AlreadySelectedException;
import org.apache.commons.cli.AmbiguousOptionException;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.CommandLineParser;
import org.apache.commons.cli.DefaultParser;
@ -31,7 +30,6 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.node.internal.InternalSettingsPreparer;
import java.io.IOException;
import java.util.Locale;
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
@ -127,7 +125,7 @@ public abstract class CliTool {
String cmdName = args[0];
cmd = config.cmd(cmdName);
if (cmd == null) {
terminal.printError("unknown command [%s]. Use [-h] option to list available commands", cmdName);
terminal.printError("unknown command [" + cmdName + "]. Use [-h] option to list available commands");
return ExitStatus.USAGE;
}

36
core/src/main/java/org/elasticsearch/common/cli/Terminal.java
View File

@ -89,37 +89,37 @@ public abstract class Terminal {
println(Verbosity.NORMAL);
}
public void println(String msg, Object... args) {
println(Verbosity.NORMAL, msg, args);
public void println(String msg) {
println(Verbosity.NORMAL, msg);
}
public void print(String msg, Object... args) {
print(Verbosity.NORMAL, msg, args);
public void print(String msg) {
print(Verbosity.NORMAL, msg);
}
public void println(Verbosity verbosity) {
println(verbosity, "");
}
public void println(Verbosity verbosity, String msg, Object... args) {
print(verbosity, msg + System.lineSeparator(), args);
public void println(Verbosity verbosity, String msg) {
print(verbosity, msg + System.lineSeparator());
}
public void print(Verbosity verbosity, String msg, Object... args) {
public void print(Verbosity verbosity, String msg) {
if (this.verbosity.enabled(verbosity)) {
doPrint(msg, args);
doPrint(msg);
}
}
public void printError(String msg, Object... args) {
println(Verbosity.SILENT, "ERROR: " + msg, args);
public void printError(String msg) {
println(Verbosity.SILENT, "ERROR: " + msg);
}
public void printWarn(String msg, Object... args) {
println(Verbosity.SILENT, "WARN: " + msg, args);
public void printWarn(String msg) {
println(Verbosity.SILENT, "WARN: " + msg);
}
protected abstract void doPrint(String msg, Object... args);
protected abstract void doPrint(String msg);
private static class ConsoleTerminal extends Terminal {
@ -130,8 +130,8 @@ public abstract class Terminal {
}
@Override
public void doPrint(String msg, Object... args) {
console.printf(msg, args);
public void doPrint(String msg) {
console.printf("%s", msg);
console.flush();
}
@ -157,13 +157,13 @@ public abstract class Terminal {
private final PrintWriter printWriter = new PrintWriter(System.out);
@Override
public void doPrint(String msg, Object... args) {
System.out.print(String.format(Locale.ROOT, msg, args));
public void doPrint(String msg) {
System.out.print(msg);
}
@Override
public String readText(String text, Object... args) {
print(text, args);
print(text);
BufferedReader reader = new BufferedReader(new InputStreamReader(System.in));
try {
return reader.readLine();

4
core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java
View File

@ -247,8 +247,8 @@ public class InternalSettingsPreparer {
}
if (secret) {
return new String(terminal.readSecret("Enter value for [%s]: ", key));
return new String(terminal.readSecret("Enter value for [" + key + "]: ", key));
}
return terminal.readText("Enter value for [%s]: ", key);
return terminal.readText("Enter value for [" + key + "]: ", key);
}
}

28
core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java
View File

@ -19,6 +19,18 @@
package org.elasticsearch.plugins;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.Build;
import org.elasticsearch.Version;
import org.elasticsearch.bootstrap.JarHell;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.cli.UserError;
import org.elasticsearch.common.hash.MessageDigests;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
@ -42,18 +54,6 @@ import java.util.Set;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.Build;
import org.elasticsearch.Version;
import org.elasticsearch.bootstrap.JarHell;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.cli.UserError;
import org.elasticsearch.common.hash.MessageDigests;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import static java.util.Collections.unmodifiableSet;
import static org.elasticsearch.common.cli.Terminal.Verbosity.VERBOSE;
import static org.elasticsearch.common.util.set.Sets.newHashSet;
@ -133,7 +133,7 @@ class InstallPluginCommand extends CliTool.Command {
// TODO: remove this leniency!! is it needed anymore?
if (Files.exists(env.pluginsFile()) == false) {
terminal.println("Plugins directory [%s] does not exist. Creating...", env.pluginsFile());
terminal.println("Plugins directory [" + env.pluginsFile() + "] does not exist. Creating...");
Files.createDirectory(env.pluginsFile());
}
@ -242,7 +242,7 @@ class InstallPluginCommand extends CliTool.Command {
private PluginInfo verify(Path pluginRoot, Environment env) throws Exception {
// read and validate the plugin descriptor
PluginInfo info = PluginInfo.readFromProperties(pluginRoot);
terminal.println(VERBOSE, "%s", info);
terminal.println(VERBOSE, info.toString());
// don't let luser install plugin as a module...
// they might be unavoidably in maven central and are packaged up the same way)

2
core/src/main/java/org/elasticsearch/plugins/PluginSecurity.java
View File

@ -87,7 +87,7 @@ class PluginSecurity {
terminal.println(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@");
// print all permissions:
for (Permission permission : requested) {
terminal.println(Verbosity.NORMAL, "* %s", formatPermission(permission));
terminal.println(Verbosity.NORMAL, "* " + formatPermission(permission));
}
terminal.println(Verbosity.NORMAL, "See http://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html");
terminal.println(Verbosity.NORMAL, "for descriptions of what these permissions allow and the associated risks.");

16
core/src/main/java/org/elasticsearch/plugins/RemovePluginCommand.java
View File

@ -19,12 +19,6 @@
package org.elasticsearch.plugins;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.util.ArrayList;
import java.util.List;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.cli.CliTool;
@ -33,6 +27,12 @@ import org.elasticsearch.common.cli.UserError;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.util.ArrayList;
import java.util.List;
import static org.elasticsearch.common.cli.Terminal.Verbosity.VERBOSE;
/**
@ -63,10 +63,10 @@ class RemovePluginCommand extends CliTool.Command {
throw new UserError(CliTool.ExitStatus.IO_ERROR, "Bin dir for " + pluginName + " is not a directory");
}
pluginPaths.add(pluginBinDir);
terminal.println(VERBOSE, "Removing: %s", pluginBinDir);
terminal.println(VERBOSE, "Removing: " + pluginBinDir);
}
terminal.println(VERBOSE, "Removing: %s", pluginDir);
terminal.println(VERBOSE, "Removing: " + pluginDir);
Path tmpPluginDir = env.pluginsFile().resolve(".removing-" + pluginName);
Files.move(pluginDir, tmpPluginDir, StandardCopyOption.ATOMIC_MOVE);
pluginPaths.add(tmpPluginDir);

8
core/src/test/java/org/elasticsearch/common/cli/TerminalTests.java
View File

@ -19,9 +19,6 @@
package org.elasticsearch.common.cli;
import java.nio.file.NoSuchFileException;
import java.util.List;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasSize;
@ -46,6 +43,11 @@ public class TerminalTests extends CliToolTestCase {
assertPrinted(terminal, Terminal.Verbosity.VERBOSE, "text");
}
public void testEscaping() throws Exception {
CaptureOutputTerminal terminal = new CaptureOutputTerminal(Terminal.Verbosity.NORMAL);
assertPrinted(terminal, Terminal.Verbosity.NORMAL, "This message contains percent like %20n");
}
private void assertPrinted(CaptureOutputTerminal logTerminal, Terminal.Verbosity verbosity, String text) {
logTerminal.print(verbosity, text);
assertThat(logTerminal.getTerminalOutput(), hasSize(1));

5
plugins/mapper-attachments/src/test/java/org/elasticsearch/mapper/attachments/StandaloneRunner.java
View File

@ -36,7 +36,6 @@ import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.DocumentMapperParser;
import org.elasticsearch.index.mapper.ParseContext;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
@ -119,7 +118,7 @@ public class StandaloneRunner extends CliTool {
terminal.println("## Extracted text");
terminal.println("--------------------- BEGIN -----------------------");
terminal.println("%s", doc.get("file.content"));
terminal.println(doc.get("file.content"));
terminal.println("---------------------- END ------------------------");
terminal.println("## Metadata");
printMetadataContent(doc, AttachmentMapper.FieldNames.AUTHOR);
@ -135,7 +134,7 @@ public class StandaloneRunner extends CliTool {
}
private void printMetadataContent(ParseContext.Document doc, String field) {
terminal.println("- %s: %s", field, doc.get(docMapper.mappers().getMapper("file." + field).fieldType().name()));
terminal.println("- " + field + ":" + doc.get(docMapper.mappers().getMapper("file." + field).fieldType().name()));
}
public static byte[] copyToBytes(Path path) throws IOException {

17
test/framework/src/main/java/org/elasticsearch/common/cli/CliToolTestCase.java
View File

@ -28,11 +28,8 @@ import org.junit.After;
import org.junit.Before;
import java.io.IOException;
import java.io.PrintWriter;
import java.io.Writer;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.greaterThan;
@ -73,7 +70,7 @@ public abstract class CliToolTestCase extends ESTestCase {
}
@Override
protected void doPrint(String msg, Object... args) {
protected void doPrint(String msg) {
}
@Override
@ -87,7 +84,7 @@ public abstract class CliToolTestCase extends ESTestCase {
}
@Override
public void print(String msg, Object... args) {
public void print(String msg) {
}
@Override
@ -99,7 +96,7 @@ public abstract class CliToolTestCase extends ESTestCase {
*/
public static class CaptureOutputTerminal extends MockTerminal {
List<String> terminalOutput = new ArrayList();
List<String> terminalOutput = new ArrayList<>();
public CaptureOutputTerminal() {
super(Verbosity.NORMAL);
@ -110,13 +107,13 @@ public abstract class CliToolTestCase extends ESTestCase {
}
@Override
protected void doPrint(String msg, Object... args) {
terminalOutput.add(String.format(Locale.ROOT, msg, args));
protected void doPrint(String msg) {
terminalOutput.add(msg);
}
@Override
public void print(String msg, Object... args) {
doPrint(msg, args);
public void print(String msg) {
doPrint(msg);
}
@Override