Repository: incubator-toree Updated Branches: refs/heads/master f20a9f6ab -> ab095e1b0
[TOREE-486] Raise exception when %AddJar called with invalid jar Closes #164 Project: http://git-wip-us.apache.org/repos/asf/incubator-toree/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-toree/commit/ab095e1b Tree: http://git-wip-us.apache.org/repos/asf/incubator-toree/tree/ab095e1b Diff: http://git-wip-us.apache.org/repos/asf/incubator-toree/diff/ab095e1b Branch: refs/heads/master Commit: ab095e1b085c642aa24d8bcfdaa77d87f276f83d Parents: f20a9f6 Author: Luciano Resende <lrese...@apache.org> Authored: Mon Sep 24 22:29:26 2018 -0400 Committer: Luciano Resende <lrese...@apache.org> Committed: Tue Oct 2 10:04:22 2018 -0700 ---------------------------------------------------------------------- .../org/apache/toree/magic/builtin/AddJar.scala | 23 ++++++++ .../apache/toree/magic/builtin/AddJarSpec.scala | 58 ++++++++------------ 2 files changed, 47 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-toree/blob/ab095e1b/kernel/src/main/scala/org/apache/toree/magic/builtin/AddJar.scala ---------------------------------------------------------------------- diff --git a/kernel/src/main/scala/org/apache/toree/magic/builtin/AddJar.scala b/kernel/src/main/scala/org/apache/toree/magic/builtin/AddJar.scala index ef5e927..f445ade 100644 --- a/kernel/src/main/scala/org/apache/toree/magic/builtin/AddJar.scala +++ b/kernel/src/main/scala/org/apache/toree/magic/builtin/AddJar.scala @@ -20,6 +20,7 @@ package org.apache.toree.magic.builtin import java.io.{File, PrintStream} import java.net.{URL, URI} import java.nio.file.{Files, Paths} +import java.util.zip.ZipFile import org.apache.toree.magic._ import org.apache.toree.magic.builtin.AddJar._ import org.apache.toree.magic.dependencies._ @@ -64,6 +65,23 @@ class AddJar private def printStream = new PrintStream(outputStream) /** + * Validate jar file structure + * + * @param jarFile + * @return boolean value based on validity of jar + */ + def isValidJar(jarFile: File): Boolean = { + try { + val jarZip: ZipFile = new ZipFile(jarFile) + val entries = jarZip.entries + if (entries.hasMoreElements) return true else return false + } catch { + case _: Throwable => return false + } + } + + + /** * Retrieves file name from a URI. * * @param location a URI @@ -151,6 +169,11 @@ class AddJar printStream.println(s"Using cached version of $jarName") } + // validate jar file + if(! isValidJar(fileDownloadLocation)) { + throw new IllegalArgumentException(s"Jar '$jarName' is not valid.") + } + if (_magic) { val plugins = pluginManager.loadPlugins(fileDownloadLocation) pluginManager.initializePlugins(plugins) http://git-wip-us.apache.org/repos/asf/incubator-toree/blob/ab095e1b/kernel/src/test/scala/org/apache/toree/magic/builtin/AddJarSpec.scala ---------------------------------------------------------------------- diff --git a/kernel/src/test/scala/org/apache/toree/magic/builtin/AddJarSpec.scala b/kernel/src/test/scala/org/apache/toree/magic/builtin/AddJarSpec.scala index 8d1f44b..5661250 100644 --- a/kernel/src/test/scala/org/apache/toree/magic/builtin/AddJarSpec.scala +++ b/kernel/src/test/scala/org/apache/toree/magic/builtin/AddJarSpec.scala @@ -17,9 +17,10 @@ package org.apache.toree.magic.builtin -import java.io.OutputStream +import java.io.{File, OutputStream} import java.net.{URI, URL} -import java.nio.file.{Files, FileSystems} +import java.nio.file.{FileSystems, Files} + import org.apache.toree.interpreter.Interpreter import org.apache.toree.magic.dependencies.{IncludeConfig, IncludeInterpreter, IncludeKernel, IncludeOutputStream} import com.typesafe.config.ConfigFactory @@ -49,11 +50,9 @@ class AddJarSpec extends FunSpec with Matchers with MockitoSugar { override val outputStream: OutputStream = mockOutputStream override lazy val pluginManager: PluginManager = mockPluginManager override val config = testConfig - override def downloadFile(fileUrl: URL, destinationUrl: URL): URL = - new URL("file://someFile") // Cannot mock URL } - addJarMagic.execute("""http://www.example.com/someJar.jar""") + addJarMagic.execute("""http://repo1.maven.org/maven2/org/scala-rules/rule-engine-core_2.11/0.5.1/rule-engine-core_2.11-0.5.1.jar""") verify(mockKernel).addJars(any[URI]) verify(mockPluginManager, times(0)).loadPlugins(any()) @@ -76,6 +75,20 @@ class AddJarSpec extends FunSpec with Matchers with MockitoSugar { } } + it("should raise exception if jar file does not exist") { + val mockOutputStream = mock[OutputStream] + + val addJarMagic = new AddJar + with IncludeOutputStream + { + override val outputStream: OutputStream = mockOutputStream + } + + intercept[IllegalArgumentException] { + addJarMagic.execute("""http://ibm.com/this.jar.does.not.exist.jar""") + } + } + it("should extract jar file name from jar URL") { val mockOutputStream = mock[OutputStream] @@ -116,21 +129,11 @@ class AddJarSpec extends FunSpec with Matchers with MockitoSugar { override val config = testConfig override def downloadFile(fileUrl: URL, destinationUrl: URL): URL = { downloadFileCalled = true - new URL("file://someFile") // Cannot mock URL + super.downloadFile(fileUrl, destinationUrl) } } - // Create a temporary file representing our jar to fake the cache - val tmpFilePath = Files.createTempFile( - FileSystems.getDefault.getPath(AddJar.getJarDir(testConfig)), - "someJar", - ".jar" - ) - - addJarMagic.execute( - """http://www.example.com/""" + tmpFilePath.getFileName) - - tmpFilePath.toFile.delete() + addJarMagic.execute("""http://repo1.maven.org/maven2/org/scala-rules/rule-engine-core_2.11/0.5.1/rule-engine-core_2.11-0.5.1.jar""") downloadFileCalled should be (false) verify(mockKernel).addJars(any[URI]) @@ -153,28 +156,17 @@ class AddJarSpec extends FunSpec with Matchers with MockitoSugar { override val config = testConfig override def downloadFile(fileUrl: URL, destinationUrl: URL): URL = { downloadFileCalled = true - new URL("file://someFile") // Cannot mock URL + super.downloadFile(fileUrl, destinationUrl) } } - // Create a temporary file representing our jar to fake the cache - val tmpFilePath = Files.createTempFile( - FileSystems.getDefault.getPath(AddJar.getJarDir(testConfig)), - "someJar", - ".jar" - ) - - addJarMagic.execute( - """-f http://www.example.com/""" + tmpFilePath.getFileName) - - tmpFilePath.toFile.delete() + addJarMagic.execute("""-f http://repo1.maven.org/maven2/org/scala-rules/rule-engine-core_2.11/0.5.1/rule-engine-core_2.11-0.5.1.jar""") downloadFileCalled should be (true) verify(mockKernel).addJars(any[URI]) } - it("should add magic jar to magicloader and not to interpreter and spark"+ - "context") { + it("should add magic jar to magicloader and not to interpreter and spark context") { val mockSparkContext = mock[SparkContext] val mockInterpreter = mock[Interpreter] val mockOutputStream = mock[OutputStream] @@ -190,12 +182,10 @@ class AddJarSpec extends FunSpec with Matchers with MockitoSugar { override val outputStream: OutputStream = mockOutputStream override lazy val pluginManager: PluginManager = mockPluginManager override val config = testConfig - override def downloadFile(fileUrl: URL, destinationUrl: URL): URL = - new URL("file://someFile") // Cannot mock URL } addJarMagic.execute( - """--magic http://www.example.com/someJar.jar""") + """--magic http://repo1.maven.org/maven2/org/scala-rules/rule-engine-core_2.11/0.5.1/rule-engine-core_2.11-0.5.1.jar""") verify(mockPluginManager).loadPlugins(any()) verify(mockSparkContext, times(0)).addJar(anyString())