This is an automated email from the ASF dual-hosted git repository.
difin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new c3f56b4522e HIVE-29985: Validate orderByClause on compaction requests
(#6850)
c3f56b4522e is described below
commit c3f56b4522ebc0997178ead3ad7f49087239443c
Author: Dmitriy Fingerman <[email protected]>
AuthorDate: Tue Oct 6 19:48:40 2026 -0600
HIVE-29985: Validate orderByClause on compaction requests (#6850)
Co-authored-by: Dmitriy Fingerman <[email protected]>
---
.../mr/hive/compaction/IcebergQueryCompactor.java | 17 +++++-
.../ql/txn/compactor/CompactionQueryBuilder.java | 4 ++
.../utils/CompactionOrderByValidator.java | 63 ++++++++++++++++++++++
.../utils/TestCompactionOrderByValidator.java | 62 +++++++++++++++++++++
.../txn/jdbc/functions/CompactFunction.java | 10 ++++
5 files changed, 155 insertions(+), 1 deletion(-)
diff --git
a/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/compaction/IcebergQueryCompactor.java
b/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/compaction/IcebergQueryCompactor.java
index 6aa2855f82d..d30f8a8be58 100644
---
a/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/compaction/IcebergQueryCompactor.java
+++
b/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/compaction/IcebergQueryCompactor.java
@@ -29,6 +29,7 @@
import org.apache.hadoop.hive.metastore.api.CompactionType;
import org.apache.hadoop.hive.metastore.api.MetaException;
import org.apache.hadoop.hive.metastore.txn.entities.CompactionInfo;
+import org.apache.hadoop.hive.metastore.utils.CompactionOrderByValidator;
import org.apache.hadoop.hive.ql.Context.RewritePolicy;
import org.apache.hadoop.hive.ql.DriverUtils;
import org.apache.hadoop.hive.ql.ErrorMsg;
@@ -101,7 +102,7 @@ private String buildCompactionQuery(CompactorContext
context, String compactTabl
org.apache.hadoop.hive.ql.metadata.Table table =
Hive.get(conf).getTable(context.getTable().getDbName(),
context.getTable().getTableName());
Table icebergTable = IcebergTableUtil.getTable(conf, table.getTTable());
- String orderBy = ci.orderByClause == null ? "" : ci.orderByClause;
+ String orderBy = validatedOrderByClause(ci.orderByClause,
compactTableName);
String fileSizePredicate = buildMinorFileSizePredicate(ci,
compactTableName, conf, table);
String columnsList = "*";
@@ -251,4 +252,18 @@ private String buildPartitionPredicate(CompactionInfo ci,
PartitionSpec spec) th
return String.format("`partition`.%s = %s", column, literal);
}).collect(Collectors.joining(" AND "));
}
+
+ /**
+ * The compaction queue is written from a client-settable Thrift field:
treat its contents as
+ * untrusted and re-validate here, so a clause injected before (or around)
the enqueue-time
+ * check can never be concatenated into the privileged compaction query.
+ */
+ private static String validatedOrderByClause(String orderByClause, String
compactTableName) throws HiveException {
+ try {
+ CompactionOrderByValidator.validate(orderByClause);
+ } catch (IllegalArgumentException e) {
+ throw new HiveException("Refusing to compact " + compactTableName + ": "
+ e.getMessage(), e);
+ }
+ return orderByClause == null ? "" : orderByClause;
+ }
}
diff --git
a/ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactionQueryBuilder.java
b/ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactionQueryBuilder.java
index 1c5baa1ea9c..2280fdd8944 100644
---
a/ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactionQueryBuilder.java
+++
b/ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactionQueryBuilder.java
@@ -26,6 +26,7 @@
import org.apache.hadoop.hive.metastore.api.Partition;
import org.apache.hadoop.hive.metastore.api.StorageDescriptor;
import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.utils.CompactionOrderByValidator;
import org.apache.hadoop.hive.ql.exec.DDLPlanUtils;
import org.apache.hadoop.hive.ql.io.AcidDirectory;
import org.apache.hadoop.hive.ql.io.AcidUtils;
@@ -144,6 +145,9 @@ CompactionQueryBuilder setSourceTabForInsert(String
sourceTabForInsert) {
* @param orderByClause The ORDER BY clause to use for data reordering.
*/
public CompactionQueryBuilder setOrderByClause(String orderByClause) {
+ // The clause travels through the client-writable compaction queue; allow
only a plain ORDER BY
+ // over column names before it is concatenated into a compaction query.
+ CompactionOrderByValidator.validate(orderByClause);
this.orderByClause = orderByClause;
return this;
}
diff --git
a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/CompactionOrderByValidator.java
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/CompactionOrderByValidator.java
new file mode 100644
index 00000000000..9e4f1497485
--- /dev/null
+++
b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/CompactionOrderByValidator.java
@@ -0,0 +1,63 @@
+/*
+ * 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.hadoop.hive.metastore.utils;
+
+import java.util.regex.Pattern;
+
+/**
+ * Validates the ORDER BY clause carried on a compaction request.
+ *
+ * The clause originates from the client-settable Thrift field {@code
CompactionRequest.orderByClause},
+ * is persisted verbatim in the compaction queue and is later concatenated
into a query that the
+ * compaction worker executes with the compaction session's privileges. It
must therefore be treated
+ * as untrusted input everywhere it is consumed: only a plain {@code ORDER BY}
over column names
+ * (optionally qualified, quoted, with ASC/DESC and NULLS FIRST/LAST
modifiers) is accepted -
+ * exactly the shape HiveServer2 produces for
+ * {@code ALTER TABLE ... COMPACT ... ORDER BY col [ASC|DESC] [NULLS
FIRST|LAST], ...}.
+ * Anything else (subqueries, UNION tails, LIMIT, expressions, comments) is
rejected.
+ */
+public final class CompactionOrderByValidator {
+
+ private static final String IDENTIFIER = "(?:[A-Za-z0-9_]+|`(?:[^`]|``)+`)";
+ private static final String COLUMN = IDENTIFIER + "(?:\\s*\\.\\s*" +
IDENTIFIER + ")*";
+ private static final String ITEM =
+ COLUMN + "(?:\\s+(?i:ASC|DESC))?(?:\\s+(?i:NULLS)\\s+(?i:FIRST|LAST))?";
+ private static final Pattern ORDER_BY_PATTERN = Pattern.compile(
+ "\\s*(?i:ORDER)\\s+(?i:BY)\\s+" + ITEM + "(?:\\s*,\\s*" + ITEM +
")*\\s*");
+
+ private CompactionOrderByValidator() {
+ throw new UnsupportedOperationException("CompactionOrderByValidator should
not be instantiated");
+ }
+
+ /**
+ * Validates a compaction request ORDER BY clause.
+ *
+ * @param orderByClause the clause as stored on the request, e.g. "order by
`col` DESC nulls last, col2";
+ * null or blank is accepted (no reordering requested)
+ * @throws IllegalArgumentException if the clause is anything but a plain
ORDER BY over column names
+ */
+ public static void validate(String orderByClause) {
+ if (orderByClause == null || orderByClause.trim().isEmpty()) {
+ return;
+ }
+ if (!ORDER_BY_PATTERN.matcher(orderByClause).matches()) {
+ throw new IllegalArgumentException("orderByClause must be a plain ORDER
BY over column names " +
+ "(optionally with ASC/DESC and NULLS FIRST/LAST), got: " +
orderByClause);
+ }
+ }
+}
diff --git
a/standalone-metastore/metastore-common/src/test/java/org/apache/hadoop/hive/metastore/utils/TestCompactionOrderByValidator.java
b/standalone-metastore/metastore-common/src/test/java/org/apache/hadoop/hive/metastore/utils/TestCompactionOrderByValidator.java
new file mode 100644
index 00000000000..874c8ac0a96
--- /dev/null
+++
b/standalone-metastore/metastore-common/src/test/java/org/apache/hadoop/hive/metastore/utils/TestCompactionOrderByValidator.java
@@ -0,0 +1,62 @@
+/*
+ * 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.hadoop.hive.metastore.utils;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestCompactionOrderByValidator {
+
+ @Test
+ public void testNullAndBlankAccepted() {
+ CompactionOrderByValidator.validate(null);
+ CompactionOrderByValidator.validate("");
+ CompactionOrderByValidator.validate(" ");
+ }
+
+ @Test
+ public void testPlainOrderByAccepted() {
+ CompactionOrderByValidator.validate("order by col1");
+ CompactionOrderByValidator.validate("ORDER BY col1 ASC");
+ CompactionOrderByValidator.validate("order by col1 desc, col2");
+ CompactionOrderByValidator.validate("Order By col1 asc nulls first, col2
DESC NULLS LAST");
+ CompactionOrderByValidator.validate("order by `quoted col`, t.col2");
+ CompactionOrderByValidator.validate("order by 1, 2 desc");
+ }
+
+ @Test
+ public void testInjectionsRejected() {
+ // statement tail smuggling (privileged data copy)
+ assertRejected("order by 1 union all select * from finance.salaries");
+ // empty-rewrite data destruction
+ assertRejected("order by 1 limit 0");
+ // expressions / function calls
+ assertRejected("order by upper(col1)");
+ assertRejected("order by (select 1)");
+ // comment smuggling and statement separators
+ assertRejected("order by col1 -- comment");
+ assertRejected("order by col1; drop table t");
+ // missing ORDER BY prefix
+ assertRejected("col1 asc");
+ assertRejected("union all select 1");
+ }
+
+ private static void assertRejected(String clause) {
+ Assert.assertThrows(IllegalArgumentException.class, () ->
CompactionOrderByValidator.validate(clause));
+ }
+}
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/jdbc/functions/CompactFunction.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/jdbc/functions/CompactFunction.java
index ea43a4e74b4..ab01273ad4a 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/jdbc/functions/CompactFunction.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/jdbc/functions/CompactFunction.java
@@ -31,6 +31,7 @@
import
org.apache.hadoop.hive.metastore.txn.jdbc.commands.InsertCompactionRequestCommand;
import org.apache.hadoop.hive.metastore.txn.jdbc.MultiDataSourceJdbcResource;
import org.apache.hadoop.hive.metastore.txn.jdbc.TransactionalFunction;
+import org.apache.hadoop.hive.metastore.utils.CompactionOrderByValidator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
@@ -63,6 +64,15 @@ public CompactFunction(CompactionRequest rqst, long
openTxnTimeOutMillis, TxnSto
@Override
public CompactionResponse execute(MultiDataSourceJdbcResource jdbcResource)
throws MetaException {
+ // CompactionRequest.orderByClause is client-supplied over Thrift and is
later concatenated into
+ // a query the compaction worker executes with the compaction session's
privileges. Reject
+ // anything that is not a plain ORDER BY before it enters the queue.
+ try {
+ CompactionOrderByValidator.validate(rqst.getOrderByClause());
+ } catch (IllegalArgumentException e) {
+ throw new MetaException("Invalid orderByClause in compaction request for
" + rqst.getDbname() + "." +
+ rqst.getTablename() + ": " + e.getMessage());
+ }
// Put a compaction request in the queue.
TxnStore.MutexAPI.LockHandle handle = null;
try {
