Fixed commands with paths passed as String to process execution #155

vodorok · 2019-04-25T09:42:15Z

Depends on #145.

Fixed commands with paths passed as String to process execution

Modified every ShellExecutorHelper method signature. Now contains an
optional substitution map for adding Paths as File for proper escaping.
Extended test with test case that test Change directory into a directory
with spaces.

Added a test case that tests getting the checkers from various path
sequences. Look in CodeCheckEnvironmentCheckerTest.java for more info.
For this purpose, a CodeChecker stub was created that return some
checkers. This Resides in the Unit test's resources folder. Current
functionality only support getting the checkers.

Resolves #147

gyorb · 2019-04-26T09:27:30Z

...r.eclipse.plugin/src/org/codechecker/eclipse/plugin/runtime/CodeCheckEnvironmentChecker.java

-             " -j "+ getConfigValue(ConfigTypes.ANAL_THREADS) + " -n javarunner" +
-             " -o "+ getConfigValue(ConfigTypes.CHECKER_WORKSPACE)+"/results/ " + buildLog;
+    public String createAnalyzeCommmand(String buildLog) {
+        return "'" + Utils.esc(codeCheckerCommand) + "'" +


Wouldn't be easier to handle the command line options as a list of strings until the CommandLine type construction in the buildScriptCommandLine

There is a CommandLineParser which could be useful too.

Thanks @gyorb for the suggestion. This section was revised, and modified according to this https://commons.apache.org/proper/commons-exec/tutorial.html . The cli parser you mentioned was being used already but extended a bit.

gyorb · 2019-04-26T09:29:10Z

bundles/org.codechecker.eclipse.plugin/src/org/codechecker/eclipse/plugin/utils/Utils.java

+     * {@link https://unix.stackexchange.com/a/357932}
+     */
+    public static String esc(String path) {
+        return path.replace("'", "'\\''");


There are some string escaping methods could we use them? Should we escape other characters too?

This section was replaced with the path substitution map.

vodorok · 2019-07-01T11:33:02Z

...r.eclipse.plugin/src/org/codechecker/eclipse/plugin/runtime/CodeCheckEnvironmentChecker.java

@@ -1,34 +1,42 @@
 package org.codechecker.eclipse.plugin.runtime;


A large part of modification is related to this class. Paths in Shell command scripts got misinterpreted before. This was corrected with the usage of String - File Command substitution maps.

vodorok · 2019-07-01T11:35:11Z

...dechecker.eclipse.plugin/src/org/codechecker/eclipse/plugin/runtime/ShellExecutorHelper.java

@@ -3,15 +3,20 @@
 import com.google.common.base.Optional;


The other large part is in this class that actually makes the calls. Method signatures were changed to be able to use the command substitution maps.

...dechecker.eclipse.plugin/src/org/codechecker/eclipse/plugin/runtime/ShellExecutorHelper.java

vodorok · 2019-07-01T11:38:10Z

tests/org.codechecker.eclipse.rcp.shared/resources/CodeChecker/bin/CodeChecker

@@ -6,6 +6,17 @@ CodeChecker Stub used to emulate some CodeChecker commands for the tests.
 import sys
 import argparse

+"""


The dummy Codechecker got a new functionality. Now it can return some checkers.

vodorok · 2019-07-01T11:42:38Z

...p.unit.tests/src/org/codechecker/eclipse/plugin/runtime/CodeCheckEnvironmentCheckerTest.java

@@ -0,0 +1,97 @@
+package org.codechecker.eclipse.plugin.runtime;


Tests that the checkers are correctly returned.

vodorok · 2019-07-01T11:43:22Z

...lipse.rcp.unit.tests/src/org/codechecker/eclipse/plugin/runtime/ShellExecutorHelperTest.java

@@ -1,16 +1,18 @@
 package org.codechecker.eclipse.plugin.runtime;


Modified because the method signatures were changed.

gyorb

One comment only, otherwise LGTM.

...dechecker.eclipse.plugin/src/org/codechecker/eclipse/plugin/runtime/ShellExecutorHelper.java

Modified every ShellExecutorHelper method signature. Now contains an optional substitution map for adding Paths as File for proper escaping. Extended test with test case that test Change directory into a directory with spaces. Added a test case that tests getting the checkers from various path sequences. Look in CodeCheckEnvironmentCheckerTest.java for more info. For this purpose, a CodeChecker stub was created that return some checkers. This Resides in the Unit test's resources folder. Current functionality only support getting the checkers.

dkrupp

LGTM

vodorok requested a review from gyorb April 25, 2019 09:42

vodorok self-assigned this Apr 25, 2019

vodorok added the enhancement label Apr 25, 2019

vodorok added this to the v0.0.7 milestone Apr 25, 2019

vodorok added the bugfix label Apr 25, 2019

gyorb suggested changes Apr 26, 2019

View reviewed changes

vodorok force-pushed the pathSpecChar_patch branch from d42645f to 420316a Compare April 26, 2019 12:41

Ericsson deleted a comment Apr 26, 2019

Ericsson deleted a comment Jun 29, 2019

vodorok force-pushed the pathSpecChar_patch branch from d768ace to 09a0d9f Compare July 1, 2019 10:39

Ericsson deleted a comment Jul 1, 2019

vodorok force-pushed the pathSpecChar_patch branch from 09a0d9f to 3d15467 Compare July 1, 2019 11:19

Ericsson deleted a comment Jul 1, 2019

vodorok commented Jul 1, 2019

View reviewed changes

...dechecker.eclipse.plugin/src/org/codechecker/eclipse/plugin/runtime/ShellExecutorHelper.java Show resolved Hide resolved

vodorok commented Jul 1, 2019

View reviewed changes

vodorok removed the WIP label Jul 1, 2019

gyorb suggested changes Jul 5, 2019

View reviewed changes

...dechecker.eclipse.plugin/src/org/codechecker/eclipse/plugin/runtime/ShellExecutorHelper.java Show resolved Hide resolved

vodorok force-pushed the pathSpecChar_patch branch from 3d15467 to 5861f7e Compare July 25, 2019 09:31

Ericsson deleted a comment Jul 25, 2019

Ericsson deleted a comment from vodorok Jul 25, 2019

dkrupp approved these changes Aug 6, 2019

View reviewed changes

dkrupp merged commit bfac6b0 into Ericsson:master Aug 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixed commands with paths passed as String to process execution #155

Fixed commands with paths passed as String to process execution #155

Uh oh!

vodorok commented Apr 25, 2019 •

edited

Loading

Uh oh!

gyorb Apr 26, 2019

Uh oh!

vodorok Jul 1, 2019

Uh oh!

gyorb Apr 26, 2019

Uh oh!

vodorok Jul 1, 2019

Uh oh!

vodorok Jul 1, 2019

Uh oh!

vodorok Jul 1, 2019

Uh oh!

Uh oh!

vodorok Jul 1, 2019 •

edited

Loading

Uh oh!

vodorok Jul 1, 2019

Uh oh!

vodorok Jul 1, 2019

Uh oh!

gyorb left a comment

Uh oh!

Uh oh!

dkrupp left a comment

Uh oh!

Uh oh!

		@@ -1,34 +1,42 @@
		package org.codechecker.eclipse.plugin.runtime;

		@@ -0,0 +1,97 @@
		package org.codechecker.eclipse.plugin.runtime;

		@@ -1,16 +1,18 @@
		package org.codechecker.eclipse.plugin.runtime;

Fixed commands with paths passed as String to process execution #155

Fixed commands with paths passed as String to process execution #155

Uh oh!

Conversation

vodorok commented Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vodorok Jul 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gyorb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dkrupp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vodorok commented Apr 25, 2019 •

edited

Loading

vodorok Jul 1, 2019 •

edited

Loading