From eb8d110e52af33dd839b15439861ef040c091f04 Mon Sep 17 00:00:00 2001 From: Karan Batavia <118820668+karan-batavia@users.noreply.github.com> Date: Thu, 28 Nov 2024 21:59:38 +0530 Subject: [PATCH] Handle Dependency Download and DotNetAstGen errors gracefully. (#151) * handle errors gracefully * use scala best practices * trim dependency names and versions * test for asserting if version is trimmed --- .../csharpsrc2cpg/passes/DependencyPass.scala | 4 +- .../utils/DependencyDownloader.scala | 96 ++++++++++--------- .../utils/DotNetAstGenRunner.scala | 13 ++- .../passes/DependencyTests.scala | 28 +++++- 4 files changed, 89 insertions(+), 52 deletions(-) diff --git a/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/passes/DependencyPass.scala b/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/passes/DependencyPass.scala index 7614d4b58cac..db4d3a576947 100644 --- a/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/passes/DependencyPass.scala +++ b/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/passes/DependencyPass.scala @@ -45,8 +45,8 @@ class DependencyPass(cpg: Cpg, buildFiles: List[String], registerPackageId: Stri } val packageVersion = packageReference.attribute("Version").map(_.toString()).getOrElse("") val dependencyNode = NewDependency() - .name(packageName) - .version(packageVersion) + .name(packageName.trim()) + .version(packageVersion.trim()) builder.addNode(dependencyNode) } match { case Failure(exception) => diff --git a/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DependencyDownloader.scala b/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DependencyDownloader.scala index 3316a704647e..580da7a5beab 100644 --- a/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DependencyDownloader.scala +++ b/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DependencyDownloader.scala @@ -88,8 +88,13 @@ class DependencyDownloader( case Success(x) => x } - def createUrl(packageType: String, version: String): URL = { - URI(s"https://$NUGET_BASE_API_V2/$packageType/${dependencyName}/$version").toURL + def createUrl(packageType: String, version: String): Option[URL] = { + Try(new URI(s"https://$NUGET_BASE_API_V2/$packageType/${dependencyName}/$version").toURL) match { + case Success(url) => Some(url) + case Failure(e) => + logger.debug(s"Failed to create URL for packageType: $packageType, version: $version. Error: ${e.getMessage}") + None + } } // If dependency version is not specified, latest is returned @@ -115,50 +120,54 @@ class DependencyDownloader( * @return * the package version. */ - private def downloadPackage(targetDir: File, dependency: Dependency, url: URL): Unit = { + private def downloadPackage(targetDir: File, dependency: Dependency, url: Option[URL]): Unit = { var connection: Option[HttpURLConnection] = None - try { - connection = Option(url.openConnection()).collect { case x: HttpURLConnection => x } - // allow both GZip and Deflate (ZLib) encodings - connection.foreach(_.setRequestProperty("Accept-Encoding", "gzip, deflate")) - connection match { - case Some(conn: HttpURLConnection) if conn.getResponseCode == HttpURLConnection.HTTP_OK => - val ext = if url.toString.contains("/package/") then "nupkg" else "snupkg" - val fileName = targetDir / s"${dependency.name}.$ext" - - val inputStream = Option(conn.getContentEncoding) match { - case Some(encoding) if encoding.equalsIgnoreCase("gzip") => GZIPInputStream(conn.getInputStream) - case Some(encoding) if encoding.equalsIgnoreCase("deflate") => InflaterInputStream(conn.getInputStream) - case _ => conn.getInputStream - } - - Try { - Using.resources(inputStream, new FileOutputStream(fileName.pathAsString)) { (is, fos) => - val buffer = new Array[Byte](4096) - Iterator - .continually(is.read(buffer)) - .takeWhile(_ != -1) - .foreach(bytesRead => fos.write(buffer, 0, bytesRead)) - } - } match { - case Failure(exception) => - logger.error( - s"Exception occurred while downloading $fileName (${dependency.name}:${dependency.version})", - exception - ) - case Success(_) => - logger.info(s"Successfully downloaded dependency ${dependency.name}:${dependency.version}") + url.foreach { validUrl => + { + try { + connection = Option(validUrl.openConnection()).collect { case x: HttpURLConnection => x } + // allow both GZip and Deflate (ZLib) encodings + connection.foreach(_.setRequestProperty("Accept-Encoding", "gzip, deflate")) + connection match { + case Some(conn: HttpURLConnection) if conn.getResponseCode == HttpURLConnection.HTTP_OK => + val ext = if url.toString.contains("/package/") then "nupkg" else "snupkg" + val fileName = targetDir / s"${dependency.name}.$ext" + + val inputStream = Option(conn.getContentEncoding) match { + case Some(encoding) if encoding.equalsIgnoreCase("gzip") => GZIPInputStream(conn.getInputStream) + case Some(encoding) if encoding.equalsIgnoreCase("deflate") => InflaterInputStream(conn.getInputStream) + case _ => conn.getInputStream + } + + Try { + Using.resources(inputStream, new FileOutputStream(fileName.pathAsString)) { (is, fos) => + val buffer = new Array[Byte](4096) + Iterator + .continually(is.read(buffer)) + .takeWhile(_ != -1) + .foreach(bytesRead => fos.write(buffer, 0, bytesRead)) + } + } match { + case Failure(exception) => + logger.error( + s"Exception occurred while downloading $fileName (${dependency.name}:${dependency.version})", + exception + ) + case Success(_) => + logger.info(s"Successfully downloaded dependency ${dependency.name}:${dependency.version}") + } + case Some(conn: HttpURLConnection) => + logger.error(s"Connection to $url responded with non-200 code ${conn.getResponseCode}") + case _ => + logger.error(s"Unknown URL connection made, aborting") } - case Some(conn: HttpURLConnection) => - logger.error(s"Connection to $url responded with non-200 code ${conn.getResponseCode}") - case _ => - logger.error(s"Unknown URL connection made, aborting") + } catch { + case exception: Throwable => + logger.error(s"Unable to download dependency ${dependency.name}:${dependency.version}", exception) + } finally { + connection.foreach(_.disconnect()) + } } - } catch { - case exception: Throwable => - logger.error(s"Unable to download dependency ${dependency.name}:${dependency.version}", exception) - } finally { - connection.foreach(_.disconnect()) } } @@ -217,5 +226,4 @@ class DependencyDownloader( .map(CSharpProgramSummary(_)) CSharpProgramSummary(summaries) } - } diff --git a/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DotNetAstGenRunner.scala b/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DotNetAstGenRunner.scala index 5d05dbaa9a00..7e0cdffd137c 100644 --- a/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DotNetAstGenRunner.scala +++ b/joern-cli/frontends/csharpsrc2cpg/src/main/scala/io/joern/csharpsrc2cpg/utils/DotNetAstGenRunner.scala @@ -30,11 +30,14 @@ class DotNetAstGenRunner(config: Config) extends AstGenRunnerBase(config) { override def skippedFiles(in: File, astGenOut: List[String]): List[String] = { val diagnosticMap = mutable.LinkedHashMap.empty[String, Seq[String]] - def addReason(reason: String, lastFile: Option[String] = None) = { - val key = lastFile.getOrElse(diagnosticMap.last._1) - diagnosticMap.updateWith(key) { - case Some(x) => Option(x :+ reason) - case None => Option(reason :: Nil) + def addReason(reason: String, lastFile: Option[String] = None): Unit = { + val key = lastFile.orElse(diagnosticMap.lastOption.map(_._1)) + + key.foreach { resolvedKey => + diagnosticMap.updateWith(resolvedKey) { + case Some(existingReasons) => Some(existingReasons :+ reason) + case None => Some(List(reason)) + } } } diff --git a/joern-cli/frontends/csharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/passes/DependencyTests.scala b/joern-cli/frontends/csharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/passes/DependencyTests.scala index 488baaa5a695..c7536e535458 100644 --- a/joern-cli/frontends/csharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/passes/DependencyTests.scala +++ b/joern-cli/frontends/csharpsrc2cpg/src/test/scala/io/joern/csharpsrc2cpg/passes/DependencyTests.scala @@ -160,9 +160,35 @@ class DependencyTests extends CSharpCode2CpgFixture { .withConfig(config) inside(cpg.dependency.l) { case dep :: Nil => - dep.name shouldBe " System.Security.Cryptography.Pkcs" + dep.name shouldBe "System.Security.Cryptography.Pkcs" dep.version shouldBe "6.0.4" } } } + + "a csproj file specifying a dependency with whitespaces in between" should { + val config = Config().withDownloadDependencies(true); + "not throw a exception" in { + val cpg = code(""" + |namespace Foo; + |""".stripMargin) + .moreCode( + """ + | + | + | + | + | + | + |""".stripMargin, + "Foo.csproj" + ) + .withConfig(config) + + inside(cpg.dependency.l) { case dep :: dep2 :: Nil => + dep.name shouldBe "System.Security.Cryptography.Pkcs" + dep.version shouldBe "6 .0.4" + } + } + } }