[x2cpg] Refactor ExternalCommand (#5017)

Works with Java Process / ProcessBuilder now. No more scala.sys.process. ----------- Co-authored-by: Michael Pollmeier <michael@michaelpollmeier.com>
joernio · Oct 22, 2024 · 398af04 · 398af04
1 parent 2cc61c8
commit 398af04
Show file tree

Hide file tree

Showing 20 changed files with 182 additions and 143 deletions.
diff --git a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/utils/ExternalCommand.scala b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/utils/ExternalCommand.scala
@@ -1,33 +1,24 @@
 package io.joern.c2cpg.utils
 
-import java.util.concurrent.ConcurrentLinkedQueue
-import scala.sys.process.{Process, ProcessLogger}
 import scala.util.{Failure, Success, Try}
-import scala.jdk.CollectionConverters.*
 
-object ExternalCommand extends io.joern.x2cpg.utils.ExternalCommand {
+object ExternalCommand {
 
-  override def handleRunResult(result: Try[Int], stdOut: Seq[String], stdErr: Seq[String]): Try[Seq[String]] = {
-    result match {
-      case Success(0) =>
+  import io.joern.x2cpg.utils.ExternalCommand.ExternalCommandResult
+
+  private val IsWin = scala.util.Properties.isWin
+
+  def run(command: Seq[String], cwd: String, extraEnv: Map[String, String] = Map.empty): Try[Seq[String]] = {
+    io.joern.x2cpg.utils.ExternalCommand.run(command, cwd, mergeStdErrInStdOut = true, extraEnv) match {
+      case ExternalCommandResult(0, stdOut, _) =>
         Success(stdOut)
-      case Success(1) if IsWin && IncludeAutoDiscovery.gccAvailable() =>
+      case ExternalCommandResult(1, stdOut, _) if IsWin && IncludeAutoDiscovery.gccAvailable() =>
         // the command to query the system header file locations within a Windows
         // environment always returns Success(1) for whatever reason...
         Success(stdOut)
-      case _ =>
+      case ExternalCommandResult(_, stdOut, _) =>
         Failure(new RuntimeException(stdOut.mkString(System.lineSeparator())))
     }
   }
 
-  override def run(command: String, cwd: String, extraEnv: Map[String, String] = Map.empty): Try[Seq[String]] = {
-    val stdOutOutput  = new ConcurrentLinkedQueue[String]
-    val processLogger = ProcessLogger(stdOutOutput.add, stdOutOutput.add)
-    val process = shellPrefix match {
-      case Nil => Process(command, new java.io.File(cwd), extraEnv.toList*)
-      case _   => Process(shellPrefix :+ command, new java.io.File(cwd), extraEnv.toList*)
-    }
-    handleRunResult(Try(process.!(processLogger)), stdOutOutput.asScala.toSeq, Nil)
-  }
-
 }
diff --git a/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/utils/IncludeAutoDiscovery.scala b/joern-cli/frontends/c2cpg/src/main/scala/io/joern/c2cpg/utils/IncludeAutoDiscovery.scala
@@ -13,13 +13,15 @@ object IncludeAutoDiscovery {
 
   private val IS_WIN = scala.util.Properties.isWin
 
-  val GCC_VERSION_COMMAND = "gcc --version"
+  val GCC_VERSION_COMMAND = Seq("gcc", "--version")
 
   private val CPP_INCLUDE_COMMAND =
-    if (IS_WIN) "gcc -xc++ -E -v . -o nul" else "gcc -xc++ -E -v /dev/null -o /dev/null"
+    if (IS_WIN) Seq("gcc", "-xc++", "-E", "-v", ".", "-o", "nul")
+    else Seq("gcc", "-xc++", "-E", "-v", "/dev/null", "-o", "/dev/null")
 
   private val C_INCLUDE_COMMAND =
-    if (IS_WIN) "gcc -xc -E -v . -o nul" else "gcc -xc -E -v /dev/null -o /dev/null"
+    if (IS_WIN) Seq("gcc", "-xc", "-E", "-v", ".", "-o", "nul")
+    else Seq("gcc", "-xc", "-E", "-v", "/dev/null", "-o", "/dev/null")
 
   // Only check once
   private var isGccAvailable: Option[Boolean] = None
@@ -57,7 +59,7 @@ object IncludeAutoDiscovery {
     output.slice(startIndex, endIndex).map(p => Paths.get(p.trim).toRealPath()).toSet
   }
 
-  private def discoverPaths(command: String): Set[Path] = ExternalCommand.run(command, ".") match {
+  private def discoverPaths(command: Seq[String]): Set[Path] = ExternalCommand.run(command, ".") match {
     case Success(output) => extractPaths(output)
     case Failure(exception) =>
       logger.warn(s"Unable to discover system include paths. Running '$command' failed.", exception)

diff --git a/...ntends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DotNetAstGenRunner.scala b/...ntends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DotNetAstGenRunner.scala
@@ -63,8 +63,8 @@ class DotNetAstGenRunner(config: Config) extends AstGenRunnerBase(config) {
   override def runAstGenNative(in: String, out: File, exclude: String, include: String)(implicit
     metaData: AstGenProgramMetaData
   ): Try[Seq[String]] = {
-    val excludeCommand = if (exclude.isEmpty) "" else s"-e \"$exclude\""
-    ExternalCommand.run(s"$astGenCommand -o ${out.toString()} -i \"$in\" $excludeCommand", ".")
+    val excludeCommand = if (exclude.isEmpty) Seq.empty else Seq("-e", exclude)
+    ExternalCommand.run(Seq(astGenCommand, "-o", out.toString(), "-i", in) ++ excludeCommand, ".").toTry
   }
 
 }
diff --git a/...ontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/passes/DownloadDependenciesPass.scala b/...ontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/passes/DownloadDependenciesPass.scala
@@ -27,13 +27,13 @@ class DownloadDependenciesPass(cpg: Cpg, parentGoMod: GoModHelper, goGlobal: GoG
       parentGoMod
         .getModMetaData()
         .foreach(mod => {
-          ExternalCommand.run("go mod init joern.io/temp", projDir) match {
+          ExternalCommand.run(Seq("go", "mod", "init", "joern.io/temp"), projDir).toTry match {
             case Success(_) =>
               mod.dependencies
                 .filter(dep => dep.beingUsed)
                 .map(dependency => {
-                  val cmd     = s"go get ${dependency.dependencyStr()}"
-                  val results = ExternalCommand.run(cmd, projDir)
+                  val cmd     = Seq("go", "get", dependency.dependencyStr())
+                  val results = ExternalCommand.run(cmd, projDir).toTry
                   results match {
                     case Success(_) =>
                       print(". ")

diff --git a/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/utils/AstGenRunner.scala b/joern-cli/frontends/gosrc2cpg/src/main/scala/io/joern/gosrc2cpg/utils/AstGenRunner.scala
@@ -75,16 +75,18 @@ class AstGenRunner(config: Config, includeFileRegex: String = "") extends AstGen
   override def runAstGenNative(in: String, out: File, exclude: String, include: String)(implicit
     metaData: AstGenProgramMetaData
   ): Try[Seq[String]] = {
-    val excludeCommand = if (exclude.isEmpty) "" else s"-exclude \"$exclude\""
-    val includeCommand = if (include.isEmpty) "" else s"-include-packages \"$include\""
-    ExternalCommand.run(s"$astGenCommand $excludeCommand $includeCommand -out ${out.toString()} $in", ".")
+    val excludeCommand = if (exclude.isEmpty) Seq.empty else Seq("-exclude", exclude)
+    val includeCommand = if (include.isEmpty) Seq.empty else Seq("-include-packages", include)
+    ExternalCommand
+      .run((astGenCommand +: excludeCommand) ++ includeCommand ++ Seq("-out", out.toString(), in), ".")
+      .toTry
   }
 
   def executeForGo(out: File): List[GoAstGenRunnerResult] = {
     implicit val metaData: AstGenProgramMetaData = config.astGenMetaData
     val in                                       = File(config.inputPath)
     logger.info(s"Running goastgen in '$config.inputPath' ...")
-    runAstGenNative(config.inputPath, out, config.ignoredFilesRegex.toString(), includeFileRegex.toString()) match {
+    runAstGenNative(config.inputPath, out, config.ignoredFilesRegex.toString(), includeFileRegex) match {
       case Success(result) =>
         val srcFiles = SourceFiles.determine(
           out.toString(),
@@ -114,19 +116,19 @@ class AstGenRunner(config: Config, includeFileRegex: String = "") extends AstGen
   ): List[GoAstGenRunnerResult] = {
     val moduleMeta: ModuleMeta =
       ModuleMeta(inputPath, outPath, None, ListBuffer[String](), ListBuffer[String](), ListBuffer[ModuleMeta]())
-    if (parsedModFiles.size > 0) {
+    if (parsedModFiles.nonEmpty) {
       parsedModFiles
         .sortBy(_.split(UtilityConstants.fileSeparateorPattern).length)
         .foreach(modFile => {
           moduleMeta.addModFile(modFile, inputPath, outPath)
         })
       parsedFiles.foreach(moduleMeta.addParsedFile)
       skippedFiles.foreach(moduleMeta.addSkippedFile)
-      moduleMeta.getOnlyChilds()
+      moduleMeta.getOnlyChildren
     } else {
       parsedFiles.foreach(moduleMeta.addParsedFile)
       skippedFiles.foreach(moduleMeta.addSkippedFile)
-      moduleMeta.getAllChilds()
+      moduleMeta.getAllChildren
     }
   }
 
@@ -184,12 +186,12 @@ class AstGenRunner(config: Config, includeFileRegex: String = "") extends AstGen
       }
     }
 
-    def getOnlyChilds(): List[GoAstGenRunnerResult] = {
-      childModules.flatMap(_.getAllChilds()).toList
+    def getOnlyChildren: List[GoAstGenRunnerResult] = {
+      childModules.flatMap(_.getAllChildren).toList
     }
 
-    def getAllChilds(): List[GoAstGenRunnerResult] = {
-      getOnlyChilds() ++ List(
+    def getAllChildren: List[GoAstGenRunnerResult] = {
+      getOnlyChildren ++ List(
         GoAstGenRunnerResult(
           modulePath = modulePath,
           parsedModFile = modFilePath,

diff --git a/joern-cli/frontends/javasrc2cpg/src/main/scala/io/joern/javasrc2cpg/util/Delombok.scala b/joern-cli/frontends/javasrc2cpg/src/main/scala/io/joern/javasrc2cpg/util/Delombok.scala
@@ -1,14 +1,14 @@
 package io.joern.javasrc2cpg.util
 
 import better.files.File
-import io.joern.x2cpg.utils.ExternalCommand
 import io.joern.javasrc2cpg.util.Delombok.DelombokMode.*
+import io.joern.x2cpg.utils.ExternalCommand
 import org.slf4j.LoggerFactory
 
-import java.nio.file.{Path, Paths}
-import scala.collection.mutable
-import scala.util.matching.Regex
-import scala.util.{Failure, Success, Try}
+import java.nio.file.Path
+import scala.util.Failure
+import scala.util.Success
+import scala.util.Try
 
 object Delombok {
 
@@ -53,8 +53,17 @@ object Delombok {
         System.getProperty("java.class.path")
     }
     val command =
-      s"$javaPath -cp $classPathArg lombok.launch.Main delombok ${inputPath.toAbsolutePath.toString} -d ${outputDir.canonicalPath}"
-    logger.debug(s"Executing delombok with command $command")
+      Seq(
+        javaPath,
+        "-cp",
+        classPathArg,
+        "lombok.launch.Main",
+        "delombok",
+        inputPath.toAbsolutePath.toString,
+        "-d",
+        outputDir.canonicalPath
+      )
+    logger.debug(s"Executing delombok with command ${command.mkString(" ")}")
     command
   }
 
@@ -72,6 +81,7 @@ object Delombok {
     Try(delombokTempDir.createChild(relativeOutputPath, asDirectory = true)).flatMap { packageOutputDir =>
       ExternalCommand
         .run(delombokToTempDirCommand(inputDir, packageOutputDir, analysisJavaHome), ".")
+        .toTry
         .map(_ => delombokTempDir.path.toAbsolutePath.toString)
     }
   }

diff --git a/joern-cli/frontends/jssrc2cpg/src/main/scala/io/joern/jssrc2cpg/utils/AstGenRunner.scala b/joern-cli/frontends/jssrc2cpg/src/main/scala/io/joern/jssrc2cpg/utils/AstGenRunner.scala
@@ -126,7 +126,7 @@ object AstGenRunner {
     val astGenCommand = path.getOrElse("astgen")
     val localPath     = path.flatMap(File(_).parentOption.map(_.pathAsString)).getOrElse(".")
     val debugMsgPath  = path.getOrElse("PATH")
-    ExternalCommand.run(s"$astGenCommand --version", localPath).toOption.map(_.mkString.strip()) match {
+    ExternalCommand.run(Seq(astGenCommand, "--version"), localPath).successOption.map(_.mkString.strip()) match {
       case Some(installedVersion)
           if installedVersion != "unknown" &&
             Try(VersionHelper.compare(installedVersion, astGenVersion)).toOption.getOrElse(-1) >= 0 =>
@@ -175,7 +175,7 @@ class AstGenRunner(config: Config) {
 
   import io.joern.jssrc2cpg.utils.AstGenRunner._
 
-  private val executableArgs = if (!config.tsTypes) " --no-tsTypes" else ""
+  private val executableArgs = if (!config.tsTypes) Seq("--no-tsTypes") else Seq.empty
 
   private def skippedFiles(astGenOut: List[String]): List[String] = {
     val skipped = astGenOut.collect {
@@ -297,7 +297,11 @@ class AstGenRunner(config: Config) {
     }
 
     val result =
-      ExternalCommand.run(s"$astGenCommand$executableArgs -t ts -o $out", out.toString(), extraEnv = NODE_OPTIONS)
+      ExternalCommand.run(
+        (astGenCommand +: executableArgs) ++ Seq("-t", "ts", "-o", out.toString),
+        out.toString(),
+        extraEnv = NODE_OPTIONS
+      )
 
     val jsons = SourceFiles.determine(out.toString(), Set(".json"))
     jsons.foreach { jsonPath =>
@@ -312,7 +316,7 @@ class AstGenRunner(config: Config) {
     }
 
     tmpJsFiles.foreach(_.delete())
-    result
+    result.toTry
   }
 
   private def ejsFiles(in: File, out: File): Try[Seq[String]] = {
@@ -337,12 +341,24 @@ class AstGenRunner(config: Config) {
       ignoredFilesPath = Some(config.ignoredFiles)
     )
     if (files.nonEmpty)
-      ExternalCommand.run(s"$astGenCommand$executableArgs -t vue -o $out", in.toString(), extraEnv = NODE_OPTIONS)
+      ExternalCommand
+        .run(
+          (astGenCommand +: executableArgs) ++ Seq("-t", "vue", "-o", out.toString),
+          in.toString(),
+          extraEnv = NODE_OPTIONS
+        )
+        .toTry
     else Success(Seq.empty)
   }
 
   private def jsFiles(in: File, out: File): Try[Seq[String]] =
-    ExternalCommand.run(s"$astGenCommand$executableArgs -t ts -o $out", in.toString(), extraEnv = NODE_OPTIONS)
+    ExternalCommand
+      .run(
+        (astGenCommand +: executableArgs) ++ Seq("-t", "ts", "-o", out.toString),
+        in.toString(),
+        extraEnv = NODE_OPTIONS
+      )
+      .toTry
 
   private def runAstGenNative(in: File, out: File): Try[Seq[String]] = for {
     ejsResult <- ejsFiles(in, out)

diff --git a/...i/frontends/kotlin2cpg/src/test/scala/io/joern/kotlin2cpg/compiler/CompilerAPITests.scala b/...i/frontends/kotlin2cpg/src/test/scala/io/joern/kotlin2cpg/compiler/CompilerAPITests.scala
@@ -77,8 +77,9 @@ class CompilerAPITests extends AnyFreeSpec with Matchers {
 
     "should not contain methods with unresolved types/namespaces" in {
       val command =
-        if (scala.util.Properties.isWin) "cmd.exe /C gradlew.bat gatherDependencies" else "./gradlew gatherDependencies"
-      ExternalCommand.run(command, projectDirPath) shouldBe Symbol("success")
+        if (scala.util.Properties.isWin) Seq("cmd.exe", "/C", "gradlew.bat", "gatherDependencies")
+        else Seq("./gradlew", "gatherDependencies")
+      ExternalCommand.run(command, projectDirPath).toTry shouldBe Symbol("success")
       val config = Config(classpath = Set(projectDependenciesPath.toString))
       val cpg = new Kotlin2Cpg().createCpg(projectDirPath)(config).getOrElse {
         fail("Could not create a CPG!")

diff --git a/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/Php2Cpg.scala b/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/Php2Cpg.scala
@@ -22,7 +22,7 @@ class Php2Cpg extends X2CpgFrontend[Config] {
   private val PhpVersionRegex = new Regex("^PHP ([78]\\.[1-9]\\.[0-9]|[9-9]\\d\\.\\d\\.\\d)")
 
   private def isPhpVersionSupported: Boolean = {
-    val result = ExternalCommand.run("php --version", ".")
+    val result = ExternalCommand.run(Seq("php", "--version"), ".").toTry
     result match {
       case Success(listString) =>
         val phpVersionStr = listString.headOption.getOrElse("")

diff --git a/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/parser/ClassParser.scala b/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/parser/ClassParser.scala
@@ -3,7 +3,6 @@ import better.files.File
 import io.joern.x2cpg.utils.ExternalCommand
 import org.slf4j.LoggerFactory
 
-import scala.collection.immutable.LazyList.from
 import scala.io.Source
 import scala.util.{Failure, Success, Try, Using}
 import upickle.default.*
@@ -26,11 +25,12 @@ class ClassParser(targetDir: File) {
     f
   }
 
-  private lazy val phpClassParseCommand: String = s"php ${classParserScript.pathAsString} ${targetDir.pathAsString}"
+  private lazy val phpClassParseCommand: Seq[String] =
+    Seq("php", classParserScript.pathAsString, targetDir.pathAsString)
 
   def parse(): Try[List[ClassParserClass]] = Try {
     val inputDirectory = targetDir.parent.canonicalPath
-    ExternalCommand.run(phpClassParseCommand, inputDirectory).map(_.reverse) match {
+    ExternalCommand.run(phpClassParseCommand, inputDirectory).toTry.map(_.reverse) match {
       case Success(output) =>
         read[List[ClassParserClass]](output.mkString("\n"))
       case Failure(exception) =>

diff --git a/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/parser/PhpParser.scala b/joern-cli/frontends/php2cpg/src/main/scala/io/joern/php2cpg/parser/PhpParser.scala
@@ -17,10 +17,9 @@ class PhpParser private (phpParserPath: String, phpIniPath: String, disableFileC
 
   private val logger = LoggerFactory.getLogger(this.getClass)
 
-  private def phpParseCommand(filenames: collection.Seq[String]): String = {
-    val phpParserCommands = "--with-recovery --resolve-names --json-dump"
-    val filenamesString   = filenames.mkString(" ")
-    s"php --php-ini $phpIniPath $phpParserPath $phpParserCommands $filenamesString"
+  private def phpParseCommand(filenames: collection.Seq[String]): Seq[String] = {
+    val phpParserCommands = Seq("--with-recovery", "--resolve-names", "--json-dump")
+    Seq("php", "--php-ini", phpIniPath, phpParserPath) ++ phpParserCommands ++ filenames
   }
 
   def parseFiles(inputPaths: collection.Seq[String]): collection.Seq[(String, Option[PhpFile], String)] = {
@@ -37,19 +36,19 @@ class PhpParser private (phpParserPath: String, phpIniPath: String, disableFileC
 
     val command = phpParseCommand(inputPaths)
 
-    val (returnValue, output) = ExternalCommand.runWithMergeStdoutAndStderr(command, ".")
-    returnValue match {
-      case 0 =>
-        val asJson = linesToJsonValues(output.lines().toArray(size => new Array[String](size)))
+    val result = ExternalCommand.run(command, ".", mergeStdErrInStdOut = true)
+    result match {
+      case ExternalCommand.ExternalCommandResult(0, stdOut, _) =>
+        val asJson = linesToJsonValues(stdOut)
         val asPhpFile = asJson.map { case (filename, jsonObjectOption, infoLines) =>
           (filename, jsonToPhpFile(jsonObjectOption, filename), infoLines)
         }
         val withRemappedFileName = asPhpFile.map { case (filename, phpFileOption, infoLines) =>
           (canonicalToInputPath.apply(filename), phpFileOption, infoLines)
         }
         withRemappedFileName
-      case exitCode =>
-        logger.error(s"Failure running php-parser with $command, exit code $exitCode")
+      case ExternalCommand.ExternalCommandResult(exitCode, _, _) =>
+        logger.error(s"Failure running php-parser with ${command.mkString(" ")}, exit code $exitCode")
         Nil
     }
   }

diff --git a/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/RubySrc2Cpg.scala b/joern-cli/frontends/rubysrc2cpg/src/main/scala/io/joern/rubysrc2cpg/RubySrc2Cpg.scala
@@ -91,14 +91,13 @@ class RubySrc2Cpg extends X2CpgFrontend[Config] {
 
   private def downloadDependency(inputPath: String, tempPath: String): Unit = {
     if (Files.isRegularFile(Paths.get(s"${inputPath}${java.io.File.separator}Gemfile"))) {
-      ExternalCommand.run(s"bundle config set --local path ${tempPath}", inputPath) match {
+      ExternalCommand.run(Seq("bundle", "config", "set", "--local", "path", tempPath), inputPath).toTry match {
         case Success(configOutput) =>
           logger.info(s"Gem config successfully done: $configOutput")
         case Failure(exception) =>
           logger.error(s"Error while configuring Gem Path: ${exception.getMessage}")
       }
-      val command = s"bundle install"
-      ExternalCommand.run(command, inputPath) match {
+      ExternalCommand.run(Seq("bundle", "install"), inputPath).toTry match {
         case Success(bundleOutput) =>
           logger.info(s"Dependency installed successfully: $bundleOutput")
         case Failure(exception) =>

diff --git a/...-cli/frontends/swiftsrc2cpg/src/main/scala/io/joern/swiftsrc2cpg/utils/AstGenRunner.scala b/...-cli/frontends/swiftsrc2cpg/src/main/scala/io/joern/swiftsrc2cpg/utils/AstGenRunner.scala
@@ -65,7 +65,7 @@ object AstGenRunner {
     val astGenCommand = path.getOrElse("SwiftAstGen")
     val localPath     = path.flatMap(File(_).parentOption.map(_.pathAsString)).getOrElse(".")
     val debugMsgPath  = path.getOrElse("PATH")
-    ExternalCommand.run(s"$astGenCommand -h", localPath).toOption match {
+    ExternalCommand.run(Seq(astGenCommand, "-h"), localPath).toOption match {
       case Some(_) =>
         logger.debug(s"Using SwiftAstGen from $debugMsgPath")
         true
@@ -140,7 +140,7 @@ class AstGenRunner(config: Config) {
   }
 
   private def runAstGenNative(in: File, out: File): Try[Seq[String]] =
-    ExternalCommand.run(s"$astGenCommand -o $out", in.toString())
+    ExternalCommand.run(Seq(astGenCommand, "-o", out.toString), in.toString())
 
   private def checkParsedFiles(files: List[String], in: File): List[String] = {
     val numOfParsedFiles = files.size