HADOOP-9081. Add TestWinUtils. Contributed by Chuan Liu, Ivan Mitic, Chris Nauroth, and Bikas Saha.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-trunk-win@1422280 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.branch-trunk-win.txt

@@ -66,3 +66,6 @@ branch-trunk-win changes - unreleased
   HADOOP-9144. Fix findbugs warnings. (Chris Nauroth via suresh)
 
   HADOOP-8981. TestMetricsSystemImpl fails on Windows. (Xuan Gong via suresh)
+
+  HADOOP-9081. Add TestWinUtils. (Chuan Liu, Ivan Mitic, Chris Nauroth, 
+  and Bikas Saha via suresh)

hadoop-common-project/hadoop-common/pom.xml

@@ -241,6 +241,11 @@
       <type>test-jar</type>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-compress</artifactId>
+      <version>1.4</version>
+    </dependency>
   </dependencies>
 
   <build>
@@ -330,6 +335,23 @@
               </target>
             </configuration>
           </execution>
+          <execution>
+            <id>copy-test-tarballs</id>
+            <phase>process-test-resources</phase>
+            <goals>
+              <goal>run</goal>
+            </goals>
+            <configuration>
+              <target>
+                <copy toDir="${test.cache.data}">
+                  <fileset dir="${basedir}/src/test/java/org/apache/hadoop/fs">
+                    <include name="test-untar.tar"/>
+                    <include name="test-untar.tgz"/>
+                  </fileset>
+                </copy>
+              </target>
+            </configuration>
+          </execution>
           <execution>
             <phase>pre-site</phase>
             <goals>

hadoop-common-project/hadoop-common/src/main/bin/hadoop.cmd

@@ -173,10 +173,15 @@ call :updatepath %HADOOP_BIN_PATH%
 
 :updatepath
   set path_to_add=%*
-  set current_path_comparable=%path:(x86)=%
+  set current_path_comparable=%path%
   set current_path_comparable=%current_path_comparable: =_%
-  set path_to_add_comparable=%path_to_add:(x86)=%
+  set current_path_comparable=%current_path_comparable:(=_%
+  set current_path_comparable=%current_path_comparable:)=_%
+  set path_to_add_comparable=%path_to_add%
   set path_to_add_comparable=%path_to_add_comparable: =_%
