okumin commented on code in PR #6812:
URL: https://github.com/apache/hive/pull/6812#discussion_r4113885372


##########
standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/BaseRESTCatalogTests.java:
##########
@@ -277,4 +284,56 @@ void testStageCreateTableWithDeniedLocation() {
     Assertions.assertThrows(ForbiddenException.class, 
builder::createTransaction);
     Assertions.assertThrows(NoSuchTableException.class, () -> 
catalog.loadTable(tableIdentifier));
   }
+
+  private static String writeMetadataFile(String directory, String 
tableLocation) throws IOException {
+    var metadataLocation = directory + "/v1.metadata.json";
+    Files.deleteIfExists(java.nio.file.Path.of(metadataLocation));
+    var io = new HadoopFileIO(new Configuration(false));
+    var metadata = TableMetadata.newTableMetadata(new Schema(), 
PartitionSpec.unpartitioned(), tableLocation,
+        Collections.emptyMap());
+    TableMetadataParser.write(metadata, io.newOutputFile(metadataLocation));
+    return metadataLocation;
+  }
+
+  @Test
+  void testRegisterTableWithDeniedLocation() {
+    var tableIdentifier = TableIdentifier.of("default", 
"register-table-denied");
+    var metadataLocation = MockHiveAuthorizer.DENIED_PREFIX + 
"/register-table-denied/v1.metadata.json";
+    Assertions.assertThrows(ForbiddenException.class, () -> 
catalog.registerTable(tableIdentifier, metadataLocation));
+    Assertions.assertThrows(NoSuchTableException.class, () -> 
catalog.loadTable(tableIdentifier));
+  }
+
+  @Test
+  void testRegisterTableWithDeniedEmbeddedLocation() throws IOException {
+    var tableIdentifier = TableIdentifier.of("default", 
"register-table-embedded-denied");
+    var tableLocation = MockHiveAuthorizer.DENIED_PREFIX + 
"/register-table-embedded-denied";
+    var metadataLocation = writeMetadataFile(
+        MockHiveAuthorizer.ALLOWED_PREFIX + "/register-table-embedded-denied", 
tableLocation);
+    Assertions.assertThrows(ForbiddenException.class, () -> 
catalog.registerTable(tableIdentifier, metadataLocation));
+    Assertions.assertThrows(NoSuchTableException.class, () -> 
catalog.loadTable(tableIdentifier));
+  }
+
+  @Test
+  void testDropTablePurgeDoesNotDeleteFilesOutsideTableLocation() throws 
IOException {
+    var victimDirectory = 
java.nio.file.Path.of(MockHiveAuthorizer.ALLOWED_PREFIX, 
"structural-fence-victim");
+    Files.createDirectories(victimDirectory);
+    var victimFile = victimDirectory.resolve("victim-data.txt");
+    Files.writeString(victimFile, "victim data");
+
+    var tableIdentifier = TableIdentifier.of("default", 
"structural-fence-attacker");
+    var tableLocation = MockHiveAuthorizer.ALLOWED_PREFIX + 
"/structural-fence-attacker";
+    Table table = catalog.buildTable(tableIdentifier, new 
Schema()).withLocation(tableLocation).create();
+
+    DataFile dataFile = DataFiles.builder(table.spec())
+        .withPath(victimFile.toUri().toString())
+        .withFormat(FileFormat.PARQUET)
+        .withFileSizeInBytes(Files.size(victimFile))
+        .withRecordCount(1)
+        .build();
+    table.newAppend().appendFile(dataFile).commit();

Review Comment:
   We may add one more data file under `MockHiveAuthorizer.ALLOWED_PREFIX + 
"/structural-fence-attacker/data"` and check whether the file is removed. It 
will explain how the file io works better.



##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java:
##########
@@ -161,4 +175,133 @@ void validateStageCreateTable(String catalogName, 
Namespace namespace, Map<Strin
       throw new IllegalStateException("Failed to check privileges 
stage-create", e);
     }
   }
+
+  /**
+   * Enforces authorization for REGISTER_TABLE. The request's {@code 
metadataLocation} is fetched with the
+   * catalog's shared, service-level {@link FileIO}, so both that location and 
the {@code location()} embedded in
+   * the metadata file it points to (which becomes the table's HMS {@code 
StorageDescriptor.location}, and is what
+   * a later purge trusts as its deletion root, see {@link 
#validateDropTablePurge}) must be authorized. Otherwise
+   * REGISTER_TABLE is an arbitrary-file-read primitive that returns any 
metadata file's contents to the caller.
+   *
+   * <p>When no {@code HiveAuthorizer} is configured, falls back to requiring 
both locations to be contained in
+   * the namespace's external or managed root, since there is no policy to 
otherwise decide whether the caller may
+   * read an arbitrary location with service credentials.
+   *
+   * @param catalogName the Hive catalog name
+   * @param namespace the Iceberg namespace
+   * @param namespaceMetadata the Iceberg namespace metadata
+   * @param request the register table request
+   * @param io the {@link FileIO} used to read the metadata file
+   * @throws ForbiddenException if a location is not authorized, or not 
contained in the namespace
+   * @throws IllegalStateException if the authorization plugin fails
+   */
+  void validateRegisterTable(String catalogName, Namespace namespace, 
Map<String, String> namespaceMetadata,
+      RegisterTableRequest request, FileIO io) {
+    Preconditions.checkArgument(namespace.levels().length == 1, "Hive does not 
support multi-level namespaces");
+    var databaseName = namespace.level(0);
+    var commandString = "register table " + request.name();
+    checkLocationAuthorized(catalogName, databaseName, namespaceMetadata, 
request.metadataLocation(), commandString);
+
+    var metadata = TableMetadataParser.read(io, request.metadataLocation());
+    checkLocationAuthorized(catalogName, databaseName, namespaceMetadata, 
metadata.location(), commandString);
+  }
+
+  /**
+   * Enforces authorization for DROP_TABLE with {@code purge=true}. Purge 
deletes every file referenced by the
+   * table's current metadata using the catalog's shared, service-level {@link 
FileIO}, so the location must be
+   * authorized like any other DFS_URI access.
+   *
+   * <p>Unlike {@link #validateRegisterTable}, there is no 
namespace-containment fallback here: the structural
+   * fence in {@code HiveCatalog.dropTable} already restricts purge deletions 
to files under the table's own
+   * location regardless of whether a {@code HiveAuthorizer} is configured, so 
a deployment without one relies on
+   * that fence rather than this check.
+   *
+   * @param catalogName the Hive catalog name
+   * @param identifier the table identifier being dropped
+   * @param location the table's current location
+   * @throws ForbiddenException if the location is not authorized
+   * @throws IllegalStateException if the authorization plugin fails
+   */
+  void validateDropTablePurge(String catalogName, TableIdentifier identifier, 
String location) {
+    var authorizer = authorizerSupplier.get();
+    if (authorizer == null) {
+      LOG.info("No pre-event listener is configured for catalog {}, skipping 
drop-table-purge authorization for {}",
+          catalogName, identifier);
+      return;
+    }
+
+    var inputs = Collections.singletonList(
+        new 
HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.DFS_URI, 
location));
+    var builder = new HiveAuthzContext.Builder();
+    builder.setCommandString("drop table " + identifier.name());
+    try {
+      authorizer.checkPrivileges(HiveOperationType.DROPTABLE, inputs, 
Collections.emptyList(), builder.build());
+    } catch (HiveAccessControlException e) {
+      throw new ForbiddenException(e, e.getMessage());
+    } catch (HiveAuthzPluginException e) {
+      throw new IllegalStateException("Failed to check privileges 
drop-table-purge", e);
+    }
+  }
+
+  private void checkLocationAuthorized(String catalogName, String databaseName,
+      Map<String, String> namespaceMetadata, String location, String 
commandString) {
+    var authorizer = authorizerSupplier.get();
+    if (authorizer == null) {
+      LOG.info("No pre-event listener is configured for catalog {}, falling 
back to namespace containment for {}",
+          catalogName, location);
+      checkContainedInNamespace(databaseName, namespaceMetadata, location);
+      return;
+    }
+
+    var inputs = Collections.singletonList(
+        new 
HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.DFS_URI, 
location));
+    var builder = new HiveAuthzContext.Builder();
+    builder.setCommandString(commandString);
+    try {
+      authorizer.checkPrivileges(HiveOperationType.CREATETABLE, inputs, 
Collections.emptyList(), builder.build());
+    } catch (HiveAccessControlException e) {
+      throw new ForbiddenException(e, e.getMessage());
+    } catch (HiveAuthzPluginException e) {
+      throw new IllegalStateException("Failed to check privileges for " + 
commandString, e);
+    }
+  }
+
+  private void checkContainedInNamespace(String databaseName, Map<String, 
String> namespaceMetadata,
+      String location) {
+    var externalRoot = namespaceMetadata.get("location");
+    if (externalRoot != null && isContained(externalRoot, location)) {
+      return;
+    }
+    if (isContained(managedNamespaceLocation(databaseName), location)) {
+      return;
+    }
+    throw new ForbiddenException(
+        "Location %s is not authorized and is not contained in namespace %s", 
location, databaseName);
+  }
+
+  private String managedNamespaceLocation(String databaseName) {
+    var warehouseLocation = 
conf.get(HiveConf.ConfVars.METASTORE_WAREHOUSE.varname);
+    Preconditions.checkNotNull(warehouseLocation, "Warehouse location is not 
set: hive.metastore.warehouse.dir=null");
+    if (warehouseLocation.endsWith("/")) {
+      warehouseLocation = warehouseLocation.substring(0, 
warehouseLocation.length() - 1);
+    }
+    return String.format("%s/%s.db", warehouseLocation, databaseName);

Review Comment:
   Let's use 
`asNamespaceCatalog.loadNamespaceMetadata(namespace).get("location")` that 
abstracts the naming convention. Some conventions are exceptions; for example, 
the `default.test` table is located not in `/path/to/warehouse/default.db/test` 
but in `/path/to/warehouse/test`.



##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -271,7 +275,7 @@ public boolean dropTable(TableIdentifier identifier, 
boolean purge) {
       });
 
       if (purge && lastMetadata != null) {
-        CatalogUtil.dropTableData(ops.io(), lastMetadata);
+        CatalogUtil.dropTableData(new ScopedDeleteFileIO(ops.io(), 
lastMetadata.location()), lastMetadata);

Review Comment:
   This change disallows a user from purging metadata or data files outside the 
location, even if they have storage access. I think it is OK as the default 
value.
   For future reference, [Polaris has some defensive 
configurations](https://polaris.apache.org/releases/1.7.0/configuration/configuration-reference/).
   - DROP_WITH_PURGE_ENABLED: DROP with PURGE is disabled by default
   - ALLOW_EXTERNAL_METADATA_FILE_LOCATION: Metadata files outside the location 
are not allowed by default
   - ALLOW_UNSTRUCTURED_TABLE_LOCATION: The directory structure is enforced by 
default



##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -241,6 +241,10 @@ public String name() {
     return name;
   }
 
+  public FileIO io() {

Review Comment:
   And, maybe the name should be `ioForPurge` or else so that we don't 
mistakenly use it to scan files.



##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/ScopedDeleteFileIO.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.hive;
+
+import java.util.Map;
+import org.apache.hadoop.fs.Path;
+import org.apache.iceberg.io.FileIO;
+import org.apache.iceberg.io.InputFile;
+import org.apache.iceberg.io.OutputFile;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A {@link FileIO} decorator used by {@link 
HiveCatalog#dropTable(org.apache.iceberg.catalog.TableIdentifier,
+ * boolean)} to fence purge deletions to files under a table's own location, 
regardless of what a table's
+ * metadata or manifests actually reference.
+ *
+ * <p>This does not implement {@link 
org.apache.iceberg.io.SupportsBulkOperations} or
+ * {@link org.apache.iceberg.io.SupportsPrefixOperations} even when the 
delegate does, so that
+ * {@code CatalogUtil.dropTableData} is forced to route every deletion through 
{@link #deleteFile(String)}.
+ */
+class ScopedDeleteFileIO implements FileIO {
+  private static final Logger LOG = 
LoggerFactory.getLogger(ScopedDeleteFileIO.class);
+
+  private final FileIO delegate;
+  private final String location;
+
+  ScopedDeleteFileIO(FileIO delegate, String location) {
+    this.delegate = delegate;
+    this.location = normalize(location);
+  }
+
+  @Override
+  public InputFile newInputFile(String path) {
+    return delegate.newInputFile(path);
+  }
+
+  @Override
+  public OutputFile newOutputFile(String path) {
+    return delegate.newOutputFile(path);
+  }
+
+  @Override
+  public void deleteFile(String path) {
+    if (!isContained(location, normalize(path))) {
+      LOG.warn("Skipping delete outside table location {}: {}", location, 
path);
+      return;
+    }
+    delegate.deleteFile(path);
+  }
+
+  @Override
+  public Map<String, String> properties() {
+    return delegate.properties();
+  }
+
+  @Override
+  public void initialize(Map<String, String> properties) {
+    delegate.initialize(properties);
+  }
+
+  @Override
+  public void close() {
+    delegate.close();
+  }
+
+  private static boolean isContained(String root, String candidate) {
+    return candidate.equals(root) || candidate.startsWith(root.endsWith("/") ? 
root : root + "/");

Review Comment:
   I prefer to use `FileUtils.isPathWithinSubtree`.



##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java:
##########
@@ -161,4 +175,133 @@ void validateStageCreateTable(String catalogName, 
Namespace namespace, Map<Strin
       throw new IllegalStateException("Failed to check privileges 
stage-create", e);
     }
   }
+
+  /**
+   * Enforces authorization for REGISTER_TABLE. The request's {@code 
metadataLocation} is fetched with the
+   * catalog's shared, service-level {@link FileIO}, so both that location and 
the {@code location()} embedded in
+   * the metadata file it points to (which becomes the table's HMS {@code 
StorageDescriptor.location}, and is what
+   * a later purge trusts as its deletion root, see {@link 
#validateDropTablePurge}) must be authorized. Otherwise
+   * REGISTER_TABLE is an arbitrary-file-read primitive that returns any 
metadata file's contents to the caller.
+   *
+   * <p>When no {@code HiveAuthorizer} is configured, falls back to requiring 
both locations to be contained in
+   * the namespace's external or managed root, since there is no policy to 
otherwise decide whether the caller may
+   * read an arbitrary location with service credentials.
+   *
+   * @param catalogName the Hive catalog name
+   * @param namespace the Iceberg namespace
+   * @param namespaceMetadata the Iceberg namespace metadata
+   * @param request the register table request
+   * @param io the {@link FileIO} used to read the metadata file
+   * @throws ForbiddenException if a location is not authorized, or not 
contained in the namespace
+   * @throws IllegalStateException if the authorization plugin fails
+   */
+  void validateRegisterTable(String catalogName, Namespace namespace, 
Map<String, String> namespaceMetadata,
+      RegisterTableRequest request, FileIO io) {
+    Preconditions.checkArgument(namespace.levels().length == 1, "Hive does not 
support multi-level namespaces");
+    var databaseName = namespace.level(0);
+    var commandString = "register table " + request.name();
+    checkLocationAuthorized(catalogName, databaseName, namespaceMetadata, 
request.metadataLocation(), commandString);
+
+    var metadata = TableMetadataParser.read(io, request.metadataLocation());
+    checkLocationAuthorized(catalogName, databaseName, namespaceMetadata, 
metadata.location(), commandString);

Review Comment:
   ```suggestion
   ```
   
   We can remove this one since it falls within the scope of the CREATE TABLE 
authorization. I tested it with `testRegisterTableWithDeniedEmbeddedLocation`.
   
   <img width="1082" height="143" alt="Image" 
src="https://github.com/user-attachments/assets/a2e2e895-10df-462a-804d-21f288f7e46f";
 />



##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java:
##########
@@ -320,6 +325,10 @@ private LoadTableResponse loadTable(Map<String, String> 
vars) {
   private LoadTableResponse registerTable(Map<String, String> vars, Object 
body) {
       Namespace namespace = namespaceFromPathVars(vars);
       RegisterTableRequest request = castRequest(RegisterTableRequest.class, 
body);
+      request.validate();
+      Map<String, String> namespaceMetadata = 
asNamespaceCatalog.loadNamespaceMetadata(namespace);
+      FileIO io = ((HiveCatalog) catalog).io();

Review Comment:
   We don't need this FileIO if we delegate the permission check to Hive 
Metastore



##########
standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java:
##########
@@ -161,4 +175,133 @@ void validateStageCreateTable(String catalogName, 
Namespace namespace, Map<Strin
       throw new IllegalStateException("Failed to check privileges 
stage-create", e);
     }
   }
+
+  /**
+   * Enforces authorization for REGISTER_TABLE. The request's {@code 
metadataLocation} is fetched with the
+   * catalog's shared, service-level {@link FileIO}, so both that location and 
the {@code location()} embedded in
+   * the metadata file it points to (which becomes the table's HMS {@code 
StorageDescriptor.location}, and is what
+   * a later purge trusts as its deletion root, see {@link 
#validateDropTablePurge}) must be authorized. Otherwise
+   * REGISTER_TABLE is an arbitrary-file-read primitive that returns any 
metadata file's contents to the caller.
+   *
+   * <p>When no {@code HiveAuthorizer} is configured, falls back to requiring 
both locations to be contained in
+   * the namespace's external or managed root, since there is no policy to 
otherwise decide whether the caller may
+   * read an arbitrary location with service credentials.
+   *
+   * @param catalogName the Hive catalog name
+   * @param namespace the Iceberg namespace
+   * @param namespaceMetadata the Iceberg namespace metadata
+   * @param request the register table request
+   * @param io the {@link FileIO} used to read the metadata file
+   * @throws ForbiddenException if a location is not authorized, or not 
contained in the namespace
+   * @throws IllegalStateException if the authorization plugin fails
+   */
+  void validateRegisterTable(String catalogName, Namespace namespace, 
Map<String, String> namespaceMetadata,
+      RegisterTableRequest request, FileIO io) {
+    Preconditions.checkArgument(namespace.levels().length == 1, "Hive does not 
support multi-level namespaces");
+    var databaseName = namespace.level(0);
+    var commandString = "register table " + request.name();
+    checkLocationAuthorized(catalogName, databaseName, namespaceMetadata, 
request.metadataLocation(), commandString);
+
+    var metadata = TableMetadataParser.read(io, request.metadataLocation());
+    checkLocationAuthorized(catalogName, databaseName, namespaceMetadata, 
metadata.location(), commandString);
+  }
+
+  /**
+   * Enforces authorization for DROP_TABLE with {@code purge=true}. Purge 
deletes every file referenced by the
+   * table's current metadata using the catalog's shared, service-level {@link 
FileIO}, so the location must be
+   * authorized like any other DFS_URI access.
+   *
+   * <p>Unlike {@link #validateRegisterTable}, there is no 
namespace-containment fallback here: the structural
+   * fence in {@code HiveCatalog.dropTable} already restricts purge deletions 
to files under the table's own
+   * location regardless of whether a {@code HiveAuthorizer} is configured, so 
a deployment without one relies on
+   * that fence rather than this check.
+   *
+   * @param catalogName the Hive catalog name
+   * @param identifier the table identifier being dropped
+   * @param location the table's current location
+   * @throws ForbiddenException if the location is not authorized
+   * @throws IllegalStateException if the authorization plugin fails
+   */
+  void validateDropTablePurge(String catalogName, TableIdentifier identifier, 
String location) {
+    var authorizer = authorizerSupplier.get();
+    if (authorizer == null) {
+      LOG.info("No pre-event listener is configured for catalog {}, skipping 
drop-table-purge authorization for {}",
+          catalogName, identifier);
+      return;
+    }
+
+    var inputs = Collections.singletonList(
+        new 
HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.DFS_URI, 
location));
+    var builder = new HiveAuthzContext.Builder();
+    builder.setCommandString("drop table " + identifier.name());
+    try {
+      authorizer.checkPrivileges(HiveOperationType.DROPTABLE, inputs, 
Collections.emptyList(), builder.build());
+    } catch (HiveAccessControlException e) {
+      throw new ForbiddenException(e, e.getMessage());
+    } catch (HiveAuthzPluginException e) {
+      throw new IllegalStateException("Failed to check privileges 
drop-table-purge", e);
+    }
+  }
+
+  private void checkLocationAuthorized(String catalogName, String databaseName,
+      Map<String, String> namespaceMetadata, String location, String 
commandString) {
+    var authorizer = authorizerSupplier.get();
+    if (authorizer == null) {
+      LOG.info("No pre-event listener is configured for catalog {}, falling 
back to namespace containment for {}",
+          catalogName, location);
+      checkContainedInNamespace(databaseName, namespaceMetadata, location);
+      return;
+    }
+
+    var inputs = Collections.singletonList(
+        new 
HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.DFS_URI, 
location));
+    var builder = new HiveAuthzContext.Builder();
+    builder.setCommandString(commandString);
+    try {
+      authorizer.checkPrivileges(HiveOperationType.CREATETABLE, inputs, 
Collections.emptyList(), builder.build());
+    } catch (HiveAccessControlException e) {
+      throw new ForbiddenException(e, e.getMessage());
+    } catch (HiveAuthzPluginException e) {
+      throw new IllegalStateException("Failed to check privileges for " + 
commandString, e);
+    }
+  }
+
+  private void checkContainedInNamespace(String databaseName, Map<String, 
String> namespaceMetadata,
+      String location) {
+    var externalRoot = namespaceMetadata.get("location");
+    if (externalRoot != null && isContained(externalRoot, location)) {
+      return;
+    }
+    if (isContained(managedNamespaceLocation(databaseName), location)) {
+      return;
+    }
+    throw new ForbiddenException(
+        "Location %s is not authorized and is not contained in namespace %s", 
location, databaseName);
+  }
+
+  private String managedNamespaceLocation(String databaseName) {
+    var warehouseLocation = 
conf.get(HiveConf.ConfVars.METASTORE_WAREHOUSE.varname);
+    Preconditions.checkNotNull(warehouseLocation, "Warehouse location is not 
set: hive.metastore.warehouse.dir=null");
+    if (warehouseLocation.endsWith("/")) {
+      warehouseLocation = warehouseLocation.substring(0, 
warehouseLocation.length() - 1);
+    }
+    return String.format("%s/%s.db", warehouseLocation, databaseName);
+  }
+
+  /**
+   * Checks whether {@code candidate} resolves under {@code root}. Both are 
resolved via {@link Path#toUri()} and
+   * {@link java.net.URI#normalize()}, which -- unlike {@link Path}'s own 
normalization -- actually collapses
+   * {@code .}/{@code ..} segments; a plain string-prefix comparison on 
unnormalized paths would let a location
+   * like {@code root/../../elsewhere} pass a naive check while actually 
resolving outside {@code root}.
+   */
+  private static boolean isContained(String root, String candidate) {

Review Comment:
   Let's use `FileUtils.isPathWithinSubtree`, which is also used in [the 
storage 
handler](https://github.com/apache/hive/blob/9d9a51e103fddfeb855e46e1606b36cab770a56e/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/mapreduce/IcebergInputFormat.java#L223-L225).
 I know there are some traps for this type of comparison (e.g., some characters 
are not allowed in a path in URI, but object storage allows).
   
https://github.com/apache/hive/blob/9d9a51e103fddfeb855e46e1606b36cab770a56e/common/src/java/org/apache/hadoop/hive/common/FileUtils.java#L1254-L1256
   
   You may have to copy 
`org.apache.hadoop.hive.common.FileUtils.isPathWithinSubtree` to 
`org.apache.hadoop.hive.metastore.utils.FileUtils`.



-- 
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]

Reply via email to