svn commit: r697241 - in /hadoop/core/trunk: CHANGES.txt src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java src/test/org/apache/hadoop/hdfs/TestDFSPermission.java

[dh...@apache.org]

Author: dhruba
Date: Fri Sep 19 14:02:54 2008
New Revision: 697241

URL: http://svn.apache.org/viewvc?rev=697241&view=rev
Log:
HADOOP-4077. Setting access and modification time for a file
requires write permissions on the file. (dhruba)


Modified:
    hadoop/core/trunk/CHANGES.txt
    hadoop/core/trunk/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
    hadoop/core/trunk/src/test/org/apache/hadoop/hdfs/TestDFSPermission.java

Modified: hadoop/core/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/core/trunk/CHANGES.txt?rev=697241&r1=697240&r2=697241&view=diff
==============================================================================
--- hadoop/core/trunk/CHANGES.txt (original)
+++ hadoop/core/trunk/CHANGES.txt Fri Sep 19 14:02:54 2008
@@ -684,6 +684,9 @@
     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

Modified: hadoop/core/trunk/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
URL: http://svn.apache.org/viewvc/hadoop/core/trunk/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=697241&r1=697240&r2=697241&view=diff
==============================================================================
--- hadoop/core/trunk/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java (original)
+++ hadoop/core/trunk/src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java Fri Sep 19 14:02:54 2008
@@ -869,14 +869,9 @@
                             " 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 @@
                       Server.getRemoteIp(),
                       "setTimes", src, null, stat);
       }
+    } else {
+      throw new FileNotFoundException("File " + src + " does not exist.");
     }
   }
 

Modified: hadoop/core/trunk/src/test/org/apache/hadoop/hdfs/TestDFSPermission.java
URL: http://svn.apache.org/viewvc/hadoop/core/trunk/src/test/org/apache/hadoop/hdfs/TestDFSPermission.java?rev=697241&r1=697240&r2=697241&view=diff
==============================================================================
--- hadoop/core/trunk/src/test/org/apache/hadoop/hdfs/TestDFSPermission.java (original)
+++ hadoop/core/trunk/src/test/org/apache/hadoop/hdfs/TestDFSPermission.java Fri Sep 19 14:02:54 2008
@@ -400,6 +400,8 @@
           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 @@
     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 {