Repository: hive
Updated Branches:
  refs/heads/master b290468c0 -> 959e77257


HIVE-18859 : Incorrect handling of thrift metastore exceptions (Ganesha 
Shreedhara via Ashutosh Chauhan)

Signed-off-by: Ashutosh Chauhan <hashut...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/959e7725
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/959e7725
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/959e7725

Branch: refs/heads/master
Commit: 959e77257a006b36769cffd9efb01dd16b139474
Parents: b290468
Author: Ganesha Shreedhara <ganu...@gmail.com>
Authored: Mon Mar 12 03:06:00 2018 -0700
Committer: Ashutosh Chauhan <hashut...@apache.org>
Committed: Mon Apr 9 08:06:58 2018 -0700

----------------------------------------------------------------------
 .../AbstractTestAuthorizationApiAuthorizer.java | 19 +++++++++++--
 .../hadoop/hive/metastore/HiveMetaStore.java    | 30 ++++++++++++++++----
 2 files changed, 40 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/959e7725/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/AbstractTestAuthorizationApiAuthorizer.java
----------------------------------------------------------------------
diff --git 
a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/AbstractTestAuthorizationApiAuthorizer.java
 
b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/AbstractTestAuthorizationApiAuthorizer.java
index abd5e32..69692d0 100644
--- 
a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/AbstractTestAuthorizationApiAuthorizer.java
+++ 
b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/AbstractTestAuthorizationApiAuthorizer.java
@@ -35,6 +35,7 @@ import 
org.apache.hadoop.hive.metastore.security.HadoopThriftAuthBridge;
 import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils;
 import 
org.apache.hadoop.hive.ql.security.authorization.MetaStoreAuthzAPIAuthorizerEmbedOnly;
 import 
org.apache.hadoop.hive.ql.security.authorization.AuthorizationPreEventListener;
+import org.apache.thrift.TException;
 import org.junit.Test;
 
 /**
@@ -91,15 +92,27 @@ public abstract class 
AbstractTestAuthorizationApiAuthorizer {
         // authorization checks passed.
         String exStackString = ExceptionUtils.getStackTrace(e);
         assertTrue("Verifying this exception came after authorization check",
-            
exStackString.contains("org.apache.hadoop.hive.metastore.ObjectStore"));
+                
exStackString.contains("org.apache.hadoop.hive.metastore.ObjectStore"));
         // If its not an exception caused by auth check, ignore it
       }
       assertFalse("Authz Exception should have been thrown in remote mode", 
isRemoteMetastoreMode);
       System.err.println("No auth exception thrown");
     } catch (MetaException e) {
       System.err.println("Caught exception");
-      caughtEx = true;
-      
assertTrue(e.getMessage().contains(MetaStoreAuthzAPIAuthorizerEmbedOnly.errMsg));
+      String exStackString = ExceptionUtils.getStackTrace(e);
+      // Check if MetaException has one of InvalidObjectException or 
NoSuchObjectExcetion or any exception thrown from ObjectStore , which means 
that the
+      // authorization checks passed.
+      
if(exStackString.contains("org.apache.hadoop.hive.metastore.api.NoSuchObjectException")
 ||
+              
exStackString.contains("org.apache.hadoop.hive.metastore.api.InvalidObjectException"))
 {
+        assertFalse("No Authz exception thrown in embedded mode", 
isRemoteMetastoreMode);
+      } else {
+        caughtEx = true;
+        
assertTrue(e.getMessage().contains(MetaStoreAuthzAPIAuthorizerEmbedOnly.errMsg));
+      }
+    } catch (TException e) {
+      String exStackString = ExceptionUtils.getStackTrace(e);
+      assertTrue("Verifying this exception came after authorization check",
+              
exStackString.contains("org.apache.hadoop.hive.metastore.ObjectStore"));
     }
     if (!isRemoteMetastoreMode) {
       assertFalse("No exception should be thrown in embedded mode", caughtEx);

http://git-wip-us.apache.org/repos/asf/hive/blob/959e7725/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
----------------------------------------------------------------------
diff --git 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
index 102e5b4..a2fe7d7 100644
--- 
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
+++ 
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
@@ -5981,8 +5981,11 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         ret = ms.grantRole(role, principalName, principalType, grantor, 
grantorType, grantOption);
       } catch (MetaException e) {
         throw e;
+      } catch (InvalidObjectException | NoSuchObjectException e) {
+        ret = false;
+        MetaStoreUtils.logAndThrowMetaException(e);
       } catch (Exception e) {
-        throw new RuntimeException(e);
+        throw new TException(e);
       }
       return ret;
     }
@@ -6030,8 +6033,11 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         ret = getMS().addRole(role.getRoleName(), role.getOwnerName());
       } catch (MetaException e) {
         throw e;
+      } catch (InvalidObjectException | NoSuchObjectException e) {
+        ret = false;
+        MetaStoreUtils.logAndThrowMetaException(e);
       } catch (Exception e) {
-        throw new RuntimeException(e);
+        throw new TException(e);
       }
       return ret;
     }
@@ -6048,8 +6054,11 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         ret = getMS().removeRole(roleName);
       } catch (MetaException e) {
         throw e;
+      } catch (NoSuchObjectException e) {
+        ret = false;
+        MetaStoreUtils.logAndThrowMetaException(e);
       } catch (Exception e) {
-        throw new RuntimeException(e);
+        throw new TException(e);
       }
       return ret;
     }
@@ -6078,8 +6087,11 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         ret = getMS().grantPrivileges(privileges);
       } catch (MetaException e) {
         throw e;
+      } catch (InvalidObjectException | NoSuchObjectException e) {
+        ret = false;
+        MetaStoreUtils.logAndThrowMetaException(e);
       } catch (Exception e) {
-        throw new RuntimeException(e);
+        throw new TException(e);
       }
       return ret;
     }
@@ -6104,8 +6116,11 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         ret = ms.revokeRole(mRole, userName, principalType, grantOption);
       } catch (MetaException e) {
         throw e;
+      } catch (NoSuchObjectException e) {
+        ret = false;
+        MetaStoreUtils.logAndThrowMetaException(e);
       } catch (Exception e) {
-        throw new RuntimeException(e);
+        throw new TException(e);
       }
       return ret;
     }
@@ -6179,8 +6194,11 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         ret = getMS().revokePrivileges(privileges, grantOption);
       } catch (MetaException e) {
         throw e;
+      } catch (InvalidObjectException | NoSuchObjectException e) {
+        ret = false;
+        MetaStoreUtils.logAndThrowMetaException(e);
       } catch (Exception e) {
-        throw new RuntimeException(e);
+        throw new TException(e);
       }
       return ret;
     }

Reply via email to