You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by ha...@apache.org on 2018/04/09 15:07:18 UTC
hive git commit: HIVE-18859 : Incorrect handling of thrift metastore
exceptions (Ganesha Shreedhara via Ashutosh Chauhan)
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 <ha...@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 <ga...@gmail.com>
Authored: Mon Mar 12 03:06:00 2018 -0700
Committer: Ashutosh Chauhan <ha...@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;
}