Copilot commented on code in PR #18345:
URL: https://github.com/apache/iotdb/pull/18345#discussion_r3671624752
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/security/TreeAccessCheckVisitor.java:
##########
@@ -1136,25 +1136,31 @@ private TSStatus checkShowOrCountDatabasePermission(
@Override
public TSStatus visitInsertBase(InsertBaseStatement statement,
TreeAccessCheckContext context) {
context.setAuditLogOperation(AuditLogOperation.DML).setPrivilegeType(PrivilegeType.WRITE_DATA);
- for (PartialPath path : statement.getDevicePaths()) {
- // External users cannot modify the audit database.
- if (includeByAuditTreeDB(path)
- &&
!context.getUsername().equals(AuthorityChecker.INTERNAL_AUDIT_USER)) {
-
AUDIT_LOGGER.recordObjectAuthenticationAuditLog(context.setResult(false),
path::toString);
- return new TSStatus(TSStatusCode.NO_PERMISSION.getStatusCode())
-
.setMessage(getUnsupportedAuditDatabaseOperationMessage(TREE_MODEL_AUDIT_DATABASE));
- }
+ // External users cannot modify the audit database.
+ final PartialPath unsupportedAuditPath =
+ context.getUsername().equals(AuthorityChecker.INTERNAL_AUDIT_USER)
+ ? null
+ : statement
+ .getDevicePathsStream()
+ .filter(Audit::includeByAuditTreeDB)
+ .findFirst()
+ .orElse(null);
Review Comment:
visitInsertBase filters device paths with Audit::includeByAuditTreeDB, but
includeByAuditTreeDB dereferences prefixPath without a null check. If any
insert statement yields a null device path in getDevicePathsStream(), this will
throw an NPE. Filtering out nulls here prevents audit-path checking from
crashing authorization.
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/crud/InsertBaseStatement.java:
##########
@@ -205,6 +206,19 @@ public List<PartialPath> getPaths() {
return Collections.emptyList();
}
+ public Stream<PartialPath> getPathsStream() {
+ if (measurements == null) {
+ return Stream.empty();
+ }
+ return Arrays.stream(measurements)
+ .filter(Objects::nonNull)
+ .map(devicePath::concatAsMeasurementPath);
+ }
Review Comment:
getPathsStream() can throw a NullPointerException when devicePath is not set
(it currently only guards measurements == null, but then dereferences
devicePath via devicePath::concatAsMeasurementPath). Returning an empty stream
when devicePath is null makes this helper safer for partially-constructed/empty
statements and aligns with the existing empty-stream behavior.
This issue also appears on line 218 of the same file.
--
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]
