Merge -c 1583241 from trunk to branch-2 to fix HDFS-4564. Ensure webhdfs returns correct HTTP response codes for denied operations. Contributed by Daryn Sharp.

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

hadoop-hdfs-project/hadoop-hdfs-httpfs/src/test/java/org/apache/hadoop/fs/http/server/TestHttpFSServer.java

@@ -349,7 +349,7 @@ public class TestHttpFSServer extends HFSTestCase {
     url = new URL(TestJettyHelper.getJettyURL(),
                   "/webhdfs/v1/?op=GETHOMEDIRECTORY&delegation=" + tokenStr);
     conn = (HttpURLConnection) url.openConnection();
-    Assert.assertEquals(HttpURLConnection.HTTP_UNAUTHORIZED,
+    Assert.assertEquals(HttpURLConnection.HTTP_FORBIDDEN,
                         conn.getResponseCode());
   }
 

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -814,6 +814,9 @@ HDFS-5535 subtasks:
     HDFS-6038. Allow JournalNode to handle editlog produced by new release with
     future layoutversion. (jing9)
 
+    HDFS-4564. Ensure webhdfs returns correct HTTP response codes for denied
+    operations. (daryn via acmurthy)
+
 Release 2.3.1 - UNRELEASED
 
   INCOMPATIBLE CHANGES

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java

@@ -304,6 +304,11 @@ public class WebHdfsFileSystem extends FileSystem
   private static Map<?, ?> validateResponse(final HttpOpParam.Op op,
       final HttpURLConnection conn, boolean unwrapException) throws IOException {
     final int code = conn.getResponseCode();
+    // server is demanding an authentication we don't support
+    if (code == HttpURLConnection.HTTP_UNAUTHORIZED) {
+      throw new IOException(
+          new AuthenticationException(conn.getResponseMessage()));
+    }
     if (code != op.getExpectedHttpResponseCode()) {
       final Map<?, ?> m;
       try {
@@ -450,52 +455,33 @@ public class WebHdfsFileSystem extends FileSystem
       this.redirected = redirected;
     }
 
-    private HttpURLConnection getHttpUrlConnection(final URL url)
-        throws IOException, AuthenticationException {
+    AbstractRunner run() throws IOException {
       UserGroupInformation connectUgi = ugi.getRealUser();
       if (connectUgi == null) {
         connectUgi = ugi;
       }
+      if (op.getRequireAuth()) {
+        connectUgi.checkTGTAndReloginFromKeytab();
+      }
       try {
+        // the entire lifecycle of the connection must be run inside the
+        // doAs to ensure authentication is performed correctly
         return connectUgi.doAs(
-            new PrivilegedExceptionAction<HttpURLConnection>() {
+            new PrivilegedExceptionAction<AbstractRunner>() {
               @Override
-              public HttpURLConnection run() throws IOException {
-                return openHttpUrlConnection(url);
+              public AbstractRunner run() throws IOException {
+                return runWithRetry();
               }
             });
-      } catch (IOException ioe) {
-        Throwable cause = ioe.getCause();
-        if (cause != null && cause instanceof AuthenticationException) {
-          throw (AuthenticationException)cause;
-        }
-        throw ioe;
       } catch (InterruptedException e) {
         throw new IOException(e);
       }
     }
     
-    private HttpURLConnection openHttpUrlConnection(final URL url)
-        throws IOException {
-      final HttpURLConnection conn;
-      try {
-        conn = (HttpURLConnection) connectionFactory.openConnection(url,
-            op.getRequireAuth());
-      } catch (AuthenticationException e) {
-        throw new IOException(e);
-      }
-      return conn;
-    }
-  
     private void init() throws IOException {
       checkRetry = !redirected;
       URL url = getUrl();
-      try {
-        conn = getHttpUrlConnection(url);
-      } catch(AuthenticationException ae) {
-        checkRetry = false;
-        throw new IOException("Authentication failed, url=" + url, ae);
-      }
+      conn = (HttpURLConnection) connectionFactory.openConnection(url);
     }
     
     private void connect() throws IOException {
@@ -516,7 +502,7 @@ public class WebHdfsFileSystem extends FileSystem
       }
     }
 
-    AbstractRunner run() throws IOException {
+    private AbstractRunner runWithRetry() throws IOException {
       /**
        * Do the real work.
        *
@@ -543,6 +529,10 @@ public class WebHdfsFileSystem extends FileSystem
           }
           return this;
         } catch(IOException ioe) {
+          Throwable cause = ioe.getCause();
+          if (cause != null && cause instanceof AuthenticationException) {
+            throw ioe; // no retries for auth failures
+          }
           shouldRetry(ioe, retry);
         }
       }

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/resources/ExceptionHandler.java

@@ -77,9 +77,9 @@ public class ExceptionHandler implements ExceptionMapper<Exception> {
     //Map response status
     final Response.Status s;
     if (e instanceof SecurityException) {
-      s = Response.Status.UNAUTHORIZED;
+      s = Response.Status.FORBIDDEN;
     } else if (e instanceof AuthorizationException) {
-      s = Response.Status.UNAUTHORIZED;
+      s = Response.Status.FORBIDDEN;
     } else if (e instanceof FileNotFoundException) {
       s = Response.Status.NOT_FOUND;
     } else if (e instanceof IOException) {

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java

@@ -410,7 +410,7 @@ public class TestWebHdfsFileSystemContract extends FileSystemContractBaseTest {
           new DoAsParam(ugi.getShortUserName() + "proxy"));
       final HttpURLConnection conn = (HttpURLConnection) url.openConnection();
       conn.connect();
-      assertEquals(HttpServletResponse.SC_UNAUTHORIZED, conn.getResponseCode());
+      assertEquals(HttpServletResponse.SC_FORBIDDEN, conn.getResponseCode());
       conn.disconnect();
     }