Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1406917 - in
 /hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common:
 ./ src/main/java/org/apache/hadoop/fs/
 src/main/java/org/apache/hadoop/fs/shell/
 src/test/java/org/apache/hadoop/fs/shell/
Date: Thu, 08 Nov 2012 04:11:02 -0000
To: common-commits@hadoop.apache.org
From: suresh@apache.org
Message-Id: <20121108041102.837062388A4A@eris.apache.org>

Author: suresh
Date: Thu Nov  8 04:11:01 2012
New Revision: 1406917

URL: http://svn.apache.org/viewvc?rev=1406917&view=rev
Log:
HADOOP-8953. Shell PathData parsing failures on Windows. Contributed by Arpit Agarwal.

Modified:
    hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/CHANGES.branch-trunk-win.txt
    hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
    hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java
    hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java

Modified: hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/CHANGES.branch-trunk-win.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/CHANGES.branch-trunk-win.txt?rev=1406917&r1=1406916&r2=1406917&view=diff
==============================================================================
--- hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/CHANGES.branch-trunk-win.txt (original)
+++ hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/CHANGES.branch-trunk-win.txt Thu Nov  8 04:11:01 2012
@@ -38,3 +38,6 @@ branch-trunk-win changes - unreleased
   HADOOP-8978. TestTrash fails on Windows. (Chris Nauroth via suresh)
 
   HADOOP-8979. TestHttpServer fails on Windows. (Chris Nauroth via suresh)
+
+  HADOOP-8953. Shell PathData parsing failures on Windows. (Arpit Agarwal via
+  suresh)

Modified: hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java?rev=1406917&r1=1406916&r2=1406917&view=diff
==============================================================================
--- hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java (original)
+++ hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java Thu Nov  8 04:11:01 2012
@@ -43,7 +43,7 @@ public class Path implements Comparable 
   
   public static final String CUR_DIR = ".";
   
-  static final boolean WINDOWS
+  public static final boolean WINDOWS
     = System.getProperty("os.name").startsWith("Windows");
 
   private URI uri;                                // a hierarchical uri
@@ -191,10 +191,11 @@ public class Path implements Comparable 
     return path;
   }
 
-  private boolean hasWindowsDrive(String path, boolean slashed) {
+  private static boolean hasWindowsDrive(String path, boolean slashed) {
     if (!WINDOWS) return false;
     int start = slashed ? 1 : 0;
     return
+      path != null &&
       path.length() >= start+2 &&
       (slashed ? path.charAt(0) == '/' : true) &&
       path.charAt(start+1) == ':' &&
@@ -202,6 +203,25 @@ public class Path implements Comparable 
        (path.charAt(start) >= 'a' && path.charAt(start) <= 'z'));
   }
 
+  /**
+   * Determine whether a given path string represents an absolute path on
+   * Windows. e.g. "C:/a/b" is an absolute path. "C:a/b" is not.
+   *
+   * @param pathString Supplies the path string to evaluate.
+   * @param slashed true if the given path is prefixed with "/".
+   * @return true if the supplied path looks like an absolute path with a Windows
+   * drive-specifier.
+   */
+  public static boolean isWindowsAbsolutePath(final String pathString,
+                                              final boolean slashed) {
+    int start = (slashed ? 1 : 0);
+
+    return
+        hasWindowsDrive(pathString, slashed) &&
+        pathString.length() >= (start + 3) &&
+        ((pathString.charAt(start + 2) == SEPARATOR_CHAR) ||
+          (pathString.charAt(start + 2) == '\\'));
+  }
 
   /** Convert this to a URI. */
   public URI toUri() { return uri; }

Modified: hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java?rev=1406917&r1=1406916&r2=1406917&view=diff
==============================================================================
--- hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java (original)
+++ hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/PathData.java Thu Nov  8 04:11:01 2012
@@ -39,6 +39,9 @@ import org.apache.hadoop.fs.shell.PathEx
 
 /**
  * Encapsulates a Path (path), its FileStatus (stat), and its FileSystem (fs).
+ * PathData ensures that the returned path string will be the same as the
+ * one passed in during initialization (unlike Path objects which can
+ * modify the path string).
  * The stat field will be null if the path does not exist.
  */
 @InterfaceAudience.Private
@@ -51,6 +54,14 @@ public class PathData implements Compara
   public FileStatus stat;
   public boolean exists;
 
