HADOOP-4077. Setting access and modification time for a file

requires write permissions on the file. (dhruba)



git-svn-id: https://svn.apache.org/repos/asf/hadoop/core/trunk@697241 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -684,6 +684,9 @@ Trunk (unreleased changes)
     HADOOP-4213. Fixes NPE in TestLimitTasksPerJobTaskScheduler.
     (Sreekanth Ramakrishnan via ddas)
 
+    HADOOP-4077. Setting access and modification time for a file
+    requires write permissions on the file. (dhruba)
+
 Release 0.18.1 - 2008-09-17
 
   IMPROVEMENTS

src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -869,14 +869,9 @@ public class FSNamesystem implements FSConstants, FSNamesystemMBean {
                             " Please set dfs.support.accessTime configuration parameter.");
     }
     //
-    // The caller needs to have read-access to set access times
-    // and write access to set modification times.
+    // The caller needs to have write access to set access & modification times.
     if (isPermissionEnabled) {
-      if (mtime == -1) {
-        checkPathAccess(src, FsAction.READ);
-      } else {
-        checkPathAccess(src, FsAction.WRITE);
-      }
+      checkPathAccess(src, FsAction.WRITE);
     }
     INodeFile inode = dir.getFileINode(src);
     if (inode != null) {
@@ -887,6 +882,8 @@ public class FSNamesystem implements FSConstants, FSNamesystemMBean {
                       Server.getRemoteIp(),
                       "setTimes", src, null, stat);
       }
+    } else {
+      throw new FileNotFoundException("File " + src + " does not exist.");
     }
   }
 

src/test/org/apache/hadoop/hdfs/TestDFSPermission.java

@@ -400,6 +400,8 @@ public class TestDFSPermission extends TestCase {
           filePermission[i]);
       testSetReplication(ugi, files[i], ancestorPermission[i],
           parentPermission[i], filePermission[i]);
+      testSetTimes(ugi, files[i], ancestorPermission[i],
+          parentPermission[i], filePermission[i]);
       testStats(ugi, files[i], ancestorPermission[i], parentPermission[i]);
       testList(ugi, files[i], dirs[i], ancestorPermission[i],
           parentPermission[i], filePermission[i]);
@@ -657,6 +659,34 @@ public class TestDFSPermission extends TestCase {
     replicatorVerifier.verifyPermission(ugi);
   }
 
+  /* A class that verifies the permission checking is correct for 
+   * setTimes */
+  private class SetTimesPermissionVerifier extends PermissionVerifier {
+    @Override
+    void setOpPermission() {
+      this.opParentPermission = SEARCH_MASK;
+      this.opPermission = WRITE_MASK;
+    }
+
+    @Override
+    void call() throws IOException {
+      fs.setTimes(path, 100, 100);
+      fs.setTimes(path, -1, 100);
+      fs.setTimes(path, 100, -1);
+    }
+  }
+
+  private SetTimesPermissionVerifier timesVerifier =
+    new SetTimesPermissionVerifier();
+  /* test if the permission checking of setReplication is correct */
+  private void testSetTimes(UnixUserGroupInformation ugi, Path path,
+      short ancestorPermission, short parentPermission, short filePermission)
+      throws Exception {
+    timesVerifier.set(path, ancestorPermission, parentPermission,
+        filePermission);
+    timesVerifier.verifyPermission(ugi);
+  }
+
   /* A class that verifies the permission checking is correct for isDirectory,
    * exist,  getFileInfo, getContentSummary */
   private class StatsPermissionVerifier extends PermissionVerifier {