Mercurial > hg > release > thermostat-1.6
changeset 1949:311aeb2c95aa
Fix bug where shell args parsing ignored final single-character arguments
Reviewed-by: jerboaa
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-June/019839.html
PR3050
author | Andrew Azores <aazores@redhat.com> |
---|---|
date | Fri, 24 Jun 2016 15:04:08 -0400 |
parents | 131195abd5fc |
children | 8ea5f5f57cd9 |
files | launcher/src/main/java/com/redhat/thermostat/launcher/internal/ShellArgsParser.java launcher/src/test/java/com/redhat/thermostat/launcher/internal/ShellArgsParserTest.java |
diffstat | 2 files changed, 59 insertions(+), 22 deletions(-) [+] |
line wrap: on
line diff
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ShellArgsParser.java Mon Jun 27 12:24:03 2016 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ShellArgsParser.java Fri Jun 24 15:04:08 2016 -0400 @@ -41,6 +41,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Objects; import static java.util.Objects.requireNonNull; @@ -74,14 +75,13 @@ readChar(); List<String> result = new ArrayList<>(); do { - if (isWhitespace()) { - whitespace(); - } else if (isQuote()) { + consumeWhitespace(); + if (isQuote()) { result.add(quote()); - } else { + } else if (isWord()) { result.add(word()); } - } while (!finished()); + } while (ready()); return result.toArray(new String[result.size()]); } @@ -92,7 +92,7 @@ return issues; } - private void whitespace() { + private void consumeWhitespace() { while (isWhitespace() && ready()) { readChar(); } @@ -158,10 +158,6 @@ return pos < input.length() - 1; } - private boolean finished() { - return pos == (input.length() - 1); - } - private char readChar() { c = input.charAt(++pos); return c; @@ -183,6 +179,10 @@ return Character.isWhitespace(c); } + private boolean isWord() { + return !(isQuote() || isWhitespace()); + } + static class Issues { private final List<Issue> issues = new ArrayList<>(); @@ -232,21 +232,21 @@ @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + 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; - + return columnNumber == issue.columnNumber && type == issue.type; } @Override public int hashCode() { - int result = columnNumber; - result = 31 * result + type.hashCode(); - return result; + return Objects.hash(columnNumber, type); } @Override
--- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ShellArgsParserTest.java Mon Jun 27 12:24:03 2016 -0400 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ShellArgsParserTest.java Fri Jun 24 15:04:08 2016 -0400 @@ -38,12 +38,13 @@ import org.junit.Test; +import java.util.Arrays; +import java.util.Collections; 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; @@ -370,6 +371,42 @@ } @Test + public void testSingleCharArgument() { + assertResult("some-command -f 2", "some-command", "-f", "2"); + assertNoIssues(); + } + + @Test + public void testSingleCharArgumentAdjoined() { + assertResult("some-command -f2", "some-command", "-f2"); + assertNoIssues(); + } + + @Test + public void testSingleCharArgumentWithSingleQuotes() { + assertResult("some-command -f '2'", "some-command", "-f", "2"); + assertNoIssues(); + } + + @Test + public void testSingleCharArgumentWithDoubleQuotes() { + assertResult("some-command -f \"2\"", "some-command", "-f", "2"); + assertNoIssues(); + } + + @Test + public void testSingleCharArgumentWithExtraWhitespace() { + assertResult("some-command -f 2", "some-command", "-f", "2"); + assertNoIssues(); + } + + @Test + public void testSingleCharArgumentWithTrailingWhitespace() { + assertResult("some-command -f 2 ", "some-command", "-f", "2"); + assertNoIssues(); + } + + @Test public void testIssueGetters() { ShellArgsParser.Issue issue = new ShellArgsParser.Issue(10, ShellArgsParser.Issue.Type.UNEXPECTED_QUOTE); int columnNumber = issue.getColumnNumber(); @@ -513,14 +550,14 @@ ShellArgsParser sap = new ShellArgsParser(input); String[] result = sap.parse(); issues = sap.getParseIssues(); - assertArrayEquals(new String[]{}, result); + assertEquals(Collections.emptyList(), Arrays.asList(result)); } private void assertResult(String input, String ... expecteds) { ShellArgsParser sap = new ShellArgsParser(input); String[] result = sap.parse(); issues = sap.getParseIssues(); - assertArrayEquals(expecteds, result); + assertEquals(Arrays.asList(expecteds), Arrays.asList(result)); } private void assertNoIssues() { @@ -528,6 +565,6 @@ } private void assertIssues(ShellArgsParser.Issue ... expecteds) { - assertArrayEquals(expecteds, issues.getAllIssues().toArray(new ShellArgsParser.Issue[issues.getAllIssues().size()])); + assertEquals(Arrays.asList(expecteds), issues.getAllIssues()); } }