HADOOP-3225. Unwrapping methods of RemoteException should initialize detailedMassage field. Contributed by Mahadev Konar, Konstantin Shvachko, Chris Douglas.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/core/branches/branch-0.17@648421 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -551,20 +551,23 @@ Release 0.17.0 - Unreleased
     HADOOP-3208. Fix WritableDeserializer to set the Configuration on
     deserialized Writables. (Enis Soztutar via cdouglas)
 
-   HADOOP-3224. 'dfs -du /dir' does not return correct size.
-   (Lohit Vjayarenu via rangadi)
+    HADOOP-3224. 'dfs -du /dir' does not return correct size.
+    (Lohit Vjayarenu via rangadi)
 
-   HADOOP-3223. Fix typo in help message for -chmod. (rangadi)
+    HADOOP-3223. Fix typo in help message for -chmod. (rangadi)
 
-   HADOOP-1373. checkPath() should ignore case when it compares authoriy.
-   (Edward J. Yoon via rangadi)
+    HADOOP-1373. checkPath() should ignore case when it compares authoriy.
+    (Edward J. Yoon via rangadi)
 
-   HADOOP-3204. Fixes a problem to do with ReduceTask's LocalFSMerger not
-   catching Throwable.  (Amar Ramesh Kamat via ddas)
+    HADOOP-3204. Fixes a problem to do with ReduceTask's LocalFSMerger not
+    catching Throwable.  (Amar Ramesh Kamat via ddas)
 
     HADOOP-3229. Report progress when collecting records from the mapper and
     the combiner. (Doug Cutting via cdouglas)
 
+    HADOOP-3225. Unwrapping methods of RemoteException should initialize
+    detailedMassage field. (Mahadev Konar, shv, cdouglas)
+
 Release 0.16.3 - 2008-04-16
 
   BUG FIXES

src/java/org/apache/hadoop/fs/FsShell.java

@@ -923,8 +923,7 @@ public class FsShell extends Configured implements Tool {
         try {
           String[] content;
           content = e.getLocalizedMessage().split("\n");
-          System.err.println(cmd.substring(1) + ": " +
-                             content[0]);
+          System.err.println(cmd.substring(1) + ": " + content[0]);
         } catch (Exception ex) {
           System.err.println(cmd.substring(1) + ": " +
                              ex.getLocalizedMessage());
@@ -1571,8 +1570,12 @@ public class FsShell extends Configured implements Tool {
         // IO exception encountered locally.
         //
         exitCode = -1;
+        String content = e.getLocalizedMessage();
+        if (content != null) {
+          content = content.split("\n")[0];
+        }
         System.err.println(cmd.substring(1) + ": " +
-                           e.getLocalizedMessage());
+                          content);
       }
     }
     return exitCode;

src/java/org/apache/hadoop/fs/permission/AccessControlException.java

