Mercurial > hg > release > thermostat-1.6
changeset 1712:b2a64131e8a6
Add warning emission ability to ShellArgsParser
ShellArgsParser can emit warnings/errors and ShellCommand will print warnings,
if any, before attempting to run commands (which will likely fail because of
malformed arguments). If errors are present then they are printed and the
command run is aborted.
Reviewed-by: jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2015-July/014408.html
author | Andrew Azores <aazores@redhat.com> |
---|---|
date | Tue, 07 Jul 2015 12:23:20 -0400 |
parents | 276b0f97412a |
children | a1f58895a63f |
files | launcher/src/main/java/com/redhat/thermostat/launcher/internal/LocaleResources.java launcher/src/main/java/com/redhat/thermostat/launcher/internal/ShellArgsParser.java launcher/src/main/java/com/redhat/thermostat/launcher/internal/ShellCommand.java launcher/src/main/resources/com/redhat/thermostat/launcher/internal/strings.properties launcher/src/test/java/com/redhat/thermostat/launcher/internal/ShellArgsParserTest.java |
diffstat | 5 files changed, 411 insertions(+), 11 deletions(-) [+] |
line wrap: on
line diff
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LocaleResources.java Thu Jul 02 10:10:40 2015 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LocaleResources.java Tue Jul 07 12:23:20 2015 -0400 @@ -69,6 +69,9 @@ LAUNCHER_FIRST_LAUNCH_MSG, INVALID_DB_URL, + PARSE_ISSUES_CALLED_BEFORE_PARSE, + PARSER_ERROR, + PARSER_WARNING, ; static final String RESOURCE_BUNDLE = "com.redhat.thermostat.launcher.internal.strings";
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ShellArgsParser.java Thu Jul 02 10:10:40 2015 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ShellArgsParser.java Tue Jul 07 12:23:20 2015 -0400 @@ -36,9 +36,14 @@ package com.redhat.thermostat.launcher.internal; +import com.redhat.thermostat.shared.locale.Translate; + import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import static java.util.Objects.requireNonNull; + /** * Parser for thermostat shell command line input. Splits lines on whitespaces to chunk commands and arguments, * respecting quotation marks, so that ex: @@ -49,23 +54,27 @@ */ class ShellArgsParser { + private static final Translate<LocaleResources> translator = LocaleResources.createLocalizer(); + + private static final int NOT_YET_PARSED = -1; private final String input; - private int pos = 0; + private int pos = NOT_YET_PARSED; private char c; + private int quoteCount = 0; + private final Issues issues = new Issues(); ShellArgsParser(String input) { this.input = input; - if (input.length() > 0) { - c = input.charAt(pos); - } } String[] parse() { if (input.isEmpty()) { + ++pos; return new String[]{}; } + readChar(); List<String> result = new ArrayList<>(); - while (ready()) { + do { if (isWhitespace()) { whitespace(); } else if (isQuote()) { @@ -73,8 +82,15 @@ } else { result.add(word()); } + } while (!finished()); + return result.toArray(new String[result.size()]); + } + + Issues getParseIssues() { + if (pos == NOT_YET_PARSED) { + throw new IllegalStateException(translator.localize(LocaleResources.PARSE_ISSUES_CALLED_BEFORE_PARSE).getContents()); } - return result.toArray(new String[result.size()]); + return issues; } private void whitespace() { @@ -85,6 +101,8 @@ private String quote() { StringBuilder sb = new StringBuilder(); + boolean closed = false; + int startPos = pos; while (ready()) { readChar(); if (isEscapedQuote()) { @@ -93,10 +111,26 @@ continue; } if (isQuote()) { + if (ready()) { + readChar(); + if (!ready()) { + if (isQuote()) { + issues.addIssue(new Issue(pos, Issue.Type.UNMATCHED_QUOTE)); + } + } else { + if (!isWhitespace()) { + issues.addIssue(new Issue(pos, Issue.Type.EXPECTED_WHITESPACE)); + } + } + } + closed = true; break; } sb.append(c); } + if (!closed) { + issues.addIssue(new Issue(startPos, Issue.Type.UNMATCHED_QUOTE)); + } return sb.toString(); } @@ -113,6 +147,9 @@ if (!isWhitespace()) { sb.append(c); } + if (isQuote()) { + issues.addIssue(new Issue(pos, Issue.Type.UNEXPECTED_QUOTE)); + } } return sb.toString(); } @@ -121,6 +158,10 @@ return pos < input.length() - 1; } + private boolean finished() { + return pos == (input.length() - 1); + } + private char readChar() { c = input.charAt(++pos); return c; @@ -142,4 +183,112 @@ return Character.isWhitespace(c); } + static class Issues { + private final List<Issue> issues = new ArrayList<>(); + + void addIssue(Issue issue) { + this.issues.add(requireNonNull(issue)); + } + + List<Issue> getAllIssues() { + return Collections.unmodifiableList(issues); + } + + List<Issue> getWarnings() { + return Collections.unmodifiableList(filterIssuesBySeverity(getAllIssues(), Issue.Type.Severity.WARN)); + } + + List<Issue> getErrors() { + return Collections.unmodifiableList(filterIssuesBySeverity(getAllIssues(), Issue.Type.Severity.ERROR)); + } + + private static List<Issue> filterIssuesBySeverity(Iterable<Issue> issues, Issue.Type.Severity severity) { + List<Issue> results = new ArrayList<>(); + for (Issue issue : issues) { + if (issue.getType().getSeverity().equals(severity)) { + results.add(issue); + } + } + return results; + } + } + + static class Issue { + private final int columnNumber; + private final Type type; + + Issue(int columnNumber, Type type) { + this.columnNumber = columnNumber; + this.type = requireNonNull(type); + } + + int getColumnNumber() { + return columnNumber; + } + + Type getType() { + return type; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + Issue issue = (Issue) o; + + if (columnNumber != issue.columnNumber) return false; + return type == issue.type; + + } + + @Override + public int hashCode() { + int result = columnNumber; + result = 31 * result + type.hashCode(); + return result; + } + + @Override + public String toString() { + return type.getSeverity() + " : " + type.toString() + " col " + Integer.toString(columnNumber); + } + + enum Type { + UNMATCHED_QUOTE(Severity.ERROR), + UNEXPECTED_QUOTE(Severity.ERROR), + EXPECTED_WHITESPACE(Severity.WARN), + ; + + private final Severity severity; + + Type(Severity severity) { + this.severity = requireNonNull(severity); + } + + public Severity getSeverity() { + return severity; + } + + enum Severity { + WARN, + ERROR, + } + } + } + + static class IssuesFormatter { + String format(Issues issues) { + return format(issues.getAllIssues()); + } + + String format(Iterable<Issue> issues) { + StringBuilder sb = new StringBuilder(); + for (Issue issue : issues) { + sb.append(issue.toString()).append(System.lineSeparator()); + } + return sb.toString(); + } + } + }
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ShellCommand.java Thu Jul 02 10:10:40 2015 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ShellCommand.java Tue Jul 07 12:23:20 2015 -0400 @@ -196,7 +196,21 @@ } private void launchCommand(String line) throws CommandException { - String[] parsed = new ShellArgsParser(line).parse(); + ShellArgsParser parser = new ShellArgsParser(line); + String[] parsed = parser.parse(); + ShellArgsParser.Issues issues = parser.getParseIssues(); + if (!issues.getAllIssues().isEmpty()) { + ShellArgsParser.IssuesFormatter issuesFormatter = new ShellArgsParser.IssuesFormatter(); + if (!issues.getWarnings().isEmpty()) { + String formattedIssues = issuesFormatter.format(issues.getWarnings()); + logger.warning(t.localize(LocaleResources.PARSER_WARNING, formattedIssues).getContents()); + } + if (!issues.getErrors().isEmpty()) { + String formattedIssues = issuesFormatter.format(issues.getErrors()); + logger.warning(t.localize(LocaleResources.PARSER_ERROR, formattedIssues).getContents()); + return; + } + } ServiceReference launcherRef = bundleContext.getServiceReference(Launcher.class.getName()); if (launcherRef != null) { Launcher launcher = (Launcher) bundleContext.getService(launcherRef);
--- a/launcher/src/main/resources/com/redhat/thermostat/launcher/internal/strings.properties Thu Jul 02 10:10:40 2015 -0400 +++ b/launcher/src/main/resources/com/redhat/thermostat/launcher/internal/strings.properties Tue Jul 07 12:23:20 2015 -0400 @@ -33,4 +33,8 @@ LAUNCHER_FIRST_LAUNCH_MSG = This appears to be the first time Thermostat has been launched.\n\ Please run ''{0}''. -INVALID_DB_URL = Warning: Shell encountered a invalid database connection url: {0} Connection to storage may be corrupted. \ No newline at end of file +INVALID_DB_URL = Warning: Shell encountered a invalid database connection url: {0} Connection to storage may be corrupted. + +PARSE_ISSUES_CALLED_BEFORE_PARSE = ShellArgsParser#getParseIssues called before ShellArgsParser#parse +PARSER_ERROR = Could not parse input:\n{0} +PARSER_WARNING = Malformed input, attempting to proceed anyway:\n{0} \ No newline at end of file
--- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ShellArgsParserTest.java Thu Jul 02 10:10:40 2015 -0400 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ShellArgsParserTest.java Tue Jul 07 12:23:20 2015 -0400 @@ -38,122 +38,352 @@ import org.junit.Test; +import java.util.Arrays; +import java.util.List; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.core.IsNot.not; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class ShellArgsParserTest { + private ShellArgsParser.Issues issues; + + @Test(expected = IllegalStateException.class) + public void testGetIssuesBeforeParse() { + ShellArgsParser sap = new ShellArgsParser(""); + sap.getParseIssues(); + sap.parse(); + } + @Test public void testEmptyString() { assertEmpty(""); + assertNoIssues(); + } + + @Test + public void testOneCharArg() { + assertResult("f", "f"); + assertNoIssues(); } @Test public void testSingleArg() { assertResult("foo", "foo"); + assertNoIssues(); } @Test public void testTwoArgs() { assertResult("foo bar", "foo", "bar"); + assertNoIssues(); } @Test public void testThreeArgs() { assertResult("foo bar baz", "foo", "bar", "baz"); + assertNoIssues(); } @Test public void testLeadingSpaces() { assertResult(" foo", "foo"); + assertNoIssues(); } @Test public void testTrailingSpaces() { assertResult("foo ", "foo"); + assertNoIssues(); } @Test public void testInnerSpaces() { assertResult("foo bar", "foo", "bar"); + assertNoIssues(); } @Test public void testLotsOfSpaces() { assertResult(" foo bar ", "foo", "bar"); + assertNoIssues(); } @Test public void testOnlySpaces() { assertEmpty(" "); + assertNoIssues(); } @Test public void testTabCharacter() { assertResult("foo\tbar", "foo", "bar"); + assertNoIssues(); } @Test public void testQuotedArg() { assertResult("\"foo\"", "foo"); + assertNoIssues(); } @Test public void testQuotedString() { assertResult("\"foo bar\"", "foo bar"); + assertNoIssues(); + } + + @Test + public void testQuotedArgFirst() { + assertResult("\"foo bar\" baz", "foo bar", "baz"); + assertNoIssues(); } @Test public void testSingleStartingQuote() { - // malformed argument assertResult("\"foo", "foo"); + assertIssues(new ShellArgsParser.Issue(0, ShellArgsParser.Issue.Type.UNMATCHED_QUOTE)); } @Test public void testSingleEndingQuote() { - // malformed argument assertResult("foo\"", "foo\""); + assertIssues(new ShellArgsParser.Issue(3, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE)); } @Test public void testSingleMiddleQuote() { - // malformed argument assertResult("foo \" bar", "foo", " bar"); + assertIssues(new ShellArgsParser.Issue(4, ShellArgsParser.Issue.Type.UNMATCHED_QUOTE)); + } + + @Test + public void testOneQuoteMark() { + assertResult("\"", ""); + assertIssues(new ShellArgsParser.Issue(0, ShellArgsParser.Issue.Type.UNMATCHED_QUOTE)); + } + + @Test + public void testThreeQuoteMarks() { + assertResult("\"\"\"", ""); + assertIssues(new ShellArgsParser.Issue(2, ShellArgsParser.Issue.Type.UNMATCHED_QUOTE)); + } + + @Test + public void testFourQuoteMarks() { + assertResult("\"\"\"\"", "", ""); + assertIssues(new ShellArgsParser.Issue(2, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE)); + } + + @Test + public void testAdjacentQuotes() { + assertResult("\"f\"\"b\"", "f", "b"); + assertIssues(new ShellArgsParser.Issue(3 , ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE)); + } + + @Test + public void testQuoteAdjacentToWord() { + assertResult("foo\"bar\"", "foo\"bar\""); + assertIssues(new ShellArgsParser.Issue(3, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE), + new ShellArgsParser.Issue(7, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE)); } @Test public void testSingleEscapedQuote() { assertResult("foo\\\"", "foo\""); + assertNoIssues(); } @Test public void testQuoteContainingEscapedQuoteLiteral() { assertResult("\"foo \\\" bar\"", "foo \" bar"); + assertNoIssues(); } @Test public void testQuoteContainingEscapedQuoteLiteral2() { assertResult("\"foo \\\"\"", "foo \""); + assertNoIssues(); } @Test public void testQuotedEmptyString() { assertResult("\"\"", ""); + assertNoIssues(); } @Test public void testQuotedSpacesString() { assertResult("\" \"", " "); + assertNoIssues(); + } + + @Test + public void testIssueGetters() { + ShellArgsParser.Issue issue = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + int columnNumber = issue.getColumnNumber(); + ShellArgsParser.Issue.Type issueType = issue.getType(); + assertEquals(10, columnNumber); + assertEquals(ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE, issueType); + } + + @Test + public void testIssueEquality() { + ShellArgsParser.Issue issue1 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue2 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue3 = new ShellArgsParser.Issue(1, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue4 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE); + assertThat(issue1, is(equalTo(issue2))); + assertThat(issue1.hashCode(), is(equalTo(issue2.hashCode()))); + assertThat(issue1, not(equalTo(issue3))); + assertThat(issue1, not(equalTo(issue4))); + } + + @Test + public void testIssuesAllIssuesListContents() { + ShellArgsParser.Issue issue1 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue2 = new ShellArgsParser.Issue(15, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE); + ShellArgsParser.Issue issue3 = new ShellArgsParser.Issue(20, ShellArgsParser.Issue.Type.UNMATCHED_QUOTE); + ShellArgsParser.Issues issues = new ShellArgsParser.Issues(); + issues.addIssue(issue1); + issues.addIssue(issue1); + issues.addIssue(issue2); + issues.addIssue(issue3); + + assertThat(issues.getAllIssues().size(), is(equalTo(4))); + assertThat(issues.getAllIssues().size(), is(equalTo(issues.getWarnings().size() + issues.getErrors().size()))); + + assertTrue(issues.getAllIssues().contains(issue1)); + assertTrue(issues.getAllIssues().contains(issue2)); + assertTrue(issues.getAllIssues().contains(issue3)); + } + + @Test + public void testIssuesWarningsListContents() { + ShellArgsParser.Issue issue1 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue2 = new ShellArgsParser.Issue(15, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE); + ShellArgsParser.Issue issue3 = new ShellArgsParser.Issue(20, ShellArgsParser.Issue.Type.UNMATCHED_QUOTE); + ShellArgsParser.Issues issues = new ShellArgsParser.Issues(); + issues.addIssue(issue1); + issues.addIssue(issue1); + issues.addIssue(issue2); + issues.addIssue(issue3); + + assertThat(issues.getWarnings().size(), is(equalTo(1))); + + assertFalse(issues.getWarnings().contains(issue1)); + assertTrue(issues.getWarnings().contains(issue2)); + assertFalse(issues.getWarnings().contains(issue3)); + } + + @Test + public void testIssuesErrorsListContents() { + ShellArgsParser.Issue issue1 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue2 = new ShellArgsParser.Issue(15, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE); + ShellArgsParser.Issue issue3 = new ShellArgsParser.Issue(20, ShellArgsParser.Issue.Type.UNMATCHED_QUOTE); + ShellArgsParser.Issues issues = new ShellArgsParser.Issues(); + issues.addIssue(issue1); + issues.addIssue(issue1); + issues.addIssue(issue2); + issues.addIssue(issue3); + + assertThat(issues.getErrors().size(), is(equalTo(3))); + + assertTrue(issues.getErrors().contains(issue1)); + assertFalse(issues.getErrors().contains(issue2)); + assertTrue(issues.getErrors().contains(issue3)); + } + + @Test(expected = UnsupportedOperationException.class) + public void testIssuesAllIssuesListUnmodifiable() { + ShellArgsParser.Issue issue1 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue2 = new ShellArgsParser.Issue(15, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE); + ShellArgsParser.Issues issues = new ShellArgsParser.Issues(); + issues.addIssue(issue1); + + List<ShellArgsParser.Issue> allIssuesList = issues.getAllIssues(); + allIssuesList.add(issue2); + fail("Should have hit UnsupportedOperationException on add"); + } + + @Test(expected = UnsupportedOperationException.class) + public void testIssuesWarningsListUnmodifiable() { + ShellArgsParser.Issue issue1 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue2 = new ShellArgsParser.Issue(15, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE); + ShellArgsParser.Issues issues = new ShellArgsParser.Issues(); + issues.addIssue(issue1); + + List<ShellArgsParser.Issue> warningsList = issues.getWarnings(); + warningsList.add(issue2); + fail("Should have hit UnsupportedOperationException on add"); + } + + @Test(expected = UnsupportedOperationException.class) + public void testIssuesErrorsListUnmodifiable() { + ShellArgsParser.Issue issue1 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue2 = new ShellArgsParser.Issue(15, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE); + ShellArgsParser.Issues issues = new ShellArgsParser.Issues(); + issues.addIssue(issue1); + + List<ShellArgsParser.Issue> errorsList = issues.getErrors(); + errorsList.add(issue2); + fail("Should have hit UnsupportedOperationException on add"); + } + + @Test + public void testIssuesFormatterOutputContents() { + ShellArgsParser.Issue issue1 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue2 = new ShellArgsParser.Issue(15, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE); + ShellArgsParser.Issues issues = new ShellArgsParser.Issues(); + issues.addIssue(issue1); + issues.addIssue(issue2); + + ShellArgsParser.IssuesFormatter formatter = new ShellArgsParser.IssuesFormatter(); + String output = formatter.format(issues); + assertTrue(output.contains(issue1.toString())); + assertTrue(output.contains(issue2.toString())); + } + + @Test + public void testIssuesFormatterOutputSame() { + ShellArgsParser.Issue issue1 = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); + ShellArgsParser.Issue issue2 = new ShellArgsParser.Issue(15, ShellArgsParser.Issue.Type.EXPECTED_WHITESPACE); + ShellArgsParser.Issues issues = new ShellArgsParser.Issues(); + issues.addIssue(issue1); + issues.addIssue(issue2); + + ShellArgsParser.IssuesFormatter formatter = new ShellArgsParser.IssuesFormatter(); + String issuesOutput = formatter.format(issues); + String iterableOutput = formatter.format(issues); + assertThat(issuesOutput, is(equalTo(iterableOutput))); } private void assertEmpty(String input) { ShellArgsParser sap = new ShellArgsParser(input); String[] result = sap.parse(); + issues = sap.getParseIssues(); assertArrayEquals(new String[]{}, result); } private void assertResult(String input, String ... expecteds) { ShellArgsParser sap = new ShellArgsParser(input); String[] result = sap.parse(); + issues = sap.getParseIssues(); assertArrayEquals(expecteds, result); } + + private void assertNoIssues() { + assertTrue(issues.getAllIssues().isEmpty()); + } + + private void assertIssues(ShellArgsParser.Issue ... expecteds) { + assertArrayEquals(expecteds, issues.getAllIssues().toArray(new ShellArgsParser.Issue[issues.getAllIssues().size()])); + } }