commit 22a0d0d97d2ac5bdeecfe1adadcaa75328c65ad2

Author: Hemanth Yamijala <yhemanth@yahoo-inc.com>
Date:   Mon Jan 11 19:59:10 2010 +0530

    Reverting https://issues.apache.org/jira/secure/attachment/12428180/y896.v2.1.fix.v2.patch for bug fix for MAPREDUCE:896
    
    +++ b/YAHOO-CHANGES.txt


git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-0.20-security-patches@1077100 13f79535-47bb-0310-9956-ffa450edef68

src/mapred/org/apache/hadoop/mapred/CleanupQueue.java

@@ -88,10 +88,7 @@ class CleanupQueue {
     if (LOG.isDebugEnabled()) {
       LOG.debug("Trying to delete " + context.fullPath);
     }
-    if (context.fs.exists(new Path(context.fullPath))) {
-      return context.fs.delete(new Path(context.fullPath), true);
-    }
-    return true;
+    return context.fs.delete(new Path(context.fullPath), true);
   }
 
   private static class PathCleanupThread extends Thread {

src/mapred/org/apache/hadoop/mapred/DefaultTaskController.java

@@ -147,8 +147,6 @@ class DefaultTaskController extends TaskController {
     } catch(InterruptedException e) {
       LOG.warn("Interrupted while setting permissions for " + context.fullPath +
           " for deletion.");
-    } catch(IOException ioe) {
-      LOG.warn("Unable to change permissions of " + context.fullPath);
     }
   }
 }

src/mapred/org/apache/hadoop/mapred/LinuxTaskController.java

@@ -265,15 +265,10 @@ class LinuxTaskController extends TaskController {
       TaskControllerPathDeletionContext tContext =
         (TaskControllerPathDeletionContext) context;
     
-      if (tContext.task.getUser() != null &&
-          tContext.fs instanceof LocalFileSystem) {
-        try {
-          runCommand(TaskCommands.ENABLE_TASK_FOR_CLEANUP,
+      if (tContext.task.getUser() != null && tContext.fs instanceof LocalFileSystem) {
+        runCommand(TaskCommands.ENABLE_TASK_FOR_CLEANUP,
                    tContext.task.getUser(),
                    buildTaskCleanupArgs(tContext), null, null);
-        } catch(IOException e) {
-          LOG.warn("Uanble to change permissions for " + tContext.fullPath);
-        }
       }
       else {
         throw new IllegalArgumentException("Either user is null or the "  +

src/mapred/org/apache/hadoop/mapred/TaskController.java

@@ -179,9 +179,7 @@ abstract class TaskController implements Configurable {
     @Override
     protected void enablePathForCleanup() throws IOException {
       getPathForCleanup();// allow init of fullPath
-      if (fs.exists(new Path(fullPath))) {
-        taskController.enableTaskForCleanup(this); 
-      }
+      taskController.enableTaskForCleanup(this);
     }
   }
 

src/mapred/org/apache/hadoop/mapred/TaskRunner.java

@@ -557,6 +557,7 @@ 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 {
@@ -565,6 +566,18 @@ abstract class TaskRunner extends Thread {
       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(),
+                                      "a+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]);
           }

src/test/org/apache/hadoop/mapred/TestSetupWorkDir.java

@@ -23,36 +23,45 @@ import java.io.IOException;
 
 import junit.framework.TestCase;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.fs.Path;
 
-/**
- * Validates if TaskRunner.deleteDirContents() is properly cleaning up the
- * contents of workDir.
- */
 public class TestSetupWorkDir extends TestCase {
-  private static int NUM_SUB_DIRS = 3;
+  private static final Log LOG =
+    LogFactory.getLog(TestSetupWorkDir.class);
 
   /**
-   * Creates subdirectories under given dir and files under those subdirs.
-   * Creates dir/subDir1, dir/subDir1/file, dir/subDir2, dir/subDir2/file, etc.
+   * 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 createSubDirs(JobConf jobConf, Path dir)
+  static void createFileAndSetPermissions(JobConf jobConf, Path dir)
        throws IOException {
-    for (int i = 1; i <= NUM_SUB_DIRS; i++) {
-      Path subDir = new Path(dir, "subDir" + i);
-      FileSystem fs = FileSystem.getLocal(jobConf);
-      fs.mkdirs(subDir);
-      Path p = new Path(subDir, "file");
-      DataOutputStream out = fs.create(p);
-      out.writeBytes("dummy input");
-      out.close();
+    Path subDir = new Path(dir, "subDir");
+    FileSystem fs = FileSystem.getLocal(jobConf);
+    fs.mkdirs(subDir);
+    Path p = new Path(subDir, "file");
+    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 if TaskRunner.deleteDirContents() is properly cleaning up the
-   * contents of workDir.
+   * Validates if setupWorkDir is properly cleaning up contents of workDir.
+   * TODO: other things of TaskRunner.setupWorkDir() related to distributed
+   * cache need to be validated.
    */
   public void testSetupWorkDir() throws IOException {
     Path rootDir = new Path(System.getProperty("test.build.data",  "/tmp"),
@@ -67,12 +76,9 @@ public class TestSetupWorkDir extends TestCase {
       throw new IOException("Unable to create workDir " + myWorkDir);
     }
 
-    // create subDirs under work dir
-    createSubDirs(jConf, myWorkDir);
+    // create {myWorkDir}/subDir/file and set 555 perms for subDir and file
+    createFileAndSetPermissions(jConf, myWorkDir);
 
-    assertTrue("createDirAndSubDirs() did not create subdirs under "
-        + myWorkDir, fs.listStatus(myWorkDir).length == NUM_SUB_DIRS);
-    
     TaskRunner.deleteDirContents(jConf, new File(myWorkDir.toUri().getPath()));
     
     assertTrue("Contents of " + myWorkDir + " are not cleaned up properly.",