This is an automated email from the ASF dual-hosted git repository.
CalvinKirs pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 9d499a9400f [fix](auth) Reject privilege object names with more than
three parts in GRANT/REVOKE (#68660)
9d499a9400f is described below
commit 9d499a9400f2bd245eec3951b7cb38ac7e92c84f
Author: Calvin Kirs <[email protected]>
AuthorDate: Wed Sep 30 18:24:47 2026 +0800
[fix](auth) Reject privilege object names with more than three parts in
GRANT/REVOKE (#68660)
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
The grammar of table privilege statements accepts an object name with
any number of dot-separated parts (`multipartIdentifierOrAsterisk`), but
`LogicalPlanBuilder` only turned 1, 2 or 3 parts into a `TablePattern`
(db, db.tbl, ctl.db.tbl) and left it null for anything else. The null
pattern then reached the commands and surfaced as an internal error:
```sql
GRANT SELECT_PRIV ON a.b.c.d TO 'u'@'%';
-- errCode = 2, detailMessage = tablePattern is null
REVOKE SELECT_PRIV ON a.b.c.d FROM 'u'@'%';
-- errCode = 2, detailMessage = Cannot invoke
"org.apache.doris.analysis.TablePattern.analyze()" because "this.tablePattern"
is null
```
GRANT failed on the `Objects.requireNonNull` in the
`GrantTablePrivilegeCommand` constructor; REVOKE had no such check and
hit a NullPointerException in `validate()`. For comparison, MySQL
rejects the same statements as a syntax error (`ERROR 1064 ... near
'.c.d TO ...'`).
Fix:
- Build the `TablePattern` of both statements in one helper,
`parsePrivilegeTablePattern`, which throws a `ParseException` naming the
accepted shapes for any other part count:
```
Privilege object name should be db, db.tbl or ctl.db.tbl, but got:
a.b.c.d(line 1, pos 21)
== SQL ==
GRANT SELECT_PRIV ON a.b.c.d TO 'u'@'%'
---------------------^^^
```
- `RevokeTablePrivilegeCommand` now rejects null arguments in its
constructor, the same as `GrantTablePrivilegeCommand`. The `tablePattern
!= null` checks in `RevokeTablePrivilegeCommand.validate()` and
`Auth.revokeTablePrivilegeCommand()` are removed since the pattern can
no longer be null; the one in `Auth` would have skipped the revoke
silently instead of failing.
### Release note
GRANT/REVOKE with a table privilege object name of more than three parts
(for example `a.b.c.d`) now fails with the parse error "Privilege object
name should be db, db.tbl or ctl.db.tbl" instead of an internal null
pointer error.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- FE UT: `GrantTablePrivilegeCommandTest` and
`RevokeTablePrivilegeCommandTest` gain `testObjectName` (1/2/3-part
mapping) and `testObjectNameWithTooManyParts` (4/5 parts and `*.*.*.*`
rejected); the rejection cases fail without the fix. 14 privilege and
parser test classes (63 tests) pass.
- Regression: new `account_p0/test_grant_revoke_object_name`, run on a
local cluster with the FE built from this branch.
- Manual: the statements above, plus `a.b.c.d.e`, `` `a`.`b`.`c`.`d` ``
and `GRANT ... TO ROLE`, all return the parse error; `a.b.c` still goes
through the normal path.
---
.../org/apache/doris/mysql/privilege/Auth.java | 10 ++--
.../doris/nereids/parser/LogicalPlanBuilder.java | 68 +++++++---------------
.../commands/RevokeTablePrivilegeCommand.java | 13 ++---
.../commands/GrantTablePrivilegeCommandTest.java | 29 +++++++++
.../commands/RevokeTablePrivilegeCommandTest.java | 29 +++++++++
.../test_grant_revoke_object_name.groovy | 51 ++++++++++++++++
6 files changed, 139 insertions(+), 61 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java
index e25720f63e7..69f2c33e3fd 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/Auth.java
@@ -926,12 +926,10 @@ public class Auth implements Writable {
// revoke table
public void revokeTablePrivilegeCommand(RevokeTablePrivilegeCommand
command) throws DdlException {
- if (command.getTablePattern() != null) {
- PrivBitSet privs = PrivBitSet.of(command.getPrivileges());
- revokeInternal(command.getUserIdentity().orElse(null),
command.getRole().orElse(null),
- command.getTablePattern(), privs,
command.getColPrivileges(),
- true /* err on non exist */, false /* is replay */);
- }
+ PrivBitSet privs = PrivBitSet.of(command.getPrivileges());
+ revokeInternal(command.getUserIdentity().orElse(null),
command.getRole().orElse(null),
+ command.getTablePattern(), privs, command.getColPrivileges(),
+ true /* err on non exist */, false /* is replay */);
}
public void replayRevoke(PrivInfo info) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
index 443e0b50a3a..a7c520d2f5d 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
@@ -9182,35 +9182,28 @@ public class LogicalPlanBuilder extends
DorisParserBaseVisitor<Object> {
return new TransactionRollbackCommand();
}
+ /**
+ * The object of a table privilege is db, db.tbl or ctl.db.tbl;
TablePattern.analyze() checks its wildcards.
+ */
+ private TablePattern
parsePrivilegeTablePattern(DorisParser.MultipartIdentifierOrAsteriskContext
ctx) {
+ List<String> parts = visitMultipartIdentifierOrAsterisk(ctx);
+ switch (parts.size()) {
+ case 1:
+ return new TablePattern(parts.get(0), "");
+ case 2:
+ return new TablePattern(parts.get(0), parts.get(1));
+ case 3:
+ return new TablePattern(parts.get(0), parts.get(1),
parts.get(2));
+ default:
+ throw new ParseException("Privilege object name should be db,
db.tbl or ctl.db.tbl, but got: "
+ + String.join(".", parts), ctx);
+ }
+ }
+
@Override
public LogicalPlan
visitGrantTablePrivilege(DorisParser.GrantTablePrivilegeContext ctx) {
List<AccessPrivilegeWithCols> accessPrivilegeWithCols =
visitPrivilegeList(ctx.privilegeList());
-
- List<String> parts =
visitMultipartIdentifierOrAsterisk(ctx.multipartIdentifierOrAsterisk());
- int size = parts.size();
-
- if (size < 1) {
- throw new AnalysisException("grant table privilege statement
missing parameters");
- }
-
- TablePattern tablePattern = null;
- if (size == 1) {
- String db = parts.get(size - 1);
- tablePattern = new TablePattern(db, "");
- }
-
- if (size == 2) {
- String db = parts.get(size - 2);
- String tbl = parts.get(size - 1);
- tablePattern = new TablePattern(db, tbl);
- }
-
- if (size == 3) {
- String ctl = parts.get(size - 3);
- String db = parts.get(size - 2);
- String tbl = parts.get(size - 1);
- tablePattern = new TablePattern(ctl, db, tbl);
- }
+ TablePattern tablePattern =
parsePrivilegeTablePattern(ctx.multipartIdentifierOrAsterisk());
Optional<UserIdentity> userIdentity = Optional.empty();
Optional<String> role = Optional.empty();
@@ -9356,28 +9349,7 @@ public class LogicalPlanBuilder extends
DorisParserBaseVisitor<Object> {
@Override
public LogicalPlan
visitRevokeTablePrivilege(DorisParser.RevokeTablePrivilegeContext ctx) {
List<AccessPrivilegeWithCols> accessPrivilegeWithCols =
visitPrivilegeList(ctx.privilegeList());
-
- List<String> parts =
visitMultipartIdentifierOrAsterisk(ctx.multipartIdentifierOrAsterisk());
- int size = parts.size();
-
- TablePattern tablePattern = null;
- if (size == 1) {
- String db = parts.get(size - 1);
- tablePattern = new TablePattern(db, "");
- }
-
- if (size == 2) {
- String db = parts.get(size - 2);
- String tbl = parts.get(size - 1);
- tablePattern = new TablePattern(db, tbl);
- }
-
- if (size == 3) {
- String ctl = parts.get(size - 3);
- String db = parts.get(size - 2);
- String tbl = parts.get(size - 1);
- tablePattern = new TablePattern(ctl, db, tbl);
- }
+ TablePattern tablePattern =
parsePrivilegeTablePattern(ctx.multipartIdentifierOrAsterisk());
Optional<UserIdentity> userIdentity = Optional.empty();
Optional<String> role = Optional.empty();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/RevokeTablePrivilegeCommand.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/RevokeTablePrivilegeCommand.java
index 1748d6bd57b..5213df7cf01 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/RevokeTablePrivilegeCommand.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/RevokeTablePrivilegeCommand.java
@@ -38,6 +38,7 @@ import org.apache.commons.collections4.MapUtils;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
@@ -59,10 +60,10 @@ public class RevokeTablePrivilegeCommand extends Command
implements ForwardWithS
public RevokeTablePrivilegeCommand(List<AccessPrivilegeWithCols>
accessPrivileges, TablePattern tablePattern,
Optional<UserIdentity> userIdentity, Optional<String> role) {
super(PlanType.REVOKE_TABLE_PRIVILEGE_COMMAND);
- this.accessPrivileges = accessPrivileges;
- this.tablePattern = tablePattern;
- this.userIdentity = userIdentity;
- this.role = role;
+ this.accessPrivileges = Objects.requireNonNull(accessPrivileges,
"accessPrivileges is null");
+ this.tablePattern = Objects.requireNonNull(tablePattern, "tablePattern
is null");
+ this.userIdentity = Objects.requireNonNull(userIdentity, "userIdentity
is null");
+ this.role = Objects.requireNonNull(role, "role is null");
}
@Override
@@ -100,9 +101,7 @@ public class RevokeTablePrivilegeCommand extends Command
implements ForwardWithS
}
// Revoke operation obey the same rule as Grant operation. reuse the
same method
- if (tablePattern != null) {
- GrantTablePrivilegeCommand.checkTablePrivileges(privileges,
tablePattern, colPrivileges);
- }
+ GrantTablePrivilegeCommand.checkTablePrivileges(privileges,
tablePattern, colPrivileges);
}
@Override
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/GrantTablePrivilegeCommandTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/GrantTablePrivilegeCommandTest.java
index 65f80e75732..9683ed5b161 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/GrantTablePrivilegeCommandTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/GrantTablePrivilegeCommandTest.java
@@ -23,6 +23,7 @@ import org.apache.doris.catalog.AccessPrivilege;
import org.apache.doris.catalog.AccessPrivilegeWithCols;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.DdlException;
+import org.apache.doris.nereids.exceptions.ParseException;
import org.apache.doris.nereids.parser.NereidsParser;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.utframe.TestWithFeService;
@@ -191,4 +192,32 @@ public class GrantTablePrivilegeCommandTest extends
TestWithFeService {
LogicalPlan plan = new NereidsParser().parseSingle(sql);
((Command) plan).run(connectContext, null);
}
+
+ @Test
+ public void testObjectName() {
+ NereidsParser nereidsParser = new NereidsParser();
+ String[][] cases = {
+ {"GRANT SELECT_PRIV ON test TO 'jack'", "test.*"},
+ {"GRANT SELECT_PRIV ON test.test_table TO 'jack'",
"test.test_table"},
+ {"GRANT SELECT_PRIV ON internal.test.test_table TO 'jack'",
"internal.test.test_table"},
+ };
+ for (String[] c : cases) {
+ LogicalPlan plan = nereidsParser.parseSingle(c[0]);
+ Assertions.assertTrue(plan instanceof GrantTablePrivilegeCommand,
c[0]);
+ Assertions.assertEquals(c[1], ((GrantTablePrivilegeCommand)
plan).getTablePattern().toString(), c[0]);
+ }
+ }
+
+ @Test
+ public void testObjectNameWithTooManyParts() {
+ NereidsParser nereidsParser = new NereidsParser();
+ for (String name : new String[] {"a.b.c.d", "*.*.*.*", "a.b.c.d.e"}) {
+ String sql = "GRANT SELECT_PRIV ON " + name + " TO 'jack'";
+ ParseException exception =
Assertions.assertThrows(ParseException.class,
+ () -> nereidsParser.parseSingle(sql), sql);
+ Assertions.assertTrue(exception.getMessage().contains(
+ "Privilege object name should be db, db.tbl or ctl.db.tbl,
but got: " + name),
+ exception.getMessage());
+ }
+ }
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/RevokeTablePrivilegeCommandTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/RevokeTablePrivilegeCommandTest.java
index 9c04d120eb1..06738995913 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/RevokeTablePrivilegeCommandTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/RevokeTablePrivilegeCommandTest.java
@@ -20,6 +20,7 @@ package org.apache.doris.nereids.trees.plans.commands;
import org.apache.doris.analysis.TablePattern;
import org.apache.doris.catalog.AccessPrivilege;
import org.apache.doris.catalog.AccessPrivilegeWithCols;
+import org.apache.doris.nereids.exceptions.ParseException;
import org.apache.doris.nereids.parser.NereidsParser;
import org.apache.doris.nereids.trees.plans.logical.LogicalPlan;
import org.apache.doris.utframe.TestWithFeService;
@@ -95,4 +96,32 @@ public class RevokeTablePrivilegeCommandTest extends
TestWithFeService {
Assertions.assertTrue(revokeplan2 instanceof
RevokeTablePrivilegeCommand);
Assertions.assertDoesNotThrow(() -> ((RevokeTablePrivilegeCommand)
revokeplan2).run(connectContext, null));
}
+
+ @Test
+ public void testObjectName() {
+ NereidsParser nereidsParser = new NereidsParser();
+ String[][] cases = {
+ {"REVOKE SELECT_PRIV ON test FROM 'jack'", "test.*"},
+ {"REVOKE SELECT_PRIV ON test.test_table FROM 'jack'",
"test.test_table"},
+ {"REVOKE SELECT_PRIV ON internal.test.test_table FROM 'jack'",
"internal.test.test_table"},
+ };
+ for (String[] c : cases) {
+ LogicalPlan plan = nereidsParser.parseSingle(c[0]);
+ Assertions.assertTrue(plan instanceof RevokeTablePrivilegeCommand,
c[0]);
+ Assertions.assertEquals(c[1], ((RevokeTablePrivilegeCommand)
plan).getTablePattern().toString(), c[0]);
+ }
+ }
+
+ @Test
+ public void testObjectNameWithTooManyParts() {
+ NereidsParser nereidsParser = new NereidsParser();
+ for (String name : new String[] {"a.b.c.d", "*.*.*.*", "a.b.c.d.e"}) {
+ String sql = "REVOKE SELECT_PRIV ON " + name + " FROM 'jack'";
+ ParseException exception =
Assertions.assertThrows(ParseException.class,
+ () -> nereidsParser.parseSingle(sql), sql);
+ Assertions.assertTrue(exception.getMessage().contains(
+ "Privilege object name should be db, db.tbl or ctl.db.tbl,
but got: " + name),
+ exception.getMessage());
+ }
+ }
}
diff --git
a/regression-test/suites/account_p0/test_grant_revoke_object_name.groovy
b/regression-test/suites/account_p0/test_grant_revoke_object_name.groovy
new file mode 100644
index 00000000000..6801a383dd0
--- /dev/null
+++ b/regression-test/suites/account_p0/test_grant_revoke_object_name.groovy
@@ -0,0 +1,51 @@
+// 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.
+
+suite("test_grant_revoke_object_name") {
+ sql """DROP USER IF EXISTS 'test_grant_revoke_object_name_user'"""
+ sql """DROP ROLE IF EXISTS test_grant_revoke_object_name_role"""
+ sql """DROP DATABASE IF EXISTS test_grant_revoke_object_name_db"""
+ sql """CREATE DATABASE test_grant_revoke_object_name_db"""
+ sql """CREATE USER 'test_grant_revoke_object_name_user'"""
+ sql """CREATE ROLE test_grant_revoke_object_name_role"""
+
+ // db, db.tbl and ctl.db.tbl are the only object name shapes a table
privilege takes
+ sql """GRANT SELECT_PRIV ON test_grant_revoke_object_name_db TO
'test_grant_revoke_object_name_user'"""
+ sql """REVOKE SELECT_PRIV ON test_grant_revoke_object_name_db FROM
'test_grant_revoke_object_name_user'"""
+ sql """GRANT SELECT_PRIV ON test_grant_revoke_object_name_db.* TO
'test_grant_revoke_object_name_user'"""
+ sql """REVOKE SELECT_PRIV ON test_grant_revoke_object_name_db.* FROM
'test_grant_revoke_object_name_user'"""
+ sql """GRANT SELECT_PRIV ON internal.test_grant_revoke_object_name_db.* TO
'test_grant_revoke_object_name_user'"""
+ sql """REVOKE SELECT_PRIV ON internal.test_grant_revoke_object_name_db.*
FROM 'test_grant_revoke_object_name_user'"""
+
+ // four or more parts are rejected while parsing, for both a user and a
role
+ test {
+ sql """GRANT SELECT_PRIV ON a.b.c.d TO
'test_grant_revoke_object_name_user'"""
+ exception "Privilege object name should be db, db.tbl or ctl.db.tbl,
but got: a.b.c.d"
+ }
+ test {
+ sql """REVOKE SELECT_PRIV ON a.b.c.d FROM
'test_grant_revoke_object_name_user'"""
+ exception "Privilege object name should be db, db.tbl or ctl.db.tbl,
but got: a.b.c.d"
+ }
+ test {
+ sql """GRANT SELECT_PRIV ON
internal.test_grant_revoke_object_name_db.t.c TO ROLE
test_grant_revoke_object_name_role"""
+ exception "Privilege object name should be db, db.tbl or ctl.db.tbl,
but got: internal.test_grant_revoke_object_name_db.t.c"
+ }
+ test {
+ sql """REVOKE SELECT_PRIV ON *.*.*.* FROM ROLE
test_grant_revoke_object_name_role"""
+ exception "Privilege object name should be db, db.tbl or ctl.db.tbl,
but got: *.*.*.*"
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