+  /* True if the URI scheme was not present in the pathString but inferred.
+   */
+  private boolean inferredSchemeFromPath;
+
+  /* True if backslashes in a raw Windows path were replaced.
+   */
+  private boolean restoreBackslashes;
+
   /**
    * Creates an object to wrap the given parameters as fields.  The string
    * used to create the path will be recorded since the Path object does not
@@ -100,6 +111,15 @@ public class PathData implements Compara
     this.uri = stringToUri(pathString);
     this.path = fs.makeQualified(new Path(uri));
     setStat(stat);
+
+    if (Path.isWindowsAbsolutePath(pathString, false) ||
+        Path.isWindowsAbsolutePath(pathString, true)) {
+      inferredSchemeFromPath = true;
+    }
+
+    if (Path.WINDOWS && (pathString.indexOf('\\') != -1)) {
+      restoreBackslashes = true;
+    }
   }
 
   // need a static method for the ctor above
@@ -236,7 +256,7 @@ public class PathData implements Compara
    * Given a child of this directory, use the directory's path and the child's
    * basename to construct the string to the child.  This preserves relative
    * paths since Path will fully qualify.
-   * @param child a path contained within this directory
+   * @param childPath a path contained within this directory
    * @return String of the path relative to this directory
    */
   private String getStringForChildPath(Path childPath) {
@@ -386,7 +406,20 @@ public class PathData implements Compara
     // No interpretation of symbols. Just decode % escaped chars.
     String decodedRemainder = uri.getSchemeSpecificPart();
 
-    if (scheme == null) {
+    // Drop the scheme if it was inferred to ensure fidelity between
+    // the input and output path strings.
+    if ((scheme == null) || (inferredSchemeFromPath)) {
+
+      if (Path.isWindowsAbsolutePath(decodedRemainder, true)) {
+        // Strip the leading '/' added in stringToUri so users see a valid
+        // Windows path.
+        decodedRemainder = decodedRemainder.substring(1);
+      }
+
+      if (restoreBackslashes) {
+        decodedRemainder = decodedRemainder.replace('\\', '/');
+      }
+
       return decodedRemainder;
     } else {
       StringBuilder buffer = new StringBuilder();
@@ -427,26 +460,42 @@ public class PathData implements Compara
 
     int start = 0;
 
-    // parse uri scheme, if any
-    int colon = pathString.indexOf(':');
-    int slash = pathString.indexOf('/');
-    if (colon > 0 && (slash == colon +1)) {
-      // has a non zero-length scheme
-      scheme = pathString.substring(0, colon);
-      start = colon + 1;
-    }
-
-    // parse uri authority, if any
-    if (pathString.startsWith("//", start) &&
-        (pathString.length()-start > 2)) {
-      start += 2;
-      int nextSlash = pathString.indexOf('/', start);
-      int authEnd = nextSlash > 0 ? nextSlash : pathString.length();
-      authority = pathString.substring(start, authEnd);
-      start = authEnd;
+    if (Path.WINDOWS) {
+      // Convert backslashes to prevent URI from escaping them.
+      pathString = pathString.replace('\\', '/');
+    }
+
+    if (Path.isWindowsAbsolutePath(pathString, false)) {
+      // So we don't attempt to parse the drive specifier as a scheme.
+      // Prefix a '/' to the scheme-specific part per RFC2936.
+      scheme = "file";
+      pathString = "/" + pathString;
+    } else if (Path.isWindowsAbsolutePath(pathString, true)){
+      // So we don't attempt to parse the drive specifier as a scheme.
+      // The scheme-specific part already begins with a '/'.
+      scheme = "file";
+    } else {
+      // parse uri scheme, if any
+      int colon = pathString.indexOf(':');
+      int slash = pathString.indexOf('/');
+      if (colon > 0 && (slash == colon +1)) {
+        // has a non zero-length scheme
+        scheme = pathString.substring(0, colon);
+        start = colon + 1;
+      }
+
+      // parse uri authority, if any
+      if (pathString.startsWith("//", start) &&
+          (pathString.length()-start > 2)) {
+        start += 2;
+        int nextSlash = pathString.indexOf('/', start);
+        int authEnd = nextSlash > 0 ? nextSlash : pathString.length();
+        authority = pathString.substring(start, authEnd);
+        start = authEnd;
+      }
     }
 
-    // uri path is the rest of the string. ? or # are not interpreated,
+    // uri path is the rest of the string. ? or # are not interpreted,
     // but any occurrence of them will be quoted by the URI ctor.
     String path = pathString.substring(start, pathString.length());
 

Modified: hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java?rev=1406917&r1=1406916&r2=1406917&view=diff
==============================================================================
--- hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java (original)
+++ hadoop/common/branches/branch-trunk-win/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestPathData.java Thu Nov  8 04:11:01 2012
@@ -116,6 +116,27 @@ public class TestPathData {
   }
 
   @Test
+  public void testToFileRawWindowsPaths() throws Exception {
+    if (!Path.WINDOWS) {
+      return;
+    }
+
+    // Can we handle raw Windows paths? The files need not exist for
+    // the tests to succeed.
+    String[] winPaths = {
+        "n:\\",
+        "N:\\",
+        "N:\\foo",
+        "N:\\foo\\bar"
+    };
+
+    for (String path : winPaths) {
+      PathData item = new PathData(path, conf);
+      assertEquals(new File(path), item.toFile());
+    }
+  }
+
+  @Test
   public void testAbsoluteGlob() throws Exception {
     PathData[] items = PathData.expandAsGlob(testDir+"/d1/f1*", conf);
     assertEquals(