DanielZhu58 commented on code in PR #6884:
URL: https://github.com/apache/hive/pull/6884#discussion_r4418681049
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/TransactionHandler.java:
##########
@@ -505,9 +524,87 @@ public ShowCompactResponse show_compact(ShowCompactRequest
rqst) throws TExcepti
@Override
public boolean submit_for_cleanup(CompactionRequest rqst, long
highestWriteId, long txnId)
throws TException {
+ if (!hasProxyPrivilege()) {
+ try {
+ authorizeAndPrepareCompactionRequest(rqst);
+ } catch (NoSuchObjectException e) {
+ // The table/partition does not exist (anymore): there is nothing an
untrusted caller may clean up.
+ LOG.info("Ignoring the cleanup request of {}.{}: {}",
rqst.getDbname(), rqst.getTablename(),
+ e.getMessage());
+ return false;
+ } catch (Exception e) {
+ rethrowException(e);
+ }
+ }
return getTxnHandler().submitForCleanup(rqst, highestWriteId, txnId);
}
+ /**
+ * Authorize a compaction/cleanup request of a caller that has no proxy
privilege and derive the
+ * server-side request attributes: the table is authorized with a {@link
PreAlterTableEvent}, the
+ * canonical db/table names are used for the queued request, the run-as user
is derived from the
+ * storage location and a caller-supplied cleanup location must stay inside
the table/partition location.
+ *
+ * @throws NoSuchObjectException if the table or the partition does not exist
+ */
+ private void authorizeAndPrepareCompactionRequest(CompactionRequest rqst)
throws TException {
+ if (rqst.getDbname() == null || rqst.getTablename() == null) {
+ throw new MetaException("The DB and table name cannot be null.");
+ }
+ String[] parsedDbName = parseDbName(rqst.getDbname(), conf);
+ String catName = parsedDbName[CAT_NAME];
+ String dbName = parsedDbName[DB_NAME];
+ if (!getDefaultCatalog(conf).equalsIgnoreCase(catName)) {
+ // the compaction queue has no notion of catalogs, so the request would
end up on the default catalog
+ throw new MetaException("Compaction and cleanup requests are only
supported for the default catalog, got "
+ + catName);
+ }
+
+ GetTableRequest getTableRequest = new GetTableRequest(dbName,
rqst.getTablename());
+ getTableRequest.setCatName(catName);
+ Table table = get_table_core(getTableRequest);
+ firePreEvent(new PreAlterTableEvent(table, table, this));
+
+ // queue the request for the table that has been authorized
+ rqst.setDbname(table.getDbName());
+ rqst.setTablename(table.getTableName());
+
+ String location = table.getSd() == null ? null :
table.getSd().getLocation();
+ if (rqst.isSetPartitionname() && rqst.getPartitionname() != null) {
+ List<String> partVals = getPartValsFromName(table,
rqst.getPartitionname());
+ Partition partition = getMS().getPartition(catName, table.getDbName(),
table.getTableName(), partVals);
+ location = partition.getSd() == null ? null :
partition.getSd().getLocation();
+ }
+ if (location == null) {
+ throw new MetaException("Cannot determine storage location of " +
table.getDbName() + "."
+ + table.getTableName() + "; compaction is not supported for this
table or partition");
+ }
+
+ String requestedLocation = rqst.getProperties() == null ? null :
rqst.getProperties().get(META_TABLE_LOCATION);
Review Comment:
The Cleaner purges the location property as is, without looking at the
table.
Without this check a caller authorized on table A could pass table B's
directory and have it purged.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/TransactionHandler.java:
##########
@@ -505,9 +524,87 @@ public ShowCompactResponse show_compact(ShowCompactRequest
rqst) throws TExcepti
@Override
public boolean submit_for_cleanup(CompactionRequest rqst, long
highestWriteId, long txnId)
throws TException {
+ if (!hasProxyPrivilege()) {
+ try {
+ authorizeAndPrepareCompactionRequest(rqst);
+ } catch (NoSuchObjectException e) {
+ // The table/partition does not exist (anymore): there is nothing an
untrusted caller may clean up.
+ LOG.info("Ignoring the cleanup request of {}.{}: {}",
rqst.getDbname(), rqst.getTablename(),
+ e.getMessage());
+ return false;
+ } catch (Exception e) {
+ rethrowException(e);
+ }
+ }
return getTxnHandler().submitForCleanup(rqst, highestWriteId, txnId);
}
+ /**
+ * Authorize a compaction/cleanup request of a caller that has no proxy
privilege and derive the
+ * server-side request attributes: the table is authorized with a {@link
PreAlterTableEvent}, the
+ * canonical db/table names are used for the queued request, the run-as user
is derived from the
+ * storage location and a caller-supplied cleanup location must stay inside
the table/partition location.
+ *
+ * @throws NoSuchObjectException if the table or the partition does not exist
+ */
+ private void authorizeAndPrepareCompactionRequest(CompactionRequest rqst)
throws TException {
+ if (rqst.getDbname() == null || rqst.getTablename() == null) {
+ throw new MetaException("The DB and table name cannot be null.");
+ }
+ String[] parsedDbName = parseDbName(rqst.getDbname(), conf);
+ String catName = parsedDbName[CAT_NAME];
+ String dbName = parsedDbName[DB_NAME];
+ if (!getDefaultCatalog(conf).equalsIgnoreCase(catName)) {
+ // the compaction queue has no notion of catalogs, so the request would
end up on the default catalog
+ throw new MetaException("Compaction and cleanup requests are only
supported for the default catalog, got "
+ + catName);
+ }
+
+ GetTableRequest getTableRequest = new GetTableRequest(dbName,
rqst.getTablename());
+ getTableRequest.setCatName(catName);
+ Table table = get_table_core(getTableRequest);
+ firePreEvent(new PreAlterTableEvent(table, table, this));
+
+ // queue the request for the table that has been authorized
+ rqst.setDbname(table.getDbName());
+ rqst.setTablename(table.getTableName());
+
+ String location = table.getSd() == null ? null :
table.getSd().getLocation();
+ if (rqst.isSetPartitionname() && rqst.getPartitionname() != null) {
+ List<String> partVals = getPartValsFromName(table,
rqst.getPartitionname());
+ Partition partition = getMS().getPartition(catName, table.getDbName(),
table.getTableName(), partVals);
+ location = partition.getSd() == null ? null :
partition.getSd().getLocation();
Review Comment:
Acknowledge. Removed the null check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
