This is an automated email from the ASF dual-hosted git repository.
dongjoon-hyun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 4b0e62443fee [SPARK-38632][CORE] Use `FileUtil.unTar` for `.tar` files
at `Utils.unpack`
4b0e62443fee is described below
commit 4b0e62443fee542b06b1ea13809b7161af0c3e44
Author: Dongjoon Hyun <[email protected]>
AuthorDate: Wed Jul 8 21:21:02 2026 -0700
[SPARK-38632][CORE] Use `FileUtil.unTar` for `.tar` files at `Utils.unpack`
### What changes were proposed in this pull request?
This PR aims to remove the Java-based un-tarring workaround
(`unTarUsingJava`) from `Utils.unpack` and use Hadoop's `FileUtil.unTar` for
`.tar` files again, like `.tar.gz` and `.tgz` files.
### Why are the changes needed?
SPARK-38631 introduced this workaround because Hadoop's `FileUtil.unTar`
did not escape the file name before passing it to a shell command, which
allowed arbitrary shell command injection. This was fixed by
[HADOOP-18136](https://issues.apache.org/jira/browse/HADOOP-18136) via
`FileUtil.makeSecureShellPath` and shipped in Apache Hadoop 3.2.4, 3.3.3 or
upper. Since Apache Spark requires Hadoop 3.4.0 or later (SPARK-58053) and the
master branch uses Hadoop 3.5.0, the workaround is no lo [...]
Removing it also has the following benefits.
- `.tar` files keep the original file permissions and symlinks because the
un-tarring is delegated to the native `tar` utility, which resolves the
original TODO of SPARK-38632.
- The `setAccessible(true)` reflection hack on a Hadoop private method is
removed.
### Does this PR introduce _any_ user-facing change?
Yes, unpacking `.tar` archives now preserves file permissions and symlinks
like `.tar.gz` and `.tgz` archives. This is the original behavior.
### How was this patch tested?
Pass the CIs.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Fable 5
Closes #57150 from dongjoon-hyun/dongjoon/commit-057c051285e-review-3aaa5c.
Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.../main/scala/org/apache/spark/util/Utils.scala | 29 ++--------------------
1 file changed, 2 insertions(+), 27 deletions(-)
diff --git a/core/src/main/scala/org/apache/spark/util/Utils.scala
b/core/src/main/scala/org/apache/spark/util/Utils.scala
index 39f9ca8a472c..c8030e5a4c7c 100644
--- a/core/src/main/scala/org/apache/spark/util/Utils.scala
+++ b/core/src/main/scala/org/apache/spark/util/Utils.scala
@@ -455,11 +455,9 @@ private[spark] object Utils
} else if (lowerSrc.endsWith(".zip")) {
// After issue HADOOP-18145, unzip could keep file permissions.
FileUtil.unZip(source, dest)
- } else if (lowerSrc.endsWith(".tar.gz") || lowerSrc.endsWith(".tgz")) {
+ } else if (
+ lowerSrc.endsWith(".tar.gz") || lowerSrc.endsWith(".tgz") ||
lowerSrc.endsWith(".tar")) {
FileUtil.unTar(source, dest)
- } else if (lowerSrc.endsWith(".tar")) {
- // TODO(SPARK-38632): should keep file permissions. Java implementation
doesn't.
- unTarUsingJava(source, dest)
} else {
logWarning(log"Cannot unpack ${MDC(LogKeys.FILE_NAME, source)}, " +
log"just copying it to ${MDC(FILE_NAME2, dest)}.")
@@ -467,29 +465,6 @@ private[spark] object Utils
}
}
- /**
- * The method below was copied from `FileUtil.unTar` but uses Java-based
implementation
- * to work around a security issue, see also SPARK-38631.
- */
- private def unTarUsingJava(source: File, dest: File): Unit = {
- if (!Utils.createDirectory(dest) && !dest.isDirectory) {
- throw new IOException(s"Mkdirs failed to create $dest")
- } else {
- try {
- // Should not fail because all Hadoop 2.1+ (from HADOOP-9264)
- // have 'unTarUsingJava'.
- val mth = classOf[FileUtil].getDeclaredMethod(
- "unTarUsingJava", classOf[File], classOf[File], classOf[Boolean])
- mth.setAccessible(true)
- mth.invoke(null, source, dest, java.lang.Boolean.FALSE)
- } catch {
- // Re-throw the original exception.
- case e: java.lang.reflect.InvocationTargetException if e.getCause !=
null =>
- throw e.getCause
- }
- }
- }
-
/** Records the duration of running `body`. */
def timeTakenMs[T](body: => T): (T, Long) = {
val startTime = System.nanoTime()
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
