From 4dbb63f0857a9cfb018cf49e3d1103cacc862ba2 Mon Sep 17 00:00:00 2001
From: James Shuster <jshuster@palantir.com>
Date: Mon, 22 May 2017 21:41:11 -0700
Subject: [PATCH] [SPARK-20815][SPARKR] NullPointerException in
RPackageUtils#checkManifestForR
## What changes were proposed in this pull request?
- Add a null check to RPackageUtils#checkManifestForR so that jars w/o manifests don't NPE.
## How was this patch tested?
- Unit tests and manual tests.
Author: James Shuster <jshuster@palantir.com>
Closes #18040 from jrshust/feature/r-package-utils.
---
.../org/apache/spark/deploy/RPackageUtils.scala | 3 +++
.../org/apache/spark/deploy/IvyTestUtils.scala | 14 ++++++++++----
.../apache/spark/deploy/RPackageUtilsSuite.scala | 10 ++++++++++
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/core/src/main/scala/org/apache/spark/deploy/RPackageUtils.scala b/core/src/main/scala/org/apache/spark/deploy/RPackageUtils.scala
index 050778a895..7d356e8fc1 100644
--- a/core/src/main/scala/org/apache/spark/deploy/RPackageUtils.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/RPackageUtils.scala
@@ -92,6 +92,9 @@ private[deploy] object RPackageUtils extends Logging {
* Exposed for testing.
*/
private[deploy] def checkManifestForR(jar: JarFile): Boolean = {
+ if (jar.getManifest == null) {
+ return false
+ }
val manifest = jar.getManifest.getMainAttributes
manifest.getValue(hasRPackage) != null && manifest.getValue(hasRPackage).trim == "true"
}
diff --git a/core/src/test/scala/org/apache/spark/deploy/IvyTestUtils.scala b/core/src/test/scala/org/apache/spark/deploy/IvyTestUtils.scala
index f50cb38311..42b8cde650 100644
--- a/core/src/test/scala/org/apache/spark/deploy/IvyTestUtils.scala
+++ b/core/src/test/scala/org/apache/spark/deploy/IvyTestUtils.scala
@@ -243,16 +243,22 @@ private[deploy] object IvyTestUtils {
withManifest: Option[Manifest] = None): File = {
val jarFile = new File(dir, artifactName(artifact, useIvyLayout))
val jarFileStream = new FileOutputStream(jarFile)
- val manifest = withManifest.getOrElse {
- val mani = new Manifest()
+ val manifest: Manifest = withManifest.getOrElse {
if (withR) {
+ val mani = new Manifest()
val attr = mani.getMainAttributes
attr.put(Name.MANIFEST_VERSION, "1.0")
attr.put(new Name("Spark-HasRPackage"), "true")
+ mani
+ } else {
+ null
}
- mani
}
- val jarStream = new JarOutputStream(jarFileStream, manifest)
+ val jarStream = if (manifest != null) {
+ new JarOutputStream(jarFileStream, manifest)
+ } else {
+ new JarOutputStream(jarFileStream)
+ }
for (file <- files) {
val jarEntry = new JarEntry(file._1)
diff --git a/core/src/test/scala/org/apache/spark/deploy/RPackageUtilsSuite.scala b/core/src/test/scala/org/apache/spark/deploy/RPackageUtilsSuite.scala
index 005587051b..5e0bf6d438 100644
--- a/core/src/test/scala/org/apache/spark/deploy/RPackageUtilsSuite.scala
+++ b/core/src/test/scala/org/apache/spark/deploy/RPackageUtilsSuite.scala
@@ -133,6 +133,16 @@ class RPackageUtilsSuite
}
}
+ test("jars without manifest return false") {
+ IvyTestUtils.withRepository(main, None, None) { repo =>
+ val jar = IvyTestUtils.packJar(new File(new URI(repo)), dep1, Nil,
+ useIvyLayout = false, withR = false, None)
+ val jarFile = new JarFile(jar)
+ assert(jarFile.getManifest == null, "jar file should have null manifest")
+ assert(!RPackageUtils.checkManifestForR(jarFile), "null manifest should return false")
+ }
+ }
+
test("SparkR zipping works properly") {
val tempDir = Files.createTempDir()
Utils.tryWithSafeFinally {
--
GitLab