+  set path_to_add_comparable=%path_to_add_comparable:(=_%
+  set path_to_add_comparable=%path_to_add_comparable:)=_%
+
   for %%i in ( %current_path_comparable% ) do (
     if /i "%%i" == "%path_to_add_comparable%" (
       set path_to_add_exist=true

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java

@@ -21,9 +21,12 @@ package org.apache.hadoop.fs;
 import java.io.*;
 import java.util.Arrays;
 import java.util.Enumeration;
+import java.util.zip.GZIPInputStream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
 
+import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
+import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.conf.Configuration;
@@ -538,33 +541,46 @@ public class FileUtil {
    * @throws IOException
    */
   public static void unTar(File inFile, File untarDir) throws IOException {
-    if (!untarDir.mkdirs()) {           
+    if (!untarDir.mkdirs()) {
       if (!untarDir.isDirectory()) {
         throw new IOException("Mkdirs failed to create " + untarDir);
       }
     }
 
-    StringBuilder untarCommand = new StringBuilder();
     boolean gzipped = inFile.toString().endsWith("gz");
+    if(Shell.WINDOWS) {
+      // Tar is not native to Windows. Use simple Java based implementation for 
+      // tests and simple tar archives
+      unTarUsingJava(inFile, untarDir, gzipped);
+    }
+    else {
+      // spawn tar utility to untar archive for full fledged unix behavior such 
+      // as resolving symlinks in tar archives
+      unTarUsingTar(inFile, untarDir, gzipped);
+    }
+  }
+  
+  private static void unTarUsingTar(File inFile, File untarDir,
+      boolean gzipped) throws IOException {
+    StringBuffer untarCommand = new StringBuffer();
     if (gzipped) {
-      untarCommand.append((Shell.WINDOWS) ? " gzip -dc \"" : " gzip -dc '");
+      untarCommand.append(" gzip -dc '");
       untarCommand.append(FileUtil.makeShellPath(inFile));
-      untarCommand.append((Shell.WINDOWS) ? "\" | (" : "' | (");
+      untarCommand.append("' | (");
     } 
-    untarCommand.append((Shell.WINDOWS) ? "cd \"" : "cd '");
+    untarCommand.append("cd '");
     untarCommand.append(FileUtil.makeShellPath(untarDir)); 
-    untarCommand.append((Shell.WINDOWS) ? "\" & " : "' ; ");
+    untarCommand.append("' ; ");
 
     // Force the archive path as local on Windows as it can have a colon
-    untarCommand.append((Shell.WINDOWS) ? "tar --force-local -xf " : "tar -xf ");
+    untarCommand.append("tar -xf ");
 
     if (gzipped) {
       untarCommand.append(" -)");
     } else {
       untarCommand.append(FileUtil.makeShellPath(inFile));
     }
-    String[] shellCmd = {(Shell.WINDOWS)?"cmd":"bash", (Shell.WINDOWS)?"/c":"-c",
-      untarCommand.toString() };
+    String[] shellCmd = { "bash", "-c", untarCommand.toString() };
     ShellCommandExecutor shexec = new ShellCommandExecutor(shellCmd);
     shexec.execute();
     int exitcode = shexec.getExitCode();
@@ -573,7 +589,62 @@ public class FileUtil {
                   ". Tar process exited with exit code " + exitcode);
     }
   }
+  
+  private static void unTarUsingJava(File inFile, File untarDir,
+      boolean gzipped) throws IOException {
+    InputStream inputStream = null;
+    if (gzipped) {
+      inputStream = new BufferedInputStream(new GZIPInputStream(
+          new FileInputStream(inFile)));
+    } else {
+      inputStream = new BufferedInputStream(new FileInputStream(inFile));
+    }
+
+    TarArchiveInputStream tis = new TarArchiveInputStream(inputStream);
+
+    for (TarArchiveEntry entry = tis.getNextTarEntry(); entry != null;) {
+      unpackEntries(tis, entry, untarDir);
+      entry = tis.getNextTarEntry();
+    }
+  }
+  
+  private static void unpackEntries(TarArchiveInputStream tis,
+      TarArchiveEntry entry, File outputDir) throws IOException {
+    if (entry.isDirectory()) {
+      File subDir = new File(outputDir, entry.getName());
+      if (!subDir.mkdir() && !subDir.isDirectory()) {
+        throw new IOException("Mkdirs failed to create tar internal dir "
+            + outputDir);
+      }
+
+      for (TarArchiveEntry e : entry.getDirectoryEntries()) {
+        unpackEntries(tis, e, subDir);
+      }
+
+      return;
+    }
 
+    File outputFile = new File(outputDir, entry.getName());
+    if (!outputDir.exists()) {
+      if (!outputDir.mkdirs()) {
+        throw new IOException("Mkdirs failed to create tar internal dir "
+            + outputDir);
+      }
+    }
+
+    int count;
+    byte data[] = new byte[2048];
+    BufferedOutputStream outputStream = new BufferedOutputStream(
+        new FileOutputStream(outputFile));
+
+    while ((count = tis.read(data)) != -1) {
+      outputStream.write(data, 0, count);
+    }
+
+    outputStream.flush();
+    outputStream.close();
+  }
+  
   /**
    * Class for creating hardlinks.
    * Supports Unix, Cygwin, WindXP.
@@ -598,11 +669,34 @@ public class FileUtil {
    */
   public static int symLink(String target, String linkname) throws IOException{
     // Run the input paths through Java's File so that they are converted to the
-    // native OS form. FIXME: Long term fix is to expose symLink API that
-    // accepts File instead of String, as symlinks can only be created on the
-    // local FS.
-    String[] cmd = Shell.getSymlinkCommand(new File(target).getPath(),
-        new File(linkname).getPath());
+    // native OS form
+    File targetFile = new File(target);
+    File linkFile = new File(linkname);
+
+    // If not on Java7+, copy a file instead of creating a symlink since
+    // Java6 has close to no support for symlinks on Windows. Specifically
+    // File#length and File#renameTo do not work as expected.
+    // (see HADOOP-9061 for additional details)
+    // We still create symlinks for directories, since the scenario in this
+    // case is different. The directory content could change in which
+    // case the symlink loses its purpose (for example task attempt log folder
+    // is symlinked under userlogs and userlogs are generated afterwards).
+    if (Shell.WINDOWS && !Shell.isJava7OrAbove() && targetFile.isFile()) {
+      try {
+        LOG.info("FileUtil#symlink: On Java6, copying file instead "
+            + linkname + " -> " + target);
+        org.apache.commons.io.FileUtils.copyFile(targetFile, linkFile);
+      } catch (IOException ex) {
+        LOG.warn("FileUtil#symlink failed to copy the file with error: "
+            + ex.getMessage());
+        // Exit with non-zero exit code
+        return 1;
+      }
+      return 0;
+    }
+
+    String[] cmd = Shell.getSymlinkCommand(targetFile.getPath(),
+        linkFile.getPath());
     ShellCommandExecutor shExec = new ShellCommandExecutor(cmd);
     try {
       shExec.execute();
@@ -668,6 +762,25 @@ public class FileUtil {
     return shExec.getExitCode();
   }
 
+  /**
+   * Set the ownership on a file / directory. User name and group name
+   * cannot both be null.
+   * @param file the file to change
+   * @param username the new user owner name
+   * @param groupname the new group owner name
+   * @throws IOException
+   */
+  public static void setOwner(File file, String username,
+      String groupname) throws IOException {
+    if (username == null && groupname == null) {
+      throw new IOException("username == null && groupname == null");
+    }
+    String arg = (username == null ? "" : username)
+        + (groupname == null ? "" : ":" + groupname);
+    String [] cmd = Shell.getSetOwnerCommand(arg);
+    execCommand(file, cmd);
+  }
+
   /**
    * Set permissions to the required value. Uses the java primitives instead
    * of forking if group == other.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java

@@ -494,9 +494,9 @@ public class RawLocalFileSystem extends FileSystem {
       return !super.getOwner().isEmpty(); 
     }
     
-    RawLocalFileStatus(File f, long defaultBlockSize, FileSystem fs) {
+    RawLocalFileStatus(File f, long defaultBlockSize, FileSystem fs) { 
       super(f.length(), f.isDirectory(), 1, defaultBlockSize,
-            f.lastModified(), fs.makeQualified(new Path(f.getPath())));
+          f.lastModified(), new Path(f.getPath()).makeQualified(fs));
     }
     
     @Override
@@ -527,9 +527,10 @@ public class RawLocalFileSystem extends FileSystem {
     private void loadPermissionInfo() {
       IOException e = null;
       try {
-        StringTokenizer t = new StringTokenizer(
-            FileUtil.execCommand(new File(getPath().toUri()), 
-                                          Shell.getGetPermissionCommand()));
+        String output = FileUtil.execCommand(new File(getPath().toUri()), 
+            Shell.getGetPermissionCommand());
+        StringTokenizer t =
+            new StringTokenizer(output, Shell.TOKEN_SEPARATOR_REGEX);
         //expected format
         //-rw-------    1 username groupname ...
         String permission = t.nextToken();
@@ -549,7 +550,6 @@ public class RawLocalFileSystem extends FileSystem {
         }
         setOwner(owner);
 
-        // FIXME: Group names could have spaces on Windows
         setGroup(t.nextToken());
       } catch (Shell.ExitCodeException ioe) {
         if (ioe.getExitCode() != 1) {
@@ -585,17 +585,7 @@ public class RawLocalFileSystem extends FileSystem {
   @Override
   public void setOwner(Path p, String username, String groupname)
     throws IOException {
-    if (username == null && groupname == null) {
-      throw new IOException("username == null && groupname == null");
-    }
-
-    if (username == null) {
-      FileUtil.execCommand(pathToFile(p), Shell.SET_GROUP_COMMAND, groupname); 
-    } else {
-      //OWNER[:[GROUP]]
-      String s = username + (groupname == null? "": ":" + groupname);
-      FileUtil.execCommand(pathToFile(p), Shell.getSetOwnerCommand(s));
-    }
+    FileUtil.setOwner(pathToFile(p), username, groupname);
   }
 
   /**

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/nativeio/NativeIO.java

@@ -356,10 +356,6 @@ public class NativeIO {
     /** Windows only methods used for getOwner() implementation */
     private static native String getOwner(FileDescriptor fd) throws IOException;
 
-    /** Windows only method used for getting the file length */
-    public static native long getLengthFollowSymlink(
-        String path) throws IOException;
-
     static {
       if (NativeCodeLoader.isNativeCodeLoaded()) {
         try {

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/ShellBasedUnixGroupsMapping.java

@@ -86,7 +86,8 @@ public class ShellBasedUnixGroupsMapping
       LOG.warn("got exception trying to get groups for user " + user, e);
     }
     
-    StringTokenizer tokenizer = new StringTokenizer(result);
+    StringTokenizer tokenizer =
+        new StringTokenizer(result, Shell.TOKEN_SEPARATOR_REGEX);
     List<String> groups = new LinkedList<String>();
     while (tokenizer.hasMoreTokens()) {
       groups.add(tokenizer.nextToken());

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java

@@ -45,9 +45,12 @@ abstract public class Shell {
   
   public static final Log LOG = LogFactory.getLog(Shell.class);
   
-  /** a Windows utility to emulate Unix commands */
-  public static final String WINUTILS = System.getenv("HADOOP_HOME")
-                                        + "\\bin\\winutils";
+  private static boolean IS_JAVA7_OR_ABOVE =
+      System.getProperty("java.version").substring(0, 3).compareTo("1.7") >= 0;
+
+  public static boolean isJava7OrAbove() {
+    return IS_JAVA7_OR_ABOVE;
+  }
 
   /** a Unix command to get the current user's name */
   public final static String USER_NAME_COMMAND = "whoami";
@@ -64,7 +67,7 @@ abstract public class Shell {
   /** a Unix command to get a given user's groups list */
   public static String[] getGroupsForUserCommand(final String user) {
     //'groups username' command return is non-consistent across different unixes
-    return (WINDOWS)? new String[] { WINUTILS, "groups", user}
+    return (WINDOWS)? new String[] { WINUTILS, "groups", "-F", "\"" + user + "\""}
                     : new String [] {"bash", "-c", "id -Gn " + user};
   }
 
@@ -77,7 +80,7 @@ abstract public class Shell {
 
   /** Return a command to get permission information. */
   public static String[] getGetPermissionCommand() {
-    return (WINDOWS) ? new String[] { WINUTILS, "ls" }
+    return (WINDOWS) ? new String[] { WINUTILS, "ls", "-F" }
                      : new String[] { "/bin/ls", "-ld" };
   }
 
@@ -110,10 +113,10 @@ abstract public class Shell {
 
   /** Return a command to set owner */
   public static String[] getSetOwnerCommand(String owner) {
-    return (WINDOWS) ? new String[] { WINUTILS, "chown", owner }
+    return (WINDOWS) ? new String[] { WINUTILS, "chown", "\"" + owner + "\"" }
                      : new String[] { "chown", owner };
   }
-
+  
   /** Return a command to create symbolic links */
   public static String[] getSymlinkCommand(String target, String link) {
     return WINDOWS ? new String[] { WINUTILS, "symlink", link, target }
@@ -163,6 +166,84 @@ abstract public class Shell {
   /** If or not script timed out*/
   private AtomicBoolean timedOut;
 
+
+  /** Centralized logic to discover and validate the sanity of the Hadoop 
+   *  home directory. Returns either NULL or a directory that exists and 
+   *  was specified via either -Dhadoop.home.dir or the HADOOP_HOME ENV 
+   *  variable.  This does a lot of work so it should only be called 
+   *  privately for initialization once per process.
+   **/
+  private static String checkHadoopHome() {
+
+    // first check the Dflag hadoop.home.dir with JVM scope
+    String home = System.getProperty("hadoop.home.dir");
+
+    // fall back to the system/user-global env variable
+    if (home == null) {
+      home = System.getenv("HADOOP_HOME");
+    }
+
+    try {
+       // couldn't find either setting for hadoop's home directory
+       if (home == null) {
+         throw new IOException("HADOOP_HOME or hadoop.home.dir are not set.");
+       }
+
+       if (home.startsWith("\"") && home.endsWith("\"")) {
+         home = home.substring(1, home.length()-1);
+       }
+
+       // check that the home setting is actually a directory that exists
+       File homedir = new File(home);
+       if (!homedir.isAbsolute() || !homedir.exists() || !homedir.isDirectory()) {
+         throw new IOException("Hadoop home directory " + homedir
+           + " does not exist, is not a directory, or is not an absolute path.");
+       }
+
+       home = homedir.getCanonicalPath();
+
+    } catch (IOException ioe) {
+       LOG.error("Failed to detect a valid hadoop home directory", ioe);
+       home = null;
+    }
+    
+    return home;
+  }
+  private static String HADOOP_HOME_DIR = checkHadoopHome();
+
+  // Public getter, throws an exception if HADOOP_HOME failed validation
+  // checks and is being referenced downstream.
+  public static final String getHadoopHome() throws IOException {
+    if (HADOOP_HOME_DIR == null) {
+      throw new IOException("Misconfigured HADOOP_HOME cannot be referenced.");
+    }
+
+    return HADOOP_HOME_DIR;
+  }
+
+  /** fully qualify the path to a binary that should be in a known hadoop 
+   *  bin location. This is primarily useful for disambiguating call-outs 
+   *  to executable sub-components of Hadoop to avoid clashes with other 
+   *  executables that may be in the path.  Caveat:  this call doesn't 
+   *  just format the path to the bin directory.  It also checks for file 
+   *  existence of the composed path. The output of this call should be 
+   *  cached by callers.
+   * */
+  public static final String getQualifiedBinPath(String executable) 
+  throws IOException {
+    // construct hadoop bin path to the specified executable
+    String fullExeName = HADOOP_HOME_DIR + File.separator + "bin" 
+      + File.separator + executable;
+
+    File exeFile = new File(fullExeName);
+    if (!exeFile.exists()) {
+      throw new IOException("Could not locate executable " + fullExeName
+        + " in the Hadoop binaries.");
+    }
+
+    return exeFile.getCanonicalPath();
+  }
+
   /** Set to true on Windows platforms */
   public static final boolean WINDOWS /* borrowed from Path.WINDOWS */
                 = System.getProperty("os.name").startsWith("Windows");
@@ -170,6 +251,28 @@ abstract public class Shell {
   public static final boolean LINUX
                 = System.getProperty("os.name").startsWith("Linux");
   
+  /** a Windows utility to emulate Unix commands */
+  public static final String WINUTILS = getWinUtilsPath();
+
+  public static final String getWinUtilsPath() {
+    String winUtilsPath = null;
+
+    try {
+      if (WINDOWS) {
+        winUtilsPath = getQualifiedBinPath("winutils.exe");
+      }
+    } catch (IOException ioe) {
+       LOG.error("Failed to locate the winutils binary in the hadoop binary path",
+         ioe);
+    }
+
+    return winUtilsPath;
+  }
+
+  /** Token separator regex used to parse Shell tool outputs */
+  public static final String TOKEN_SEPARATOR_REGEX
+                = WINDOWS ? "[|\n\r]" : "[ \t\n\r\f]";
+
   public static final boolean isSetsidAvailable = isSetsidSupported();
   private static boolean isSetsidSupported() {
     if (WINDOWS) {

hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/nativeio/NativeIO.c

@@ -811,46 +811,6 @@ cleanup:
 #endif
 }
 
-/*
- * Class:     org_apache_hadoop_io_nativeio_NativeIO_Windows
- * Method:    getLengthFollowSymlink
- * Signature: (Ljava/lang/String;)J
- *
- * The "00024" in the function name is an artifact of how JNI encodes
- * special characters. U+0024 is '$'.
- */
-JNIEXPORT jlong JNICALL
-Java_org_apache_hadoop_io_nativeio_NativeIO_00024Windows_getLengthFollowSymlink
-  (JNIEnv *env, jclass clazz, jstring j_path)
-{
-#ifdef UNIX
-  THROW(env, "java/io/IOException",
-    "The function getLengthFollowSymlink(String) is not supported on Unix");
-  return 0;
-#endif
-
-#ifdef WINDOWS
-  DWORD dwRtnCode = ERROR_SUCCESS;
-  BY_HANDLE_FILE_INFORMATION fileInfo = { 0 };
-  LARGE_INTEGER fileSize = { 0 };
-
-  const wchar_t *path = (const wchar_t*) (*env)->GetStringChars(env, j_path, NULL);
-  if (path == NULL) return 0; // JVM throws Exception for us
-
-  dwRtnCode = GetFileInformationByName(path, TRUE, &fileInfo);
-  if (dwRtnCode != ERROR_SUCCESS) {
-    throw_ioe(env, dwRtnCode);
-  }
-
-  (*env)->ReleaseStringChars(env, j_path, path);
-
-  fileSize.HighPart = fileInfo.nFileSizeHigh;
-  fileSize.LowPart = fileInfo.nFileSizeLow;
-
-  return (jlong)(fileSize.QuadPart);
-#endif
-}
-
 /**
  * vim: sw=2: ts=2: et:
  */

hadoop-common-project/hadoop-common/src/main/winutils/chmod.c

@@ -263,6 +263,7 @@ static BOOL ChangeFileModeRecursively(__in LPCWSTR path, __in_opt INT mode,
   do
   {
     LPWSTR filename = NULL;
+    LPWSTR longFilename = NULL;
     size_t filenameSize = 0;
 
     if (wcscmp(ffd.cFileName, L".") == 0 ||
@@ -285,13 +286,25 @@ static BOOL ChangeFileModeRecursively(__in LPCWSTR path, __in_opt INT mode,
       goto ChangeFileModeRecursivelyEnd;
     }
      
-    if(!ChangeFileModeRecursively(filename, mode, actions))
+    // The child fileanme is not prepended with long path prefix.
+    // Convert the filename to long path format.
+    //
+    dwRtnCode = ConvertToLongPath(filename, &longFilename);
+    LocalFree(filename);
+    if (dwRtnCode != ERROR_SUCCESS)
     {
-      LocalFree(filename);
+      ReportErrorCode(L"ConvertToLongPath", dwRtnCode);
+      LocalFree(longFilename);
       goto ChangeFileModeRecursivelyEnd;
     }
 
-    LocalFree(filename);
+    if(!ChangeFileModeRecursively(longFilename, mode, actions))
+    {
+      LocalFree(longFilename);
+      goto ChangeFileModeRecursivelyEnd;
+    }
+
+    LocalFree(longFilename);
 
   } while (FindNextFileW(hFind, &ffd));
 
@@ -877,4 +890,4 @@ Change the mode of the FILE to MODE.\n\
 \n\
 Each MODE is of the form '[ugoa]*([-+=]([rwxX]*|[ugo]))+'.\n",
 program, program);
-}
+}

hadoop-common-project/hadoop-common/src/main/winutils/chown.c

@@ -18,266 +18,92 @@
 #include "winutils.h"
 
 //----------------------------------------------------------------------------
-// Function: GetNewAclSize
+// Function: ChangeFileOwnerBySid
 //
 // Description:
-//	Compute the extra size of the new ACL if we replace the old owner Sid with
-//  the new owner Sid.
+//  Change a file or directory ownership by giving new owner and group SIDs
 //
 // Returns:
-//	The extra size needed for the new ACL compared with the ACL passed in. If
-//  the value is negative, it means the size of the new ACL could be reduced.
+//  ERROR_SUCCESS: on success
+//  Error code: otherwise
 //
 // Notes:
+//  This function is long path safe, i.e. the path will be converted to long
+//  path format if not already converted. So the caller does not need to do
+//  the converstion before calling the method.
 //
-static BOOL GetNewAclSizeDelta(__in PACL pDACL,
-  __in PSID pOldOwnerSid, __in PSID pNewOwnerSid, __out PLONG pDelta)
+static DWORD ChangeFileOwnerBySid(__in LPCWSTR path,
+  __in_opt PSID pNewOwnerSid, __in_opt PSID pNewGroupSid)
 {
-  PVOID pAce = NULL;
-  DWORD i;
-  PSID aceSid = NULL;
-  ACE_HEADER *aceHeader = NULL;
-  PACCESS_ALLOWED_ACE accessAllowedAce = NULL;
-  PACCESS_DENIED_ACE accessDenieddAce = NULL;
-
-  assert(pDACL != NULL && pNewOwnerSid != NULL &&
-    pOldOwnerSid != NULL && pDelta != NULL);
-
-  *pDelta = 0;
-  for (i = 0; i < pDACL->AceCount; i++)
-  {
-    if (!GetAce(pDACL, i, &pAce))
-    {
-      ReportErrorCode(L"GetAce", GetLastError());
-      return FALSE;
-    }
-
-    aceHeader = (ACE_HEADER *) pAce;
-    if (aceHeader->AceType == ACCESS_ALLOWED_ACE_TYPE)
-    {
-      accessAllowedAce = (PACCESS_ALLOWED_ACE) pAce;
-      aceSid = (PSID) &accessAllowedAce->SidStart;
-    }
-    else if (aceHeader->AceType == ACCESS_DENIED_ACE_TYPE)
-    {
-      accessDenieddAce = (PACCESS_DENIED_ACE) pAce;
-      aceSid = (PSID) &accessDenieddAce->SidStart;
-    }
-    else
-    {
-      continue;
-    }
+  LPWSTR longPathName = NULL;
+  INT oldMode = 0;
 
-    if (EqualSid(pOldOwnerSid, aceSid))
-    {
-      *pDelta += GetLengthSid(pNewOwnerSid) - GetLengthSid(pOldOwnerSid);
-    }
-  }
+  SECURITY_INFORMATION securityInformation = 0;
 
-  return TRUE;
-}
+  DWORD dwRtnCode = ERROR_SUCCESS;
 
-//----------------------------------------------------------------------------
-// Function: AddNewAce
-//
-// Description:
-//	Add an Ace of new owner to the new ACL
-//
-// Returns:
-//	TRUE: on success
-//
-// Notes:
-//  The Ace type should be either ACCESS_ALLOWED_ACE or ACCESS_DENIED_ACE
-//
-static BOOL AddNewAce(PACL pNewDACL, PVOID pOldAce,
-  PSID pOwnerSid, PSID pUserSid)
-{
-  PVOID pNewAce = NULL;
-  DWORD newAceSize = 0;
-
-  assert(pNewDACL != NULL && pOldAce != NULL &&
-    pOwnerSid != NULL && pUserSid != NULL);
-  assert(((PACE_HEADER)pOldAce)->AceType == ACCESS_ALLOWED_ACE_TYPE ||
-    ((PACE_HEADER)pOldAce)->AceType == ACCESS_DENIED_ACE_TYPE);
-
-  newAceSize =  ((PACE_HEADER)pOldAce)->AceSize +
-    GetLengthSid(pUserSid) - GetLengthSid(pOwnerSid);
-  pNewAce = LocalAlloc(LPTR, newAceSize);
-  if (pNewAce == NULL)
-  {
-    ReportErrorCode(L"LocalAlloc", GetLastError());
-    return FALSE;
-  }
-
-  ((PACE_HEADER)pNewAce)->AceType = ((PACE_HEADER) pOldAce)->AceType;
-  ((PACE_HEADER)pNewAce)->AceFlags = ((PACE_HEADER) pOldAce)->AceFlags;
-  ((PACE_HEADER)pNewAce)->AceSize = (WORD) newAceSize;
-
-  if (((PACE_HEADER)pOldAce)->AceType == ACCESS_ALLOWED_ACE_TYPE)
-  {
-    ((PACCESS_ALLOWED_ACE)pNewAce)->Mask = ((PACCESS_ALLOWED_ACE)pOldAce)->Mask;
-    if (!CopySid(GetLengthSid(pUserSid),
-      &((PACCESS_ALLOWED_ACE) pNewAce)->SidStart, pUserSid))
-    {
-      ReportErrorCode(L"CopySid", GetLastError());
-      LocalFree(pNewAce);
-      return FALSE;
-    }
-  }
-  else
-  {
-    ((PACCESS_DENIED_ACE)pNewAce)->Mask = ((PACCESS_DENIED_ACE)pOldAce)->Mask;
-    if (!CopySid(GetLengthSid(pUserSid),
-      &((PACCESS_DENIED_ACE) pNewAce)->SidStart, pUserSid))
-    {
-      ReportErrorCode(L"CopySid", GetLastError());
-      LocalFree(pNewAce);
-      return FALSE;
-    }
-  }
-
-  if (!AddAce(pNewDACL, ACL_REVISION, MAXDWORD,
-    pNewAce, ((PACE_HEADER)pNewAce)->AceSize))
+  // Convert the path the the long path
+  //
+  dwRtnCode = ConvertToLongPath(path, &longPathName);
+  if (dwRtnCode != ERROR_SUCCESS)
   {
-    ReportErrorCode(L"AddAce", GetLastError());
-    LocalFree(pNewAce);
-    return FALSE;
+    goto ChangeFileOwnerByNameEnd;
   }
 
-  LocalFree(pNewAce);
-  return TRUE;
-}
-
-//----------------------------------------------------------------------------
-// Function: CreateDaclForNewOwner
-//
-// Description:
-//	Create a new DACL for the new owner
-//
-// Returns:
-//	TRUE: on success
-//
-// Notes:
-//  Caller needs to destroy the memory of the new DACL by calling LocalFree()
-//
-static BOOL CreateDaclForNewOwner(
-  __in PACL pDACL,
-  __in_opt PSID pOldOwnerSid,
-  __in_opt PSID pNewOwnerSid,
-  __in_opt PSID pOldGroupSid,
-  __in_opt PSID pNewGroupSid,
-  __out PACL *ppNewDACL)
-{
-  PSID aceSid = NULL;
-  PACE_HEADER aceHeader = NULL;
-  PACCESS_ALLOWED_ACE accessAllowedAce = NULL;
-  PACCESS_DENIED_ACE accessDenieddAce = NULL;
-  PVOID pAce = NULL;
-  ACL_SIZE_INFORMATION aclSizeInfo;
-  LONG delta = 0;
-  DWORD dwNewAclSize = 0;
-  DWORD dwRtnCode = 0;
-  DWORD i;
-
-  assert(pDACL != NULL && ppNewDACL != NULL);
-  assert(pOldOwnerSid != NULL && pOldGroupSid != NULL);
-  assert(pNewOwnerSid != NULL || pNewGroupSid != NULL);
-
-  if (!GetAclInformation(pDACL, (LPVOID)&aclSizeInfo,
-    sizeof(ACL_SIZE_INFORMATION), AclSizeInformation))
+  // Get a pointer to the existing owner information and DACL
+  //
+  dwRtnCode = FindFileOwnerAndPermission(longPathName, NULL, NULL, &oldMode);
+  if (dwRtnCode != ERROR_SUCCESS)
   {
-    ReportErrorCode(L"GetAclInformation", GetLastError());
-    return FALSE;
+    goto ChangeFileOwnerByNameEnd;
   }
 
-  dwNewAclSize = aclSizeInfo.AclBytesInUse + aclSizeInfo.AclBytesFree;
-
-  delta = 0;
-  if (pNewOwnerSid != NULL &&
-    !GetNewAclSizeDelta(pDACL, pOldOwnerSid, pNewOwnerSid, &delta))
+  // We need SeTakeOwnershipPrivilege to set the owner if the caller does not
+  // have WRITE_OWNER access to the object; we need SeRestorePrivilege if the
+  // SID is not contained in the caller's token, and have the SE_GROUP_OWNER
+  // permission enabled.
+  //
+  if (!EnablePrivilege(L"SeTakeOwnershipPrivilege"))
   {
-    return FALSE;
+    fwprintf(stdout, L"INFO: The user does not have SeTakeOwnershipPrivilege.\n");
   }
-  dwNewAclSize += delta;
-
-  delta = 0;
-  if (pNewGroupSid != NULL &&
-    !GetNewAclSizeDelta(pDACL, pOldGroupSid, pNewGroupSid, &delta))
+  if (!EnablePrivilege(L"SeRestorePrivilege"))
   {
-    return FALSE;
+    fwprintf(stdout, L"INFO: The user does not have SeRestorePrivilege.\n");
   }
-  dwNewAclSize += delta;
 
-  *ppNewDACL = (PACL)LocalAlloc(LPTR, dwNewAclSize);
-  if (*ppNewDACL == NULL)
-  {
-    ReportErrorCode(L"LocalAlloc", GetLastError());
-    return FALSE;
-  }
+  assert(pNewOwnerSid != NULL || pNewGroupSid != NULL);
 
-  if (!InitializeAcl(*ppNewDACL, dwNewAclSize, ACL_REVISION))
+  // Set the owners of the file.
+  //
+  if (pNewOwnerSid != NULL) securityInformation |= OWNER_SECURITY_INFORMATION;
+  if (pNewGroupSid != NULL) securityInformation |= GROUP_SECURITY_INFORMATION;
+  dwRtnCode = SetNamedSecurityInfoW(
+    longPathName,
+    SE_FILE_OBJECT,
+    securityInformation,
+    pNewOwnerSid,
+    pNewGroupSid,
+    NULL,
+    NULL);
+  if (dwRtnCode != ERROR_SUCCESS)
   {
-    ReportErrorCode(L"InitializeAcl", GetLastError());
-    return FALSE;
+    goto ChangeFileOwnerByNameEnd;
   }
 
-  // Go through the DACL to change permissions
+  // Set the permission on the file for the new owner.
   //
-  for (i = 0; i < pDACL->AceCount; i++)
+  dwRtnCode = ChangeFileModeByMask(longPathName, oldMode);
+  if (dwRtnCode != ERROR_SUCCESS)
   {
-    if (!GetAce(pDACL, i, &pAce))
-    {
-      ReportErrorCode(L"GetAce", GetLastError());
-      return FALSE;
-    }
-
-    aceHeader = (PACE_HEADER) pAce;
-    aceSid = NULL;
-    if (aceHeader->AceType == ACCESS_ALLOWED_ACE_TYPE)
-    {
-      accessAllowedAce = (PACCESS_ALLOWED_ACE) pAce;
-      aceSid = (PSID) &accessAllowedAce->SidStart;
-    }
-    else if (aceHeader->AceType == ACCESS_DENIED_ACE_TYPE)
-    {
-      accessDenieddAce = (PACCESS_DENIED_ACE) pAce;
-      aceSid = (PSID) &accessDenieddAce->SidStart;
-    }
-
-    if (aceSid != NULL)
-    {
-      if (pNewOwnerSid != NULL && EqualSid(pOldOwnerSid, aceSid))
-      {
-        if (!AddNewAce(*ppNewDACL, pAce, pOldOwnerSid, pNewOwnerSid))
-          return FALSE;
-        else
-          continue;
-      }
-      else if (pNewGroupSid != NULL && EqualSid(pOldGroupSid, aceSid))
-      {
-        if (!AddNewAce(*ppNewDACL, pAce, pOldGroupSid, pNewGroupSid))
-          return FALSE;
-        else
-          continue;
-      }
-    }
-
-    // At this point, either:
-    // 1. The Ace is not of type ACCESS_ALLOWED_ACE or ACCESS_DENIED_ACE;
-    // 2. The Ace does not belong to the owner
-    // For both cases, we just add the oringal Ace to the new ACL.
-    //
-    if (!AddAce(*ppNewDACL, ACL_REVISION, MAXDWORD, pAce, aceHeader->AceSize))
-    {
-      ReportErrorCode(L"AddAce", dwRtnCode);
-      return FALSE;
-    }
+    goto ChangeFileOwnerByNameEnd;
   }
 
-  return TRUE;
+ChangeFileOwnerByNameEnd:
+  LocalFree(longPathName);
+  return dwRtnCode;
 }
 
-
 //----------------------------------------------------------------------------
 // Function: Chown
 //
@@ -293,7 +119,6 @@ static BOOL CreateDaclForNewOwner(
 int Chown(int argc, wchar_t *argv[])
 {
   LPWSTR pathName = NULL;
-  LPWSTR longPathName = NULL;
 
   LPWSTR ownerInfo = NULL;
 
@@ -308,16 +133,6 @@ int Chown(int argc, wchar_t *argv[])
   PSID pNewOwnerSid = NULL;
   PSID pNewGroupSid = NULL;
 
-  PACL pDACL = NULL;
-  PACL pNewDACL = NULL;
-
-  PSID pOldOwnerSid = NULL;
-  PSID pOldGroupSid = NULL;
-
-  PSECURITY_DESCRIPTOR pSD = NULL;
-
-  SECURITY_INFORMATION securityInformation;
-
   DWORD dwRtnCode = 0;
 
   int ret = EXIT_FAILURE;
@@ -353,7 +168,7 @@ int Chown(int argc, wchar_t *argv[])
         goto ChownEnd;
     }
 
-    if (colonPos + 1 != NULL)
+    if (*(colonPos + 1) != 0)
     {
       // Length includes NULL terminator
       groupNameLen = wcslen(ownerInfo) - (colonPos - ownerInfo) + 1;
@@ -382,10 +197,16 @@ int Chown(int argc, wchar_t *argv[])
       goto ChownEnd;
   }
 
-  if ((userName == NULL || wcslen(userName) == 0) &&
-    (groupName == NULL || wcslen(groupName) == 0))
+  // Not allow zero length user name or group name in the parsing results.
+  //
+  assert(userName == NULL || wcslen(userName) > 0);
+  assert(groupName == NULL || wcslen(groupName) > 0);
+
+  // Nothing to change if both names are empty
+  //
+  if ((userName == NULL) && (groupName == NULL))
   {
-    fwprintf(stderr, L"User name and group name cannot both be empty.");
+    ret = EXIT_SUCCESS;
     goto ChownEnd;
   }
 
@@ -417,61 +238,10 @@ int Chown(int argc, wchar_t *argv[])
     goto ChownEnd;
   }
 
-  // Convert the path the the long path
-  //
-  dwRtnCode = ConvertToLongPath(pathName, &longPathName);
-  if (dwRtnCode != ERROR_SUCCESS)
-  {
-    ReportErrorCode(L"ConvertToLongPath", dwRtnCode);
-    goto ChownEnd;
-  }
-
-  // Get a pointer to the existing owner information and DACL
-  //
-  dwRtnCode = GetNamedSecurityInfoW(
-    longPathName,
-    SE_FILE_OBJECT, 
-    OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION |
-    DACL_SECURITY_INFORMATION,
-    &pOldOwnerSid,
-    &pOldGroupSid,
-    &pDACL,
-    NULL,
-    &pSD);
-  if (dwRtnCode != ERROR_SUCCESS)
-  {
-    ReportErrorCode(L"GetNamedSecurityInfo", dwRtnCode);
-    goto ChownEnd;
-  }
-
-  // Create the new DACL
-  //
-  if (!CreateDaclForNewOwner(pDACL,
-    pOldOwnerSid, pNewOwnerSid,
-    pOldGroupSid, pNewGroupSid,
-    &pNewDACL))
-  {
-    goto ChownEnd;
-  }
-
-  // Set the owner and DACLs in the object's security descriptor. Use
-  // PROTECTED_DACL_SECURITY_INFORMATION flag to remove permission inheritance
-  //
-  securityInformation =
-    DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION;
-  if (pNewOwnerSid != NULL) securityInformation |= OWNER_SECURITY_INFORMATION;
-  if (pNewGroupSid != NULL) securityInformation |= GROUP_SECURITY_INFORMATION;
-  dwRtnCode = SetNamedSecurityInfoW(
-    longPathName,
-    SE_FILE_OBJECT,
-    securityInformation,
-    pNewOwnerSid,
-    pNewGroupSid,
-    pNewDACL,
-    NULL);
+  dwRtnCode = ChangeFileOwnerBySid(pathName, pNewOwnerSid, pNewGroupSid);
   if (dwRtnCode != ERROR_SUCCESS)
   {
-    ReportErrorCode(L"SetNamedSecurityInfo", dwRtnCode);
+    ReportErrorCode(L"ChangeFileOwnerBySid", dwRtnCode);
     goto ChownEnd;
   }
 
@@ -482,9 +252,6 @@ ChownEnd:
   LocalFree(groupName);
   LocalFree(pNewOwnerSid);
   LocalFree(pNewGroupSid);
-  LocalFree(pNewDACL);
-  LocalFree(pSD);
-  LocalFree(longPathName);
 
   return ret;
 }
@@ -493,6 +260,11 @@ void ChownUsage(LPCWSTR program)
 {
   fwprintf(stdout, L"\
 Usage: %s [OWNER][:[GROUP]] [FILE]\n\
-Change the owner and/or group of the FILE to OWNER and/or GROUP.\n",
+Change the owner and/or group of the FILE to OWNER and/or GROUP.\n\
+\n\
+Note:\n\
+On Linux, if a colon but no group name follows the user name, the group of\n\
+the files is changed to that user\'s login group. Windows has no concept of\n\
+a user's login group. So we do not change the group owner in this case.\n",
 program);
 }

hadoop-common-project/hadoop-common/src/main/winutils/groups.c

@@ -28,8 +28,13 @@
 // Notes:
 //   This function could fail on first pass when we fail to find groups for
 //   domain account; so we do not report Windows API errors in this function.
+//   If formatOutput is true, pipe character is used as separator for groups
+//   otherwise, space.
 //
-static BOOL PrintGroups(LPLOCALGROUP_USERS_INFO_0 groups, DWORD entries)
+static BOOL PrintGroups(
+  LPLOCALGROUP_USERS_INFO_0 groups,
+  DWORD entries,
+  BOOL formatOutput)
 {
   BOOL ret = TRUE;
   LPLOCALGROUP_USERS_INFO_0 pTmpBuf = groups;
@@ -45,7 +50,14 @@ static BOOL PrintGroups(LPLOCALGROUP_USERS_INFO_0 groups, DWORD entries)
 
     if (i != 0)
     {
-      wprintf(L" ");
+      if (formatOutput)
+      {
+        wprintf(L"|");
+      }
+      else
+      {
+        wprintf(L" ");
+      }
     }
     wprintf(L"%s", pTmpBuf->lgrui0_name);
 
@@ -58,6 +70,56 @@ static BOOL PrintGroups(LPLOCALGROUP_USERS_INFO_0 groups, DWORD entries)
   return ret;
 }
 
+//----------------------------------------------------------------------------
+// Function: ParseCommandLine
+//
+// Description:
+//   Parses the command line
+//
+// Returns:
+//   TRUE on the valid command line, FALSE otherwise
+//
+static BOOL ParseCommandLine(
+  int argc, wchar_t *argv[], wchar_t **user, BOOL *formatOutput)
+{
+  *formatOutput = FALSE;
+
+  assert(argv != NULL);
+  assert(user != NULL);
+
+  if (argc == 1)
+  {
+    // implicitly use the current user
+    *user = NULL;
+    return TRUE;
+  }
+  else if (argc == 2)
+  {
+    // check if the second argument is formating
+    if (wcscmp(argv[1], L"-F") == 0)
+    {
+      *user = NULL;
+      *formatOutput = TRUE;
+      return TRUE;
+    }
+    else
+    {
+      *user = argv[1];
+      return TRUE;
+    }
+  }
+  else if (argc == 3 && wcscmp(argv[1], L"-F") == 0)
+  {
+    // if 3 args, the second argument must be "-F"
+
+    *user = argv[2];
+    *formatOutput = TRUE;
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
 //----------------------------------------------------------------------------
 // Function: Groups
 //
@@ -83,15 +145,18 @@ int Groups(int argc, wchar_t *argv[])
   DWORD dwRtnCode = ERROR_SUCCESS;
 
   int ret = EXIT_SUCCESS;
+  BOOL formatOutput = FALSE;
 
-  if (argc != 2 && argc != 1)
+  if (!ParseCommandLine(argc, argv, &input, &formatOutput))
   {
     fwprintf(stderr, L"Incorrect command line arguments.\n\n");
     GroupsUsage(argv[0]);
     return EXIT_FAILURE;
   }
 
-  if (argc == 1)
+  // if username was not specified on the command line, fallback to the
+  // current user
+  if (input == NULL)
   {
     GetUserNameW(currentUser, &cchCurrentUser);
     if (GetLastError() == ERROR_INSUFFICIENT_BUFFER)
@@ -120,10 +185,6 @@ int Groups(int argc, wchar_t *argv[])
       goto GroupsEnd;
     }
   }
-  else
-  {
-    input = argv[1];
-  }
 
   if ((dwRtnCode = GetLocalGroupsForUser(input, &groups, &entries))
     != ERROR_SUCCESS)
@@ -133,7 +194,7 @@ int Groups(int argc, wchar_t *argv[])
     goto GroupsEnd;
   }
 
-  if (!PrintGroups(groups, entries))
+  if (!PrintGroups(groups, entries, formatOutput))
   {
     ret = EXIT_FAILURE;
   }
@@ -147,8 +208,10 @@ GroupsEnd:
 void GroupsUsage(LPCWSTR program)
 {
   fwprintf(stdout, L"\
-Usage: %s [USERNAME]\n\
-Print group information of the specified USERNAME\
-(the current user by default).\n",
+Usage: %s [OPTIONS] [USERNAME]\n\
+Print group information of the specified USERNAME \
+(the current user by default).\n\
+\n\
+OPTIONS: -F format the output by separating tokens with a pipe\n",
 program);
 }

hadoop-common-project/hadoop-common/src/main/winutils/include/winutils.h

@@ -137,4 +137,6 @@ DWORD JunctionPointCheck(__in LPCWSTR pathName, __out LPBOOL result);
 DWORD ChangeFileModeByMask(__in LPCWSTR path, INT mode);
 
 DWORD GetLocalGroupsForUser(__in LPCWSTR user,
-  __out LPLOCALGROUP_USERS_INFO_0 *groups, __out LPDWORD entries);
+  __out LPLOCALGROUP_USERS_INFO_0 *groups, __out LPDWORD entries);
+
+BOOL EnablePrivilege(__in LPCWSTR privilegeName);

hadoop-common-project/hadoop-common/src/main/winutils/libwinutils.c

@@ -144,7 +144,7 @@ static BOOL IsLongWindowsPath(__in PCWSTR path)
 static BOOL IsPrefixedAlready(__in PCWSTR path)
 {
   static const PCWSTR LongPathPrefix = L"\\\\?\\";
-  int Prefixlen = (int)wcslen(LongPathPrefix);
+  size_t Prefixlen = wcslen(LongPathPrefix);
   size_t i = 0;
 
   if (path == NULL || wcslen(path) < Prefixlen)
@@ -413,10 +413,13 @@ DWORD GetSidFromAcctNameW(LPCWSTR acctName, PSID *ppSid)
   assert (acctName != NULL && ppSid != NULL);
 
   // Empty name is invalid. However, LookupAccountName() function will return a
-  // false Sid for an empty name instead failing.
+  // false Sid, i.e. Sid for 'BUILDIN', for an empty name instead failing. We
+  // report the error before calling LookupAccountName() function for this
+  // special case. The error code returned here is the same as the last error
+  // code set by LookupAccountName() function for an invalid name.
   //
   if (wcslen(acctName) == 0)
-    return FALSE;
+    return ERROR_NONE_MAPPED;
 
   // First pass to retrieve the buffer size.
   //
@@ -478,7 +481,7 @@ DWORD GetSidFromAcctNameW(LPCWSTR acctName, PSID *ppSid)
 //	Compute the 3 bit Unix mask for the owner, group, or, others
 //
 // Returns:
-//	The 3 bit Unix mask in USHORT
+//	The 3 bit Unix mask in INT
 //
 // Notes:
 //
@@ -613,8 +616,11 @@ GetEffectiveRightsForSidEnd:
 //  Error code otherwise
 //
 // Notes:
-//  Caller needs to destroy the memeory of owner and group names by calling
-//  LocalFree() function.
+//  - Caller needs to destroy the memeory of owner and group names by calling
+//    LocalFree() function.
+//
+//  - If the user or group name does not exist, the user or group SID will be
+//    returned as the name.
 //
 DWORD FindFileOwnerAndPermission(
   __in LPCWSTR pathName,
@@ -830,7 +836,17 @@ static void GetWindowsAccessMask(INT unixMask,
 //  Error code: otherwise
 //
 // Notes:
-//  none
+//  - Administrators and SYSTEM are always given full permission to the file,
+//    unless Administrators or SYSTEM itself is the file owner and the user
+//    explictly set the permission to something else. For example, file 'foo'
+//    belongs to Administrators, 'chmod 000' on the file will not directly
+//    assign Administrators full permission on the file.
+//  - Only full permission for Administrators and SYSTEM are inheritable.
+//  - CREATOR OWNER is always given full permission and the permission is
+//    inheritable, more specifically OBJECT_INHERIT_ACE, CONTAINER_INHERIT_ACE
+//    flags are set. The reason is to give the creator of child file full
+//    permission, i.e., the child file will have permission mode 700 for
+//    a user other than Administrator or SYSTEM.
 //
 static DWORD GetWindowsDACLs(__in INT unixMask,
   __in PSID pOwnerSid, __in PSID pGroupSid, __out PACL *ppNewDACL)
@@ -842,12 +858,22 @@ static DWORD GetWindowsDACLs(__in INT unixMask,
   DWORD winOtherAccessAllowMask;
 
   PSID pEveryoneSid = NULL;
+  DWORD cbEveryoneSidSize = SECURITY_MAX_SID_SIZE;
+
+  PSID pSystemSid = NULL;
+  DWORD cbSystemSidSize = SECURITY_MAX_SID_SIZE;
+  BOOL bAddSystemAcls = FALSE;
+
+  PSID pAdministratorsSid = NULL;
+  DWORD cbAdministratorsSidSize = SECURITY_MAX_SID_SIZE;
+  BOOL bAddAdministratorsAcls = FALSE;
+
+  PSID pCreatorOwnerSid = NULL;
+  DWORD cbCreatorOwnerSidSize = SECURITY_MAX_SID_SIZE;
 
   PACL pNewDACL = NULL;
   DWORD dwNewAclSize = 0;
 
-  SID_IDENTIFIER_AUTHORITY SIDAuthWorld = SECURITY_WORLD_SID_AUTHORITY;
-
   DWORD ret = ERROR_SUCCESS;
 
   GetWindowsAccessMask(unixMask,
@@ -857,12 +883,63 @@ static DWORD GetWindowsDACLs(__in INT unixMask,
 
   // Create a well-known SID for the Everyone group
   //
-  if(!AllocateAndInitializeSid(&SIDAuthWorld, 1,
-    SECURITY_WORLD_RID,
-    0, 0, 0, 0, 0, 0, 0,
-    &pEveryoneSid))
+  if ((pEveryoneSid = LocalAlloc(LPTR, cbEveryoneSidSize)) == NULL)
   {
-    return GetLastError();
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+  if (!CreateWellKnownSid(WinWorldSid, NULL, pEveryoneSid, &cbEveryoneSidSize))
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+
+  // Create a well-known SID for the Administrators group
+  //
+  if ((pAdministratorsSid = LocalAlloc(LPTR, cbAdministratorsSidSize)) == NULL)
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+  if (!CreateWellKnownSid(WinBuiltinAdministratorsSid, NULL,
+    pAdministratorsSid, &cbAdministratorsSidSize))
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+  if (!EqualSid(pAdministratorsSid, pOwnerSid)
+    && !EqualSid(pAdministratorsSid, pGroupSid))
+    bAddAdministratorsAcls = TRUE;
+
+  // Create a well-known SID for the SYSTEM
+  //
+  if ((pSystemSid = LocalAlloc(LPTR, cbSystemSidSize)) == NULL)
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+  if (!CreateWellKnownSid(WinLocalSystemSid, NULL,
+    pSystemSid, &cbSystemSidSize))
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+  if (!EqualSid(pSystemSid, pOwnerSid)
+    && !EqualSid(pSystemSid, pGroupSid))
+    bAddSystemAcls = TRUE;
+
+  // Create a well-known SID for the Creator Owner
+  //
+  if ((pCreatorOwnerSid = LocalAlloc(LPTR, cbCreatorOwnerSidSize)) == NULL)
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+  if (!CreateWellKnownSid(WinCreatorOwnerSid, NULL,
+    pCreatorOwnerSid, &cbCreatorOwnerSidSize))
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
   }
 
   // Create the new DACL
@@ -880,6 +957,22 @@ static DWORD GetWindowsDACLs(__in INT unixMask,
     GetLengthSid(pGroupSid) - sizeof(DWORD);
   dwNewAclSize += sizeof(ACCESS_ALLOWED_ACE) +
     GetLengthSid(pEveryoneSid) - sizeof(DWORD);
+
+  if (bAddSystemAcls)
+  {
+    dwNewAclSize += sizeof(ACCESS_ALLOWED_ACE) +
+      cbSystemSidSize - sizeof(DWORD);
+  }
+
+  if (bAddAdministratorsAcls)
+  {
+    dwNewAclSize += sizeof(ACCESS_ALLOWED_ACE) +
+      cbAdministratorsSidSize - sizeof(DWORD);
+  }
+
+  dwNewAclSize += sizeof(ACCESS_ALLOWED_ACE) +
+    cbCreatorOwnerSidSize - sizeof(DWORD);
+
   pNewDACL = (PACL)LocalAlloc(LPTR, dwNewAclSize);
   if (pNewDACL == NULL)
   {
@@ -892,33 +985,64 @@ static DWORD GetWindowsDACLs(__in INT unixMask,
     goto GetWindowsDACLsEnd;
   }
 
+  if (!AddAccessAllowedAceEx(pNewDACL, ACL_REVISION,
+    CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE,
+    GENERIC_ALL, pCreatorOwnerSid))
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+
+  if (bAddSystemAcls &&
+    !AddAccessAllowedAceEx(pNewDACL, ACL_REVISION,
+    CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE,
+    GENERIC_ALL, pSystemSid))
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+
+  if (bAddAdministratorsAcls &&
+    !AddAccessAllowedAceEx(pNewDACL, ACL_REVISION,
+    CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE,
+    GENERIC_ALL, pAdministratorsSid))
+  {
+    ret = GetLastError();
+    goto GetWindowsDACLsEnd;
+  }
+
   if (winUserAccessDenyMask &&
-    !AddAccessDeniedAce(pNewDACL, ACL_REVISION,
+    !AddAccessDeniedAceEx(pNewDACL, ACL_REVISION,
+    NO_PROPAGATE_INHERIT_ACE,
     winUserAccessDenyMask, pOwnerSid))
   {
     ret = GetLastError();
     goto GetWindowsDACLsEnd;
   }
-  if (!AddAccessAllowedAce(pNewDACL, ACL_REVISION,
+  if (!AddAccessAllowedAceEx(pNewDACL, ACL_REVISION,
+    NO_PROPAGATE_INHERIT_ACE,
     winUserAccessAllowMask, pOwnerSid))
   {
     ret = GetLastError();
     goto GetWindowsDACLsEnd;
   }
   if (winGroupAccessDenyMask &&
-    !AddAccessDeniedAce(pNewDACL, ACL_REVISION,
+    !AddAccessDeniedAceEx(pNewDACL, ACL_REVISION,
+    NO_PROPAGATE_INHERIT_ACE,
     winGroupAccessDenyMask, pGroupSid))
   {
     ret = GetLastError();
     goto GetWindowsDACLsEnd;
   }
-  if (!AddAccessAllowedAce(pNewDACL, ACL_REVISION,
+  if (!AddAccessAllowedAceEx(pNewDACL, ACL_REVISION,
+    NO_PROPAGATE_INHERIT_ACE,
     winGroupAccessAllowMask, pGroupSid))
   {
     ret = GetLastError();
     goto GetWindowsDACLsEnd;
   }
-  if (!AddAccessAllowedAce(pNewDACL, ACL_REVISION,
+  if (!AddAccessAllowedAceEx(pNewDACL, ACL_REVISION,
+    NO_PROPAGATE_INHERIT_ACE,
     winOtherAccessAllowMask, pEveryoneSid))
   {
     ret = GetLastError();
@@ -928,7 +1052,10 @@ static DWORD GetWindowsDACLs(__in INT unixMask,
   *ppNewDACL = pNewDACL;
 
 GetWindowsDACLsEnd:
-  if (pEveryoneSid) FreeSid(pEveryoneSid);
+  LocalFree(pEveryoneSid);
+  LocalFree(pAdministratorsSid);
+  LocalFree(pSystemSid);
+  LocalFree(pCreatorOwnerSid);
   if (ret != ERROR_SUCCESS) LocalFree(pNewDACL);
   
   return ret;
@@ -952,7 +1079,6 @@ GetWindowsDACLsEnd:
 DWORD ChangeFileModeByMask(__in LPCWSTR path, INT mode)
 {
   LPWSTR longPathName = NULL;
-  PACL pOldDACL = NULL;
   PACL pNewDACL = NULL;
   PSID pOwnerSid = NULL;
   PSID pGroupSid = NULL;
@@ -984,11 +1110,10 @@ DWORD ChangeFileModeByMask(__in LPCWSTR path, INT mode)
   dwRtnCode = GetNamedSecurityInfoW(
     longPathName,
     SE_FILE_OBJECT, 
-    OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION |
-    DACL_SECURITY_INFORMATION,
+    OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION,
     &pOwnerSid,
     &pGroupSid,
-    &pOldDACL,
+    NULL,
     NULL,
     &pSD);
   if (ERROR_SUCCESS != dwRtnCode)
@@ -1221,12 +1346,15 @@ GetAccntNameFromSidEnd:
 //  Other error code on failure
 //
 // Notes:
-// - Because NetUserGetLocalGroups() only accepts full user name, we wil try
-//   to find full user name given the user input if NetUserGetLocalGroups()
-//   fails on first time with error code NERR_UserNotFound. It is not always
-//   to find full user name given only user name. For example, a computer named
-//   'win1' joint domain 'redmond' can have both 'win1\alex' and
-//   'redmond\alex'. Given only alex, we cannot tell which one is correct.
+// - NetUserGetLocalGroups() function only accepts full user name in the format
+//   [domain name]\[username]. The user input to this function can be only the
+//   username. In this case, NetUserGetLocalGroups() will fail on the first try,
+//   and we will try to find full user name using LookupAccountNameW() method,
+//   and call NetUserGetLocalGroups() function again with full user name.
+//   However, it is not always possible to find full user name given only user
+//   name. For example, a computer named 'win1' joined domain 'redmond' can have
+//   two different users, 'win1\alex' and 'redmond\alex'. Given only 'alex', we
+//   cannot tell which one is correct.
 //
 // - Caller needs to destroy the memory of groups by using the
 //   NetApiBufferFree() function
@@ -1306,6 +1434,51 @@ GetLocalGroupsForUserEnd:
   return ret;
 }
 
+//----------------------------------------------------------------------------
+// Function: EnablePrivilege
+//
+// Description:
+//	Check if the process has the given privilege. If yes, enable the privilege
+//  to the process's access token.
+//
+// Returns:
+//	TRUE: on success
+//
+// Notes:
+//
+BOOL EnablePrivilege(__in LPCWSTR privilegeName)
+{
+  HANDLE hToken = INVALID_HANDLE_VALUE;
+  TOKEN_PRIVILEGES tp = { 0 };
+  DWORD dwErrCode = ERROR_SUCCESS;
+
+  if (!OpenProcessToken(GetCurrentProcess(),
+    TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, &hToken))
+  {
+    ReportErrorCode(L"OpenProcessToken", GetLastError());
+    return FALSE;
+  }
+
+  tp.PrivilegeCount = 1;
+  if (!LookupPrivilegeValueW(NULL,
+    privilegeName, &(tp.Privileges[0].Luid)))
+  {
+    ReportErrorCode(L"LookupPrivilegeValue", GetLastError());
+    CloseHandle(hToken);
+    return FALSE;
+  }
+  tp.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
+
+  // As stated on MSDN, we need to use GetLastError() to check if
+  // AdjustTokenPrivileges() adjusted all of the specified privileges.
+  //
+  AdjustTokenPrivileges(hToken, FALSE, &tp, 0, NULL, NULL);
+  dwErrCode = GetLastError();
+  CloseHandle(hToken);
+
+  return dwErrCode == ERROR_SUCCESS;
+}
+
 //----------------------------------------------------------------------------
 // Function: ReportErrorCode
 //

hadoop-common-project/hadoop-common/src/main/winutils/ls.c

@@ -76,6 +76,8 @@ static BOOL GetMaskString(INT accessMask, LPWSTR maskString)
 //	None
 //
 // Notes:
+//  if useSeparator is false, separates the output tokens with a space
+//  character, otherwise, with a pipe character
 //
 static BOOL LsPrintLine(
   const INT mask,
@@ -84,7 +86,8 @@ static BOOL LsPrintLine(
   LPCWSTR groupName,
   const FILETIME *lpFileWritetime,
   const LARGE_INTEGER fileSize,
-  LPCWSTR path)
+  LPCWSTR path,
+  BOOL useSeparator)
 {
   // 'd' + 'rwx' for user, group, other
   static const size_t ck_ullMaskLen = 1 + 3 * 3;
@@ -117,10 +120,20 @@ static BOOL LsPrintLine(
     goto LsPrintLineEnd;
   }
 
-  fwprintf(stdout, L"%10s %d %s %s %lld %3s %2d %4d %s\n",
-    maskString, hardlinkCount, ownerName, groupName, fileSize.QuadPart,
-    MONTHS[stFileWriteTime.wMonth-1], stFileWriteTime.wDay,
-    stFileWriteTime.wYear, path);
+  if (useSeparator)
+  {
+    fwprintf(stdout, L"%10s|%d|%s|%s|%lld|%3s|%2d|%4d|%s\n",
+      maskString, hardlinkCount, ownerName, groupName, fileSize.QuadPart,
+      MONTHS[stFileWriteTime.wMonth-1], stFileWriteTime.wDay,
+      stFileWriteTime.wYear, path);
+  }
+  else
+  {
+    fwprintf(stdout, L"%10s %d %s %s %lld %3s %2d %4d %s\n",
+      maskString, hardlinkCount, ownerName, groupName, fileSize.QuadPart,
+      MONTHS[stFileWriteTime.wMonth-1], stFileWriteTime.wDay,
+      stFileWriteTime.wYear, path);
+  }
 
   ret = TRUE;
 
@@ -130,6 +143,88 @@ LsPrintLineEnd:
   return ret;
 }
 
+// List of command line options supported by "winutils ls"
+enum CmdLineOption
+{
+  CmdLineOptionFollowSymlink = 0x1,  // "-L"
+  CmdLineOptionSeparator = 0x2  // "-F"
+  // options should be powers of 2 (aka next is 0x4)
+};
+
+static wchar_t* CurrentDir = L".";
+
+//----------------------------------------------------------------------------
+// Function: ParseCommandLine
+//
+// Description:
+//   Parses the command line
+//
+// Returns:
+//   TRUE on the valid command line, FALSE otherwise
+//
+BOOL ParseCommandLine(
+  int argc, wchar_t *argv[], wchar_t** path, int *optionsMask)
+{
+  int MaxOptions = 2; // Should be equal to the number of elems in CmdLineOption
+  int i = 0;
+
+  assert(optionsMask != NULL);
+  assert(argv != NULL);
+  assert(path != NULL);
+
+  *optionsMask = 0;
+
+  if (argc == 1)
+  {
+    // no path specified, assume "."
+    *path = CurrentDir;
+    return TRUE;
+  }
+
+  if (argc == 2)
+  {
+    // only path specified, no other options
+    *path = argv[1];
+    return TRUE;
+  }
+
+  if (argc > 2 + MaxOptions)
+  {
+    // too many parameters
+    return FALSE;
+  }
+
+  for (i = 1; i < argc - 1; ++i)
+  {
+    if (wcscmp(argv[i], L"-L") == 0)
+    {
+      // Check if this option was already specified
+      BOOL alreadySet = *optionsMask & CmdLineOptionFollowSymlink;
+      if (alreadySet)
+        return FALSE;
+
+      *optionsMask |= CmdLineOptionFollowSymlink;
+    }
+    else if (wcscmp(argv[i], L"-F") == 0)
+    {
+      // Check if this option was already specified
+      BOOL alreadySet = *optionsMask & CmdLineOptionSeparator;
+      if (alreadySet)
+        return FALSE;
+
+      *optionsMask |= CmdLineOptionSeparator;
+    }
+    else
+    {
+      return FALSE;
+    }
+  }
+
+  *path = argv[argc - 1];
+
+  return TRUE;
+}
+
 //----------------------------------------------------------------------------
 // Function: Ls
 //
@@ -158,19 +253,16 @@ int Ls(int argc, wchar_t *argv[])
   BOOL isSymlink = FALSE;
 
   int ret = EXIT_FAILURE;
+  int optionsMask = 0;
 
-  if (argc > 2)
+  if (!ParseCommandLine(argc, argv, &pathName, &optionsMask))
   {
     fwprintf(stderr, L"Incorrect command line arguments.\n\n");
     LsUsage(argv[0]);
     return EXIT_FAILURE;
   }
 
-  if (argc == 2)
-    pathName = argv[1];
-
-  if (pathName == NULL || wcslen(pathName) == 0)
-    pathName = L".";
+  assert(pathName != NULL);
 
   if (wcsspn(pathName, L"/?|><:*\"") != 0)
   {
@@ -187,14 +279,15 @@ int Ls(int argc, wchar_t *argv[])
     goto LsEnd;
   }
 
-  dwErrorCode = GetFileInformationByName(longPathName, FALSE, &fileInformation);
+  dwErrorCode = GetFileInformationByName(
+    longPathName, optionsMask & CmdLineOptionFollowSymlink, &fileInformation);
   if (dwErrorCode != ERROR_SUCCESS)
   {
     ReportErrorCode(L"GetFileInformationByName", dwErrorCode);
     goto LsEnd;
   }
 
-  dwErrorCode = SymbolicLinkCheck(pathName, &isSymlink);
+  dwErrorCode = SymbolicLinkCheck(longPathName, &isSymlink);
   if (dwErrorCode != ERROR_SUCCESS)
   {
      ReportErrorCode(L"IsSymbolicLink", dwErrorCode);
@@ -224,7 +317,8 @@ int Ls(int argc, wchar_t *argv[])
     ownerName, groupName,
     &fileInformation.ftLastWriteTime,
     fileSize,
-    pathName))
+    pathName,
+    optionsMask & CmdLineOptionSeparator))
     goto LsEnd;
 
   ret = EXIT_SUCCESS;
@@ -240,10 +334,13 @@ LsEnd:
 void LsUsage(LPCWSTR program)
 {
   fwprintf(stdout, L"\
-Usage: %s [FILE]\n\
+Usage: %s [OPTIONS] [FILE]\n\
 List information about the FILE (the current directory by default).\n\
 Using long listing format and list directory entries instead of contents,\n\
 and do not dereference symbolic links.\n\
-Provide equivalent or similar function as 'ls -ld' on GNU/Linux.\n",
+Provides equivalent or similar function as 'ls -ld' on GNU/Linux.\n\
+\n\
+OPTIONS: -L dereference symbolic links\n\
+         -F format the output by separating tokens with a pipe\n",
 program);
 }

hadoop-common-project/hadoop-common/src/main/winutils/main.c

@@ -110,10 +110,9 @@ The available commands and their usages are:\n\n", program);
 
   fwprintf(stdout, L"%-15s%s\n\n", L"systeminfo", L"System information.");
   SystemInfoUsage();
+  fwprintf(stdout, L"\n\n");
 
   fwprintf(stdout, L"%-15s%s\n\n", L"task", L"Task operations.");
   TaskUsage();
   fwprintf(stdout, L"\n\n");
-
-  fwprintf(stdout, L"\n\n");
 }

hadoop-common-project/hadoop-common/src/main/winutils/symlink.c

@@ -17,51 +17,6 @@
 
 #include "winutils.h"
 
-//----------------------------------------------------------------------------
-// Function: EnablePrivilege
-//
-// Description:
-//	Check if the process has the given privilege. If yes, enable the privilege
-//  to the process's access token.
-//
-// Returns:
-//	TRUE: on success
-//
-// Notes:
-//
-static BOOL EnablePrivilege(__in LPCWSTR privilegeName)
-{
-  HANDLE hToken;
-  TOKEN_PRIVILEGES tp;
-  DWORD dwErrCode;
-
-  if (!OpenProcessToken(GetCurrentProcess(),
-    TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY, &hToken))
-  {
-    ReportErrorCode(L"OpenProcessToken", GetLastError());
-    return FALSE;
-  }
-
-  tp.PrivilegeCount = 1;
-  if (!LookupPrivilegeValueW(NULL,
-    privilegeName, &(tp.Privileges[0].Luid)))
-  {
-    ReportErrorCode(L"LookupPrivilegeValue", GetLastError());
-    CloseHandle(hToken);
-    return FALSE;
-  }
-  tp.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
-
-  // As stated on MSDN, we need to use GetLastError() to check if
-  // AdjustTokenPrivileges() adjusted all of the specified privileges.
-  //
-  AdjustTokenPrivileges(hToken, FALSE, &tp, 0, NULL, NULL);
-  dwErrCode = GetLastError();
-  CloseHandle(hToken);
-
-  return dwErrCode == ERROR_SUCCESS;
-}
-
 //----------------------------------------------------------------------------
 // Function: Symlink
 //

hadoop-common-project/hadoop-common/src/main/winutils/tests/test-all.bat

@@ -1,19 +0,0 @@
-:: 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.
-::
-@echo off
-call test-winutils-basics
-call test-winutils-chown
-call test-winutils-chmod

hadoop-common-project/hadoop-common/src/main/winutils/tests/test-winutils-basics.bat

@@ -1,95 +0,0 @@
-:: 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.
-::
-@echo off
-echo Test most basic security settings are working
-setlocal
-set WINUTILS="%HADOOP_HOME%\bin\winutils.exe"
-set TESTDIR=winutils-test
-
-:: Setup test directory
-::
-if not exist %TESTDIR% md %TESTDIR%
-pushd %TESTDIR%
-
-:: Test cases
-::
-echo Test case 1:
-::  - Create a file.
-::  - Change mode to 377 so owner does not have read permission.
-::  - Verify the owner truly does not have the permissions to read.
-if exist a goto Failure
-type NUL>a
-%WINUTILS% chmod 377 a
-if not %ERRORLEVEL% == 0 goto Failure
-type a
-if %ERRORLEVEL% == 0 goto Failure
-del a
-if not %ERRORLEVEL% == 0 goto Failure
-echo passed.
-
-echo Test case 2:
-::  - Create a file.
-::  - Change mode to 577 so owner does not have write permission.
-::  - Verify the owner truly does not have the permissions to write.
-if exist a goto Failure
-type NUL>a
-%WINUTILS% chmod 577 a
-if not %ERRORLEVEL% == 0 goto Failure
-cmd /C "echo a>>a"
-if %ERRORLEVEL% == 0 goto Failure
-del a
-if not %ERRORLEVEL% == 0 goto Failure
-echo passed.
-
-echo Test case 3:
-::  - Copy WINUTILS to a new executable file, a.exe.
-::  - Change mode to 677 so owner does not have execute permission.
-::  - Verify the owner truly does not have the permissions to execute the file.
-if exist a.exe goto Failure
-copy %WINUTILS% a.exe >NUL
-%WINUTILS% chmod 677 a.exe
-if not %ERRORLEVEL% == 0 goto Failure
-.\a.exe ls
-if %ERRORLEVEL% == 0 goto Failure
-del a.exe
-if not %ERRORLEVEL% == 0 goto Failure
-echo passed.
-
-
-:: Cleanup
-::
-popd
-rd %TESTDIR%
-goto Success
-
-
-:: -----------------------------------------------------------------------------
-:: -- Function section starts below here
-:: -----------------------------------------------------------------------------
-
-::  Failure
-::  - Report test failure
-::
-:Failure
-echo Test failed.
-exit /B 1
-
-::  Success
-::  - Report test success
-::
-:Success
-echo Test succeeded.
-exit /B 0

hadoop-common-project/hadoop-common/src/main/winutils/tests/test-winutils-chmod.bat

@@ -1,174 +0,0 @@
-:: 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.
-::
-@echo off
-echo Test various chmod operations
-setlocal
-set WINUTILS="%HADOOP_HOME%\bin\winutils.exe"
-set TESTDIR=winutils-test
-
-:: Setup test directory
-::
-if not exist %TESTDIR% md %TESTDIR%
-pushd %TESTDIR%
-
-:: Test cases
-::
-echo Test case 1:
-call:TestChmod "7" "------rwx"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 2:
-call:TestChmod "70" "---rwx---"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 3:
-call:TestChmod "u-x,g+r,o=g" "rw-r--r--"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 4:
-call:TestChmod "u-x,g+rw" "rw-rw----"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 5:
-call:TestChmod "u-x,g+rwx-x,o=u" "rw-rw-rw-"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 6:
-call:TestChmodR "755" "rwxr-xr-x" "rwxr-xr-x"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 7:
-call:TestChmodR "u-x,g+r,o=g" "rw-r--r--" "rw-r--r--"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 8:
-call:TestChmodR "u-x,g+rw" "rw-rw----" "rw-rw----"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 9:
-call:TestChmodR "u-x,g+rwx-x,o=u" "rw-rw-rw-" "rw-rw-rw-"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 10:
-call:TestChmodR "a+rX" "rw-r--r--" "rwxr-xr-x"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-echo Test case 11:
-call:TestChmod "+" "rwx------"
-if %ERRORLEVEL% neq 0 goto Failure
-echo passed.
-
-:: Cleanup
-::
-popd
-rd %TESTDIR%
-goto Success
-
-
-:: -----------------------------------------------------------------------------
-:: -- Function section starts below here
-:: -----------------------------------------------------------------------------
-
-
-::  TestChmod
-::  - Test 'chmod' mode or octal mode string of a single file
-::  - The new permission will be compared with the expected result
-::
-:TestChmod :: [mode|octal mode] [expected permission]
-if exist a exit /B 1
-type NUL>a
-%WINUTILS% chmod 700 a
-%WINUTILS% chmod %~1 a
-call:CmpPerm "a" %~2
-if not %ERRORLEVEL% == 0 exit /B 1
-del a
-exit /B 0
-
-
-::  TestChmodR
-::  - Test 'chmod' mode or octal mode string with recursive option
-::  - The new permission will be compared with the expected results
-::  - The permissions could be different due to 'X' in mode string
-::
-:TestChmodR :: [mode|octal mode] [expected permission] [expected permission x]
-if exist a exit /B 1
-md a
-%WINUTILS% chmod 700 a
-type NUL>a\a
-%WINUTILS% chmod 600 a\a
-md a\b
-%WINUTILS% chmod 700 a\b
-md a\b\a
-%WINUTILS% chmod 700 a\b\a
-type NUL>a\b\b
-%WINUTILS% chmod 600 a\b\b
-type NUL>a\b\x
-%WINUTILS% chmod u+x a\b\x
-
-%WINUTILS% chmod -R %~1 a
-
-call:CmpPerm "a" %~3
-if not %ERRORLEVEL% == 0 exit /B 1
-call:CmpPerm "a\a" %~2
-if not %ERRORLEVEL% == 0 exit /B 1
-call:CmpPerm "a\b" %~3
-if not %ERRORLEVEL% == 0 exit /B 1
-call:CmpPerm "a\b\a" %~3
-if not %ERRORLEVEL% == 0 exit /B 1
-call:CmpPerm "a\b\b" %~2
-if not %ERRORLEVEL% == 0 exit /B 1
-call:CmpPerm "a\b\x" %~3
-if not %ERRORLEVEL% == 0 exit /B 1
-
-rd /S /Q a
-exit /B 0
-
-
-::  CmpPerm
-::  - Compare file permission against given permission
-::  - Use 'ls' to get the new permission
-::  - Exit with errorlevel > 0 on failure
-::
-:CmpPerm :: [file] [perm]
-for /F "tokens=1" %%A in ('call %WINUTILS% ls %~1') do set PERM=%%A
-if not %ERRORLEVEL% == 0 exit /B 1
-if not %PERM:~-9%==%~2 exit /B 1
-exit /B 0 
-
-
-::  Failure
-::  - Report test failure
-::
-:Failure
-echo Test failed.
-exit /B 1
-
-::  Success
-::  - Report test success
-::
-:Success
-echo Test succeeded.
-exit /B 0

hadoop-common-project/hadoop-common/src/main/winutils/tests/test-winutils-chown.bat

@@ -1,95 +0,0 @@
-:: 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.
-::
-@echo off
-echo Test chown operations
-setlocal
-set WINUTILS="%HADOOP_HOME%\bin\winutils.exe"
-set TESTDIR=winutils-test
-
-:: Setup test directory
-::
-if not exist %TESTDIR% md %TESTDIR%
-pushd %TESTDIR%
-
-:: Get username
-::
-for /F "tokens=1" %%A in ('call whoami') do set USER=%%A
-
-:: Test cases
-::
-echo Test case 1:
-if exist a goto Failure
-type NUL>a
-%WINUTILS% chown %USER%:Administrators a
-if not %ERRORLEVEL% == 0 goto Failure
-call:CmpOwn "a" "%USER%" "BUILTIN\Administrators"
-if not %ERRORLEVEL% == 0 goto Failure
-del a
-if not %ERRORLEVEL% == 0 goto Failure
-echo passed.
-
-echo Test case 2:
-if exist a goto Failure
-type NUL>a
-%WINUTILS% chown %USER% a
-if not %ERRORLEVEL% == 0 goto Failure
-%WINUTILS% chown :Administrators a
-if not %ERRORLEVEL% == 0 goto Failure
-call:CmpOwn "a" "%USER%" "BUILTIN\Administrators"
-if not %ERRORLEVEL% == 0 goto Failure
-del a
-if not %ERRORLEVEL% == 0 goto Failure
-echo passed.
-
-
-:: Cleanup
-::
-popd
-rd %TESTDIR%
-goto Success
-
-:: -----------------------------------------------------------------------------
-:: -- Function section starts below here
-:: -----------------------------------------------------------------------------
-
-::  CmpOwn
-::  - Compare file ownership against expected owner
-::  - Use 'ls' to get the ownership
-::  - Exit with errorlevel > 0 on failure
-::
-:CmpOwn :: [file] [expected user owner] [expected group owner]
-for /F "tokens=3" %%A in ('call %WINUTILS% ls %~1') do set OWNER=%%A
-if not %ERRORLEVEL% == 0 exit /B 1
-for /F "tokens=4" %%A in ('call %WINUTILS% ls %~1') do set GROUP=%%A
-if not %ERRORLEVEL% == 0 exit /B 1
-if /I not %OWNER%==%~2 exit /B 1
-if /I not %GROUP%==%~3 exit /B 1
-exit /B 0 
-
-
-::  Failure
-::  - Report test failure
-::
-:Failure
-echo Test failed.
-exit /B 1
-
-::  Success
-::  - Report test success
-::
-:Success
-echo Test succeeded.
-exit /B 0

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java

@@ -509,7 +509,7 @@ public class TestFileUtil {
 
     //ensure that symlink length is correctly reported by Java
     Assert.assertEquals(data.length, file.length());
-    Assert.assertEquals(Shell.WINDOWS ? 0 : data.length, link.length());
+    Assert.assertEquals(data.length, link.length());
 
     //ensure that we can read from link.
     FileInputStream in = new FileInputStream(link);
@@ -520,4 +520,142 @@ public class TestFileUtil {
     in.close();
     Assert.assertEquals(data.length, len);
   }
+  
+  /**
+   * Test that rename on a symlink works as expected.
+   */
+  @Test
+  public void testSymlinkRenameTo() throws Exception {
+    Assert.assertFalse(del.exists());
+    del.mkdirs();
+
+    File file = new File(del, FILE);
+    file.createNewFile();
+    File link = new File(del, "_link");
+
+    // create the symlink
+    FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath());
+
+    Assert.assertTrue(file.exists());
+    Assert.assertTrue(link.exists());
+
+    File link2 = new File(del, "_link2");
+
+    // Rename the symlink
+    Assert.assertTrue(link.renameTo(link2));
+
+    // Make sure the file still exists
+    // (NOTE: this would fail on Java6 on Windows if we didn't
+    // copy the file in FileUtil#symlink)
+    Assert.assertTrue(file.exists());
+
+    Assert.assertTrue(link2.exists());
+    Assert.assertFalse(link.exists());
+  }
+
+  /**
+   * Test that deletion of a symlink works as expected.
+   */
+  @Test
+  public void testSymlinkDelete() throws Exception {
+    Assert.assertFalse(del.exists());
+    del.mkdirs();
+
+    File file = new File(del, FILE);
+    file.createNewFile();
+    File link = new File(del, "_link");
+
+    // create the symlink
+    FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath());
+
+    Assert.assertTrue(file.exists());
+    Assert.assertTrue(link.exists());
+
+    // make sure that deleting a symlink works properly
+    Assert.assertTrue(link.delete());
+    Assert.assertFalse(link.exists());
+    Assert.assertTrue(file.exists());
+  }
+
+  /**
+   * Test that length on a symlink works as expected.
+   */
+  @Test
+  public void testSymlinkLength() throws Exception {
+    Assert.assertFalse(del.exists());
+    del.mkdirs();
+
+    byte[] data = "testSymLinkData".getBytes();
+
+    File file = new File(del, FILE);
+    File link = new File(del, "_link");
+
+    // write some data to the file
+    FileOutputStream os = new FileOutputStream(file);
+    os.write(data);
+    os.close();
+
+    Assert.assertEquals(0, link.length());
+
+    // create the symlink
+    FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath());
+
+    // ensure that File#length returns the target file and link size
+    Assert.assertEquals(data.length, file.length());
+    Assert.assertEquals(data.length, link.length());
+
+    file.delete();
+    Assert.assertFalse(file.exists());
+
+    if (Shell.WINDOWS && !Shell.isJava7OrAbove()) {
+      // On Java6 on Windows, we copied the file
+      Assert.assertEquals(data.length, link.length());
+    } else {
+      // Otherwise, the target file size is zero
+      Assert.assertEquals(0, link.length());
+    }
+
+    link.delete();
+    Assert.assertFalse(link.exists());
+  }
+
+  private void doUntarAndVerify(File tarFile, File untarDir) 
+                                 throws IOException {
+    if (untarDir.exists() && !FileUtil.fullyDelete(untarDir)) {
+      throw new IOException("Could not delete directory '" + untarDir + "'");
+    }
+    FileUtil.unTar(tarFile, untarDir);
+
+    String parentDir = untarDir.getCanonicalPath() + Path.SEPARATOR + "name";
+    File testFile = new File(parentDir + Path.SEPARATOR + "version");
+    Assert.assertTrue(testFile.exists());
+    Assert.assertTrue(testFile.length() == 0);
+    String imageDir = parentDir + Path.SEPARATOR + "image";
+    testFile = new File(imageDir + Path.SEPARATOR + "fsimage");
+    Assert.assertTrue(testFile.exists());
+    Assert.assertTrue(testFile.length() == 157);
+    String currentDir = parentDir + Path.SEPARATOR + "current";
+    testFile = new File(currentDir + Path.SEPARATOR + "fsimage");
+    Assert.assertTrue(testFile.exists());
+    Assert.assertTrue(testFile.length() == 4331);
+    testFile = new File(currentDir + Path.SEPARATOR + "edits");
+    Assert.assertTrue(testFile.exists());
+    Assert.assertTrue(testFile.length() == 1033);
+    testFile = new File(currentDir + Path.SEPARATOR + "fstime");
+    Assert.assertTrue(testFile.exists());
+    Assert.assertTrue(testFile.length() == 8);
+  }
+
+  @Test
+  public void testUntar() throws IOException {
+    String tarGzFileName = System.getProperty("test.cache.data",
+        "build/test/cache") + "/test-untar.tgz";
+    String tarFileName = System.getProperty("test.cache.data",
+        "build/test/cache") + "/test-untar.tar";
+    String dataDir = System.getProperty("test.build.data", "build/test/data");
+    File untarDir = new File(dataDir, "untarDir");
+
+    doUntarAndVerify(new File(tarGzFileName), untarDir);
+    doUntarAndVerify(new File(tarFileName), untarDir);
+  }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/TestUserGroupInformation.java

@@ -188,14 +188,15 @@ public class TestUserGroupInformation {
     }
     // get the groups
     pp = Runtime.getRuntime().exec(Shell.WINDOWS ?
-      Shell.WINUTILS + " groups" : "id -Gn");
+      Shell.WINUTILS + " groups -F" : "id -Gn");
     br = new BufferedReader(new InputStreamReader(pp.getInputStream()));
     String line = br.readLine();
 
     System.out.println(userName + ":" + line);
    
     Set<String> groups = new LinkedHashSet<String> ();    
-    for(String s: line.split("[\\s]")) {
+    String[] tokens = line.split(Shell.TOKEN_SEPARATOR_REGEX);
+    for(String s: tokens) {
       groups.add(s);
     }
     

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestWinUtils.java

@@ -0,0 +1,352 @@
+/**
+ * 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.util;
+
+import static org.junit.Assert.*;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+
+import org.apache.commons.io.FileUtils;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.fs.FileUtil;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Test cases for helper Windows winutils.exe utility.
+ */
+public class TestWinUtils {
+
+  private static final Log LOG = LogFactory.getLog(TestWinUtils.class);
+  private static File TEST_DIR = new File(System.getProperty("test.build.data",
+      "/tmp"), TestWinUtils.class.getSimpleName());
+
+  @Before
+  public void setUp() {
+    TEST_DIR.mkdirs();
+  }
+
+  @After
+  public void tearDown() throws IOException {
+    FileUtil.fullyDelete(TEST_DIR);
+  }
+
+  // Helper routine that writes the given content to the file.
+  private void writeFile(File file, String content) throws IOException {
+    byte[] data = content.getBytes();
+    FileOutputStream os = new FileOutputStream(file);
+    os.write(data);
+    os.close();
+  }
+
+  // Helper routine that reads the first 100 bytes from the file.
+  private String readFile(File file) throws IOException {
+    FileInputStream fos = new FileInputStream(file);
+    byte[] b = new byte[100];
+    fos.read(b);
+    return b.toString();
+  }
+
+  @Test
+  public void testLs() throws IOException {
+    if (!Shell.WINDOWS) {
+      // Not supported on non-Windows platforms
+      return;
+    }
+
+    final String content = "6bytes";
+    final int contentSize = content.length();
+    File testFile = new File(TEST_DIR, "file1");
+    writeFile(testFile, content);
+
+    // Verify permissions and file name return tokens
+    String output = Shell.execCommand(
+        Shell.WINUTILS, "ls", testFile.getCanonicalPath());
+    String[] outputArgs = output.split("[ \r\n]");
+    assertTrue(outputArgs[0].equals("-rwx------"));
+    assertTrue(outputArgs[outputArgs.length - 1]
+        .equals(testFile.getCanonicalPath()));
+
+    // Verify most tokens when using a formatted output (other tokens
+    // will be verified with chmod/chown)
+    output = Shell.execCommand(
+        Shell.WINUTILS, "ls", "-F", testFile.getCanonicalPath());
+    outputArgs = output.split("[|\r\n]");
+    assertEquals(9, outputArgs.length);
+    assertTrue(outputArgs[0].equals("-rwx------"));
+    assertEquals(contentSize, Long.parseLong(outputArgs[4]));
+    assertTrue(outputArgs[8].equals(testFile.getCanonicalPath()));
+
+    testFile.delete();
+    assertFalse(testFile.exists());
+  }
+
+  @Test
+  public void testGroups() throws IOException {
+    if (!Shell.WINDOWS) {
+      // Not supported on non-Windows platforms
+      return;
+    }
+
+    String currentUser = System.getProperty("user.name");
+
+    // Verify that groups command returns information about the current user
+    // groups when invoked with no args
+    String outputNoArgs = Shell.execCommand(
+        Shell.WINUTILS, "groups").trim();
+    String output = Shell.execCommand(
+        Shell.WINUTILS, "groups", currentUser).trim();
+    assertEquals(output, outputNoArgs);
+
+    // Verify that groups command with the -F flag returns the same information
+    String outputFormat = Shell.execCommand(
+        Shell.WINUTILS, "groups", "-F", currentUser).trim();
+    outputFormat = outputFormat.replace("|", " ");
+    assertEquals(output, outputFormat);
+  }
+
+  private void chmod(String mask, File file) throws IOException {
+    Shell.execCommand(
+        Shell.WINUTILS, "chmod", mask, file.getCanonicalPath());
+  }
+
+  private void chmodR(String mask, File file) throws IOException {
+    Shell.execCommand(
+        Shell.WINUTILS, "chmod", "-R", mask, file.getCanonicalPath());
+  }
+
+  private String ls(File file) throws IOException {
+    return Shell.execCommand(
+        Shell.WINUTILS, "ls", file.getCanonicalPath());
+  }
+
+  private String lsF(File file) throws IOException {
+    return Shell.execCommand(
+        Shell.WINUTILS, "ls", "-F", file.getCanonicalPath());
+  }
+
+  private void assertPermissions(File file, String expected)
+      throws IOException {
+    String output = ls(file).split("[ \r\n]")[0];
+    assertEquals(expected, output);
+  }
+
+  private void testChmodInternal(String mode, String expectedPerm)
+      throws IOException {
+    File a = new File(TEST_DIR, "file1");
+    assertTrue(a.createNewFile());
+
+    // Reset permissions on the file to default
+    chmod("700", a);
+
+    // Apply the mode mask
+    chmod(mode, a);
+
+    // Compare the output
+    assertPermissions(a, expectedPerm);
+
+    a.delete();
+    assertFalse(a.exists());
+  }
+
+  private void testNewFileChmodInternal(String expectedPerm) throws IOException {
+    // Create a new directory
+    File dir = new File(TEST_DIR, "dir1");
+
+    assertTrue(dir.mkdir());
+
+    // Set permission use chmod
+    chmod("755", dir);
+
+    // Create a child file in the directory
+    File child = new File(dir, "file1");
+    assertTrue(child.createNewFile());
+
+    // Verify the child file has correct permissions
+    assertPermissions(child, expectedPerm);
+
+    child.delete();
+    dir.delete();
+    assertFalse(dir.exists());
+  }
+
+  private void testChmodInternalR(String mode, String expectedPerm,
+      String expectedPermx) throws IOException {
+    // Setup test folder hierarchy
+    File a = new File(TEST_DIR, "a");
+    assertTrue(a.mkdir());
+    chmod("700", a);
+    File aa = new File(a, "a");
+    assertTrue(aa.createNewFile());
+    chmod("600", aa);
+    File ab = new File(a, "b");
+    assertTrue(ab.mkdir());
+    chmod("700", ab);
+    File aba = new File(ab, "a");
+    assertTrue(aba.mkdir());
+    chmod("700", aba);
+    File abb = new File(ab, "b");
+    assertTrue(abb.createNewFile());
+    chmod("600", abb);
+    File abx = new File(ab, "x");
+    assertTrue(abx.createNewFile());
+    chmod("u+x", abx);
+
+    // Run chmod recursive
+    chmodR(mode, a);
+
+    // Verify outcome
+    assertPermissions(a, "d" + expectedPermx);
+    assertPermissions(aa, "-" + expectedPerm);
+    assertPermissions(ab, "d" + expectedPermx);
+    assertPermissions(aba, "d" + expectedPermx);
+    assertPermissions(abb, "-" + expectedPerm);
+    assertPermissions(abx, "-" + expectedPermx);
+
+    assertTrue(FileUtil.fullyDelete(a));
+  }
+
+  @Test
+  public void testBasicChmod() throws IOException {
+    if (!Shell.WINDOWS) {
+      // Not supported on non-Windows platforms
+      return;
+    }
+
+    // - Create a file.
+    // - Change mode to 377 so owner does not have read permission.
+    // - Verify the owner truly does not have the permissions to read.
+    File a = new File(TEST_DIR, "a");
+    a.createNewFile();
+    chmod("377", a);
+
+    try {
+      readFile(a);
+      assertFalse("readFile should have failed!", true);
+    } catch (IOException ex) {
+      LOG.info("Expected: Failed read from a file with permissions 377");
+    }
+    // restore permissions
+    chmod("700", a);
+
+    // - Create a file.
+    // - Change mode to 577 so owner does not have write permission.
+    // - Verify the owner truly does not have the permissions to write.
+    chmod("577", a);
+ 
+    try {
+      writeFile(a, "test");
+      assertFalse("writeFile should have failed!", true);
+    } catch (IOException ex) {
+      LOG.info("Expected: Failed write to a file with permissions 577");
+    }
+    // restore permissions
+    chmod("700", a);
+    assertTrue(a.delete());
+
+    // - Copy WINUTILS to a new executable file, a.exe.
+    // - Change mode to 677 so owner does not have execute permission.
+    // - Verify the owner truly does not have the permissions to execute the file.
+
+    File winutilsFile = new File(Shell.WINUTILS);
+    File aExe = new File(TEST_DIR, "a.exe");
+    FileUtils.copyFile(winutilsFile, aExe);
+    chmod("677", aExe);
+
+    try {
+      Shell.execCommand(aExe.getCanonicalPath(), "ls");
+      assertFalse("executing " + aExe + " should have failed!", true);
+    } catch (IOException ex) {
+      LOG.info("Expected: Failed to execute a file with permissions 677");
+    }
+    assertTrue(aExe.delete());
+  }
+
+  @Test
+  public void testChmod() throws IOException {
+    if (!Shell.WINDOWS) {
+      // Not supported on non-Windows platforms
+      return;
+    }
+
+    testChmodInternal("7", "-------rwx");
+    testChmodInternal("70", "----rwx---");
+    testChmodInternal("u-x,g+r,o=g", "-rw-r--r--");
+    testChmodInternal("u-x,g+rw", "-rw-rw----");
+    testChmodInternal("u-x,g+rwx-x,o=u", "-rw-rw-rw-");
+    testChmodInternal("+", "-rwx------");
+
+    // Recursive chmod tests
+    testChmodInternalR("755", "rwxr-xr-x", "rwxr-xr-x");
+    testChmodInternalR("u-x,g+r,o=g", "rw-r--r--", "rw-r--r--");
+    testChmodInternalR("u-x,g+rw", "rw-rw----", "rw-rw----");
+    testChmodInternalR("u-x,g+rwx-x,o=u", "rw-rw-rw-", "rw-rw-rw-");
+    testChmodInternalR("a+rX", "rw-r--r--", "rwxr-xr-x");
+
+    // Test a new file created in a chmod'ed directory has expected permission
+    testNewFileChmodInternal("-rwx------");
+  }
+
+  private void chown(String userGroup, File file) throws IOException {
+    Shell.execCommand(
+        Shell.WINUTILS, "chown", userGroup, file.getCanonicalPath());
+  }
+
+  private void assertOwners(File file, String expectedUser,
+      String expectedGroup) throws IOException {
+    String [] args = lsF(file).trim().split("[\\|]");
+    assertEquals(expectedUser.toLowerCase(), args[2].toLowerCase());
+    assertEquals(expectedGroup.toLowerCase(), args[3].toLowerCase());
+  }
+
+  @Test
+  public void testChown() throws IOException {
+    if (!Shell.WINDOWS) {
+      // Not supported on non-Windows platforms
+      return;
+    }
+
+    File a = new File(TEST_DIR, "a");
+    assertTrue(a.createNewFile());
+    String username = System.getProperty("user.name");
+    // username including the domain aka DOMAIN\\user
+    String qualifiedUsername = Shell.execCommand("whoami").trim();
+    String admins = "Administrators";
+    String qualifiedAdmins = "BUILTIN\\Administrators";
+
+    chown(username + ":" + admins, a);
+    assertOwners(a, qualifiedUsername, qualifiedAdmins);
+ 
+    chown(username, a);
+    chown(":" + admins, a);
+    assertOwners(a, qualifiedUsername, qualifiedAdmins);
+
+    chown(":" + admins, a);
+    chown(username + ":", a);
+    assertOwners(a, qualifiedUsername, qualifiedAdmins);
+
+    assertTrue(a.delete());
+    assertFalse(a.exists());
+  }
+}

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java

@@ -1438,66 +1438,87 @@ public class TestDFSShell {
   @Test
   public void testGet() throws IOException {
     DFSTestUtil.setLogLevel2All(FSInputChecker.LOG);
+
+    final String fname = "testGet.txt";
+    Path root = new Path("/test/get");
+    final Path remotef = new Path(root, fname);
     final Configuration conf = new HdfsConfiguration();
-    // Race can happen here: block scanner is reading the file when test tries
-    // to corrupt the test file, which will fail the test on Windows platform.
-    // Disable block scanner to avoid this race.
-    conf.setInt(DFSConfigKeys.DFS_DATANODE_SCAN_PERIOD_HOURS_KEY, -1);
-    
-    MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).numDataNodes(2).build();
-    DistributedFileSystem dfs = (DistributedFileSystem)cluster.getFileSystem();
 
-    try {
-      final String fname = "testGet.txt";
-      final File localf = createLocalFile(new File(TEST_ROOT_DIR, fname));
-      final String localfcontent = DFSTestUtil.readFile(localf);
-      final Path root = mkdir(dfs, new Path("/test/get"));
-      final Path remotef = new Path(root, fname);
-      dfs.copyFromLocalFile(false, false, new Path(localf.getPath()), remotef);
+    TestGetRunner runner = new TestGetRunner() {
+    	private int count = 0;
+    	private FsShell shell = new FsShell(conf);
+
+    	public String run(int exitcode, String... options) throws IOException {
+    	  String dst = TEST_ROOT_DIR + "/" + fname+ ++count;
+    	  String[] args = new String[options.length + 3];
+    	  args[0] = "-get"; 
+    	  args[args.length - 2] = remotef.toString();
+    	  args[args.length - 1] = dst;
+    	  for(int i = 0; i < options.length; i++) {
+    	    args[i + 1] = options[i];
+    	  }
+    	  show("args=" + Arrays.asList(args));
+
+    	  try {
+    	    assertEquals(exitcode, shell.run(args));
+    	  } catch (Exception e) {
+    	    assertTrue(StringUtils.stringifyException(e), false); 
+    	  }
+    	  return exitcode == 0? DFSTestUtil.readFile(new File(dst)): null; 
+    	}
+    };
+
+    File localf = createLocalFile(new File(TEST_ROOT_DIR, fname));
+    MiniDFSCluster cluster = null;
+    DistributedFileSystem dfs = null;
 
-      final FsShell shell = new FsShell();
-      shell.setConf(conf);
-      TestGetRunner runner = new TestGetRunner() {
-        private int count = 0;
+    try {
+      cluster = new MiniDFSCluster(conf, 2, true, null);
+      dfs = (DistributedFileSystem)cluster.getFileSystem();
 
-        @Override
-        public String run(int exitcode, String... options) throws IOException {
-          String dst = TEST_ROOT_DIR + "/" + fname+ ++count;
-          String[] args = new String[options.length + 3];
-          args[0] = "-get"; 
-          args[args.length - 2] = remotef.toString();
-          args[args.length - 1] = dst;
-          for(int i = 0; i < options.length; i++) {
-            args[i + 1] = options[i];
-          }
-          show("args=" + Arrays.asList(args));
-          
-          try {
-            assertEquals(exitcode, shell.run(args));
-          } catch (Exception e) {
-            assertTrue(StringUtils.stringifyException(e), false); 
-          }
-          return exitcode == 0? DFSTestUtil.readFile(new File(dst)): null; 
-        }
-      };
+      mkdir(dfs, root);
+      dfs.copyFromLocalFile(false, false, new Path(localf.getPath()), remotef);
+      String localfcontent = DFSTestUtil.readFile(localf);
 
       assertEquals(localfcontent, runner.run(0));
       assertEquals(localfcontent, runner.run(0, "-ignoreCrc"));
 
-      //find and modify the block files
+      // find block files to modify later
       List<File> files = getBlockFiles(cluster);
+
+      // Shut down cluster and then corrupt the block files by overwriting a
+      // portion with junk data.  We must shut down the cluster so that threads
+      // in the data node do not hold locks on the block files while we try to
+      // write into them.  Particularly on Windows, the data node's use of the
+      // FileChannel.transferTo method can cause block files to be memory mapped
+      // in read-only mode during the transfer to a client, and this causes a
+      // locking conflict.  The call to shutdown the cluster blocks until all
+      // DataXceiver threads exit, preventing this problem.
+      dfs.close();
+      cluster.shutdown();
+
       show("files=" + files);
       corrupt(files);
 
+      // Start the cluster again, but do not reformat, so prior files remain.
+      cluster = new MiniDFSCluster(conf, 2, false, null);
+      dfs = (DistributedFileSystem)cluster.getFileSystem();
+
       assertEquals(null, runner.run(1));
       String corruptedcontent = runner.run(0, "-ignoreCrc");
       assertEquals(localfcontent.substring(1), corruptedcontent.substring(1));
       assertEquals(localfcontent.charAt(0)+1, corruptedcontent.charAt(0));
-
-      localf.delete();
     } finally {
-      try {dfs.close();} catch (Exception e) {}
-      cluster.shutdown();
+      if (null != dfs) {
+        try {
+          dfs.close();
+        } catch (Exception e) {
+        }
+      }
+      if (null != cluster) {
+        cluster.shutdown();
+      }
+      localf.delete();
     }
   }