maven-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MSHARED-749) Commandline does not thrown CommandLineException when uneven number of quotation marks used
Date Sun, 29 Jul 2018 12:32:00 GMT

    [ https://issues.apache.org/jira/browse/MSHARED-749?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16561110#comment-16561110
] 

ASF GitHub Bot commented on MSHARED-749:
----------------------------------------

khmarbaise closed pull request #5: [MSHARED-749] - Commandline does not thrown CommandLineException
when…
URL: https://github.com/apache/maven-shared-utils/pull/5
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/src/main/java/org/apache/maven/shared/utils/cli/Arg.java b/src/main/java/org/apache/maven/shared/utils/cli/Arg.java
index bd1fa08..e0eccf3 100644
--- a/src/main/java/org/apache/maven/shared/utils/cli/Arg.java
+++ b/src/main/java/org/apache/maven/shared/utils/cli/Arg.java
@@ -34,7 +34,7 @@
     /**
      * @param line The line of arguments.
      */
-    void setLine( String line );
+    void setLine( String line ) throws CommandLineException;
 
     /**
      * @param value The file to be set.
diff --git a/src/main/java/org/apache/maven/shared/utils/cli/Commandline.java b/src/main/java/org/apache/maven/shared/utils/cli/Commandline.java
index 94fbdcc..db13770 100644
--- a/src/main/java/org/apache/maven/shared/utils/cli/Commandline.java
+++ b/src/main/java/org/apache/maven/shared/utils/cli/Commandline.java
@@ -92,19 +92,11 @@ public Commandline( Shell shell )
      *
      * @param toProcess The command to process
      */
-    public Commandline( String toProcess )
+    public Commandline( String toProcess ) throws CommandLineException
     {
         setDefaultShell();
-        String[] tmp = new String[0];
-        try
-        {
-            tmp = CommandLineUtils.translateCommandline( toProcess );
-        }
-        catch ( Exception e )
-        {
-            System.err.println( "Error translating Commandline." );
-        }
-        if ( ( tmp != null ) && ( tmp.length > 0 ) )
+        String[] tmp = CommandLineUtils.translateCommandline( toProcess );
+        if ( ( tmp.length > 0 ) )
         {
             setExecutable( tmp[0] );
             for ( int i = 1; i < tmp.length; i++ )
@@ -484,7 +476,7 @@ public void setValue( String value )
         /**
          * {@inheritDoc}
          */
-        public void setLine( String line )
+        public void setLine( String line ) throws CommandLineException
         {
             if ( line == null )
             {
@@ -494,9 +486,10 @@ public void setLine( String line )
             {
                 parts = CommandLineUtils.translateCommandline( line );
             }
-            catch ( Exception e )
+            catch ( CommandLineException e )
             {
                 System.err.println( "Error translating Commandline." );
+                throw( e );
             }
         }
 
diff --git a/src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java b/src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
index d64fdd8..5b5ec2a 100644
--- a/src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
+++ b/src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
@@ -19,6 +19,7 @@
  * under the License.
  */
 
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -101,6 +102,31 @@ public void testTranslateCommandline()
     }
 
 
+    @Test
+    public void givenASingleQuoteMarkInArgument_whenExecutingCode_thenNoExceptionIsThrown()
throws Exception {
+        new Commandline("echo \"let's go\"").execute();
+    }
+
+    @Test
+    public void givenADoubleQuoteMarkInArgument_whenExecutingCode_thenCommandLineExceptionIsThrown()
throws Exception {
+        try {
+            new Commandline("echo \"let\"s go\"").execute();
+        } catch (CommandLineException e) {
+            assertTrue(true);
+            return;
+        }
+        fail("Exception was not thrown when given invalid (3 unescaped double quote) input");
+    }
+
+
+    @Test
+    public void givenASingleQuoteMarkInArgument_whenExecutingCode_thenExitCode0Returned()
throws Exception {
+        final Process p = new Commandline("echo \"let's go\"").execute();
+        // Note, this sleep should be removed when java version reaches Java 8
+        Thread.sleep(1000);
+        assertEquals(0, p.exitValue());
+    }
+
     @Test
     public void givenASingleQuoteMarkInArgument_whenTranslatingToCmdLineArgs_thenTheQuotationMarkIsNotEscaped()
throws Exception
     {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Commandline does not thrown CommandLineException when uneven number of quotation marks
used
> -------------------------------------------------------------------------------------------
>
>                 Key: MSHARED-749
>                 URL: https://issues.apache.org/jira/browse/MSHARED-749
>             Project: Maven Shared Components
>          Issue Type: Bug
>    Affects Versions: maven-shared-utils-3.2.1
>            Reporter: Kathryn Newbould
>            Assignee: Karl Heinz Marbaise
>            Priority: Major
>             Fix For: maven-shared-utils-3.3.0
>
>
> The merged fix for MSHARED-416 did not fully fix the issue as the process does not return
non-0 on failure (which is what the tests were based on).
> As an example:
>  
> {code:java}
> @Test
> public void MSHARED_416 () throws Exception {
>     Process p = new Commandline("\"Hi\" Friends\"").execute();
>     Thread.sleep(1000);
>     assertNotEquals(0, p.exitValue());
> }
> {code}
> will fail without exception (e.g. return 0):
> {code:java}
> java.lang.AssertionError: Values should be different. Actual: 0
> {code}
> Even though the logs state (implying the Exception is being thrown)
> {code:java}
> Error translating Commandline.
> {code}
> Suggested fix:
>  
> As the Exception is thrown in the constructor, it can be difficult to catch it, so I
suggest throwing it up to the process runner (executeCommandLineAsCallable) can handle it
appropriately.
> However, I'm unsure of the implications.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message