Author: omalley
Date: Fri Mar 4 03:46:46 2011
New Revision: 1077155
URL: http://svn.apache.org/viewvc?rev=1077155&view=rev
Log:
commit c39d4ff298b9a0c6a72015667714fdd82cf481ef
Author: Hemanth Yamijala <yhemanth@friendchild-lm.(none)>
Date: Mon Feb 8 20:15:15 2010 +0530
MAPREDUCE:1435 from
https://issues.apache.org/jira/secure/attachment/12435154/MR-1435-y20s.patch
+++ b/YAHOO-CHANGES.txt
+ MAPREDUCE-1435. Fixes the way symlinks are handled when cleaning up
+ work directory files. (Ravi Gummadi via yhemanth)
+
Added:
hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/fs/TestFileUtil.java
Removed:
hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/mapred/TestSetupWorkDir.java
Modified:
hadoop/common/branches/branch-0.20-security-patches/src/c++/task-controller/task-controller.c
hadoop/common/branches/branch-0.20-security-patches/src/core/org/apache/hadoop/fs/FileUtil.java
hadoop/common/branches/branch-0.20-security-patches/src/mapred/org/apache/hadoop/mapred/TaskRunner.java
hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java
Modified:
hadoop/common/branches/branch-0.20-security-patches/src/c++/task-controller/task-controller.c
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-security-patches/src/c%2B%2B/task-controller/task-controller.c?rev=1077155&r1=1077154&r2=1077155&view=diff
==============================================================================
---
hadoop/common/branches/branch-0.20-security-patches/src/c++/task-controller/task-controller.c
(original)
+++
hadoop/common/branches/branch-0.20-security-patches/src/c++/task-controller/task-controller.c
Fri Mar 4 03:46:46 2011
@@ -343,11 +343,13 @@ static int secure_path(const char *path,
break;
case FTS_SL:
// A symbolic link
- process_path = 1;
+ // We don't want to change-ownership(and set-permissions) for the
file/dir
+ // pointed to by any symlink.
+ process_path = 0;
break;
case FTS_SLNONE:
// A symbolic link with a nonexistent target
- process_path = 1;
+ process_path = 0;
break;
case FTS_NS:
// A file for which no stat(2) information was available
Modified:
hadoop/common/branches/branch-0.20-security-patches/src/core/org/apache/hadoop/fs/FileUtil.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-security-patches/src/core/org/apache/hadoop/fs/FileUtil.java?rev=1077155&r1=1077154&r2=1077155&view=diff
==============================================================================
---
hadoop/common/branches/branch-0.20-security-patches/src/core/org/apache/hadoop/fs/FileUtil.java
(original)
+++
hadoop/common/branches/branch-0.20-security-patches/src/core/org/apache/hadoop/fs/FileUtil.java
Fri Mar 4 03:46:46 2011
@@ -71,6 +71,17 @@ public class FileUtil {
* we return false, the directory may be partially-deleted.
*/
public static boolean fullyDelete(File dir) throws IOException {
+ if (!fullyDeleteContents(dir)) {
+ return false;
+ }
+ return dir.delete();
+ }
+
+ /**
+ * Delete the contents of a directory, not the directory itself. If
+ * we return false, the directory may be partially-deleted.
+ */
+ public static boolean fullyDeleteContents(File dir) throws IOException {
File contents[] = dir.listFiles();
if (contents != null) {
for (int i = 0; i < contents.length; i++) {
@@ -95,7 +106,7 @@ public class FileUtil {
}
}
}
- return dir.delete();
+ return true;
}
/**
Modified:
hadoop/common/branches/branch-0.20-security-patches/src/mapred/org/apache/hadoop/mapred/TaskRunner.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-security-patches/src/mapred/org/apache/hadoop/mapred/TaskRunner.java?rev=1077155&r1=1077154&r2=1077155&view=diff
==============================================================================
---
hadoop/common/branches/branch-0.20-security-patches/src/mapred/org/apache/hadoop/mapred/TaskRunner.java
(original)
+++
hadoop/common/branches/branch-0.20-security-patches/src/mapred/org/apache/hadoop/mapred/TaskRunner.java
Fri Mar 4 03:46:46 2011
@@ -643,39 +643,6 @@ abstract class TaskRunner extends Thread
}
/**
- * Sets permissions recursively and then deletes the contents of dir.
- * Makes dir empty directory(does not delete dir itself).
- */
- static void deleteDirContents(JobConf conf, File dir) throws IOException {
- FileSystem fs = FileSystem.getLocal(conf);
- if (fs.exists(new Path(dir.getAbsolutePath()))) {
- File contents[] = dir.listFiles();
- if (contents != null) {
- for (int i = 0; i < contents.length; i++) {
- try {
- int ret = 0;
- if ((ret = FileUtil.chmod(contents[i].getAbsolutePath(),
- "ug+rwx", true)) != 0) {
- LOG.warn("Unable to chmod for " + contents[i] +
- "; chmod exit status = " + ret);
- }
- } catch(InterruptedException e) {
- LOG.warn("Interrupted while setting permissions for contents of " +
- "workDir. Not deleting the remaining contents of workDir.");
- return;
- }
- if (!fs.delete(new Path(contents[i].getAbsolutePath()), true)) {
- LOG.warn("Unable to delete "+ contents[i]);
- }
- }
- }
- }
- else {
- LOG.warn(dir + " does not exist.");
- }
- }
-
- /**
* Creates distributed cache symlinks and tmp directory, as appropriate.
* Note that when we setup the distributed
* cache, we didn't create the symlinks. This is done on a per task basis
@@ -692,7 +659,7 @@ abstract class TaskRunner extends Thread
/** delete only the contents of workDir leaving the directory empty. We
* can't delete the workDir as it is the current working directory.
*/
- deleteDirContents(conf, workDir);
+ FileUtil.fullyDeleteContents(workDir);
if (DistributedCache.getSymlink(conf)) {
URI[] archives = DistributedCache.getCacheArchives(conf);
Added:
hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/fs/TestFileUtil.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/fs/TestFileUtil.java?rev=1077155&view=auto
==============================================================================
---
hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/fs/TestFileUtil.java
(added)
+++
hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/fs/TestFileUtil.java
Fri Mar 4 03:46:46 2011
@@ -0,0 +1,104 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.fs;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestFileUtil {
+ final static private File TEST_DIR = new File(System.getProperty(
+ "test.build.data", "/tmp"), "fu");
+ static String FILE = "x";
+ static String LINK = "y";
+ static String DIR = "dir";
+ File del = new File(TEST_DIR, "del");
+ File tmp = new File(TEST_DIR, "tmp");
+
+ /**
+ * Creates directories del and tmp for testing.
+ *
+ * Contents of them are
+ * dir:tmp:
+ * file: x
+ * dir:del:
+ * file: x
+ * dir: dir1 : file:x
+ * dir: dir2 : file:x
+ * link: y to tmp/x
+ * link: tmpDir to tmp
+ */
+ @Before
+ public void setUp() throws IOException {
+ Assert.assertFalse(del.exists());
+ Assert.assertFalse(tmp.exists());
+ del.mkdirs();
+ tmp.mkdirs();
+ new File(del, FILE).createNewFile();
+ File tmpFile = new File(tmp, FILE);
+ tmpFile.createNewFile();
+
+ // create directories
+ File one = new File(del, DIR + "1");
+ one.mkdirs();
+ File two = new File(del, DIR + "2");
+ two.mkdirs();
+ new File(one, FILE).createNewFile();
+ new File(two, FILE).createNewFile();
+
+ // create a symlink to file
+ File link = new File(del, LINK);
+ FileUtil.symLink(tmpFile.toString(), link.toString());
+
+ // create a symlink to dir
+ File linkDir = new File(del, "tmpDir");
+ FileUtil.symLink(tmp.toString(), linkDir.toString());
+ Assert.assertEquals(5, del.listFiles().length);
+ }
+
+ @After
+ public void tearDown() throws IOException {
+ FileUtil.fullyDelete(del);
+ FileUtil.fullyDelete(tmp);
+ }
+
+ @Test
+ public void testFullyDelete() throws IOException {
+ FileUtil.fullyDelete(del);
+ Assert.assertFalse(del.exists());
+ validateTmpDir();
+ }
+
+ @Test
+ public void testFullyDeleteContents() throws IOException {
+ FileUtil.fullyDeleteContents(del);
+ Assert.assertTrue(del.exists());
+ Assert.assertEquals(0, del.listFiles().length);
+ validateTmpDir();
+ }
+
+ private void validateTmpDir() {
+ Assert.assertTrue(tmp.exists());
+ Assert.assertEquals(1, tmp.listFiles().length);
+ Assert.assertTrue(new File(tmp, FILE).exists());
+ }
+}
Modified:
hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java
URL:
http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java?rev=1077155&r1=1077154&r2=1077155&view=diff
==============================================================================
---
hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java
(original)
+++
hadoop/common/branches/branch-0.20-security-patches/src/test/org/apache/hadoop/mapred/TestTaskTrackerLocalization.java
Fri Mar 4 03:46:46 2011
@@ -569,11 +569,36 @@ public class TestTaskTrackerLocalization
}
/**
+ * Create a file in the given dir and set permissions r_xr_xr_x sothat no one
+ * can delete it directly(without doing chmod).
+ * Creates dir/subDir and dir/subDir/file
+ */
+ static void createFileAndSetPermissions(JobConf jobConf, Path dir)
+ throws IOException {
+ Path subDir = new Path(dir, "subDir");
+ FileSystem fs = FileSystem.getLocal(jobConf);
+ fs.mkdirs(subDir);
+ Path p = new Path(subDir, "file");
+ java.io.DataOutputStream out = fs.create(p);
+ out.writeBytes("dummy input");
+ out.close();
+ // no write permission for subDir and subDir/file
+ try {
+ int ret = 0;
+ if((ret = FileUtil.chmod(subDir.toUri().getPath(), "a=rx", true)) != 0) {
+ LOG.warn("chmod failed for " + subDir + ";retVal=" + ret);
+ }
+ } catch(InterruptedException e) {
+ LOG.warn("Interrupted while doing chmod for " + subDir);
+ }
+ }
+
+ /**
* Validates the removal of $taskid and $tasid/work under mapred-local-dir
* in cases where those directories cannot be deleted without adding
* write permission to the newly created directories under $taskid and
* $taskid/work
- * Also see TestSetupWorkDir.createFileAndSetPermissions for details
+ * Also see createFileAndSetPermissions for details
*/
void validateRemoveFiles(boolean needCleanup, boolean jvmReuse,
TaskInProgress tip) throws IOException {
@@ -588,7 +613,7 @@ public class TestTaskTrackerLocalization
Path[] paths = tracker.getLocalFiles(localizedJobConf, dir);
for (Path p : paths) {
if (tracker.getLocalFileSystem().exists(p)) {
- TestSetupWorkDir.createFileAndSetPermissions(localizedJobConf, p);
+ createFileAndSetPermissions(localizedJobConf, p);
}
}
