hadoop-common-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From omal...@apache.org
Subject svn commit: r1077155 - in /hadoop/common/branches/branch-0.20-security-patches/src: c++/task-controller/ core/org/apache/hadoop/fs/ mapred/org/apache/hadoop/mapred/ test/org/apache/hadoop/fs/ test/org/apache/hadoop/mapred/
Date Fri, 04 Mar 2011 03:46:46 GMT
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);
       }
     }
 



Mime
View raw message