changeset 1855:b39cc7254913

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 PR3051
author Andrew Azores <aazores@redhat.com>
date Fri, 24 Jun 2016 15:04:08 -0400
parents 5f61f05dff9e
children 85f97ddaa4b3
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	Tue May 03 13:42:39 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	Tue May 03 13:42:39 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());
     }
 }