@@ -28,7 +28,9 @@ public class AccessControlException extends java.io.IOException {
    * Default constructor is needed for unwrapping from 
    * {@link org.apache.hadoop.ipc.RemoteException}.
    */
-  public AccessControlException() {}
+  public AccessControlException() {
+    super("Permission denied.");
+  }
 
   /**
    * Constructs an {@link AccessControlException}

src/java/org/apache/hadoop/ipc/RemoteException.java

@@ -19,6 +19,7 @@
 package org.apache.hadoop.ipc;
 
 import java.io.IOException;
+import java.lang.reflect.Constructor;
 
 public class RemoteException extends IOException {
   private String className;
@@ -36,50 +37,53 @@ public class RemoteException extends IOException {
    * If this remote exception wraps up one of the lookupTypes
    * then return this exception.
    * <p>
-   * Unwraps any IOException that has a default constructor.
+   * Unwraps any IOException.
    * 
    * @param lookupTypes the desired exception class.
    * @return IOException, which is either the lookupClass exception or this.
    */
-  public IOException unwrapRemoteException(Class... lookupTypes) {
+  public IOException unwrapRemoteException(Class<?>... lookupTypes) {
     if(lookupTypes == null)
       return this;
-    for(Class lookupClass : lookupTypes) {
-      if(!IOException.class.isAssignableFrom(lookupClass))
+    for(Class<?> lookupClass : lookupTypes) {
+      if(!lookupClass.getName().equals(getClassName()))
         continue;
-      if(lookupClass.getName().equals(getClassName())) {
-        try {
-          IOException ex = (IOException)lookupClass.newInstance();
-          ex.initCause(this);
-          return ex;
-        } catch(Exception e) {
-          // cannot instantiate lookupClass, just return this
-          return this;
-        }
-      } 
+      try {
+        return instantiateException(lookupClass.asSubclass(IOException.class));
+      } catch(Exception e) {
+        // cannot instantiate lookupClass, just return this
+        return this;
+      }
     }
+    // wrapped up exception is not in lookupTypes, just return this
     return this;
   }
 
   /**
-   * If this remote exception wraps an IOException that has a default
-   * contructor then instantiate and return the original exception.
-   * Otherwise return this.
+   * Instantiate and return the exception wrapped up by this remote exception.
+   * 
+   * <p> This unwraps any <code>Throwable</code> that has a constructor taking
+   * a <code>String</code> as a parameter.
+   * Otherwise it returns this.
    * 
-   * @return IOException
+   * @return <code>Throwable
    */
   public IOException unwrapRemoteException() {
-    IOException ex;
     try {
-      Class realClass = Class.forName(getClassName());
-      if(!IOException.class.isAssignableFrom(realClass))
-        return this;
-      ex = (IOException)realClass.newInstance();
-      ex.initCause(this);
-      return ex;
+      Class<?> realClass = Class.forName(getClassName());
+      return instantiateException(realClass.asSubclass(IOException.class));
     } catch(Exception e) {
-      // cannot instantiate the original exception, just throw this
+      // cannot instantiate the original exception, just return this
     }
     return this;
   }
+
+  private IOException instantiateException(Class<? extends IOException> cls)
+      throws Exception {
+    Constructor<? extends IOException> cn = cls.getConstructor(String.class);
+    cn.setAccessible(true);
+    IOException ex = cn.newInstance(this.getMessage());
+    ex.initCause(this);
+    return ex;
+  }
 }

src/test/org/apache/hadoop/dfs/TestDFSShell.java

@@ -25,7 +25,9 @@ import java.util.zip.GZIPOutputStream;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.*;
+import org.apache.hadoop.fs.permission.FsPermission;
 import org.apache.hadoop.fs.shell.*;
+import org.apache.hadoop.security.UnixUserGroupInformation;
 import org.apache.hadoop.util.StringUtils;
 import org.apache.hadoop.util.ToolRunner;
 
@@ -1095,6 +1097,44 @@ public class TestDFSShell extends TestCase {
     String run(int exitcode, String... options) throws IOException;
   }
 
+  public void testRemoteException() throws Exception {
+    UnixUserGroupInformation tmpUGI = new UnixUserGroupInformation("tmpname",
+        new String[] {
+        "mygroup"});
+    MiniDFSCluster dfs = null;
+    PrintStream bak = null;
+    try {
+      Configuration conf = new Configuration();
+      dfs = new MiniDFSCluster(conf, 2, true, null);
+      FileSystem fs = dfs.getFileSystem();
+      Path p = new Path("/foo");
+      fs.mkdirs(p);
+      fs.setPermission(p, new FsPermission((short)0700));
+      UnixUserGroupInformation.saveToConf(conf,
+          UnixUserGroupInformation.UGI_PROPERTY_NAME, tmpUGI);
+      FsShell fshell = new FsShell(conf);
+      bak = System.err;
+      ByteArrayOutputStream out = new ByteArrayOutputStream();
+      PrintStream tmp = new PrintStream(out);
+      System.setErr(tmp);
+      String[] args = new String[2];
+      args[0] = "-ls";
+      args[1] = "/foo";
+      int ret = ToolRunner.run(fshell, args);
+      assertTrue("returned should be -1", (ret == -1));
+      String str = out.toString();
+      assertTrue("permission denied printed", str.indexOf("Permission denied") != -1);
+      out.reset();
+    } finally {
+      if (bak != null) {
+        System.setErr(bak);
+      }
+      if (dfs != null) {
+        dfs.shutdown();
+      }
+    }
+  }
+  
   public void testGet() throws IOException {
     DFSTestUtil.setLogLevel2All(FSInputChecker.LOG);
     final Configuration conf = new Configuration();