roryqi commented on code in PR #13478: URL: https://github.com/apache/gravitino/pull/13478#discussion_r4256571765
########## core/src/main/java/org/apache/gravitino/policy/rego/RestrictedRegoSourceParser.java: ########## @@ -0,0 +1,353 @@ +/* + * 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.gravitino.policy.rego; + +import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import org.apache.gravitino.policy.rego.RestrictedRegoLexer.Token; +import org.apache.gravitino.policy.rego.RestrictedRegoLexer.TokenType; +import org.apache.gravitino.policy.rego.RestrictedRegoProgram.ColumnMask; +import org.apache.gravitino.policy.rego.RestrictedRegoProgram.FilterBranch; +import org.apache.gravitino.policy.rego.RestrictedRegoProgram.MaskAction; +import org.apache.gravitino.policy.rego.RestrictedRegoProgram.MaskBranch; +import org.apache.gravitino.policy.rego.RestrictedRegoProgram.RowFilter; + +/** Builds a {@code restricted-rego-v1} program from lexer tokens. */ +final class RestrictedRegoSourceParser { + private final RestrictedRegoLexer lexer; + private Token current; + private int parenthesisNesting; + + RestrictedRegoSourceParser(String source) { + this.lexer = new RestrictedRegoLexer(source); + this.current = lexer.nextToken(); + } + + RestrictedRegoProgram parseProgram() { + RestrictedRegoProgram program; + if (check(TokenType.FILTER)) { + program = parseRowFilter(); + } else if (check(TokenType.MASK)) { + program = parseColumnMask(); + } else { + throw error("expected filter or mask rule"); + } + expect(TokenType.EOF, "expected end of program"); + return program; + } + + private RowFilter parseRowFilter() { + expect(TokenType.FILTER, "expected filter"); + expect(TokenType.ASSIGN, "expected := after filter"); + CanonicalExpression result = parseExpression(); + if (!match(TokenType.IF)) { + return new RowFilter(new ArrayList<>(), result); + } + + List<FilterBranch> branches = new ArrayList<>(); + branches.add(new FilterBranch(result, parseExpression())); + while (true) { + expect(TokenType.ELSE, "expected final else branch"); + expect(TokenType.ASSIGN, "expected := after else"); + CanonicalExpression nextResult = parseExpression(); + if (!match(TokenType.IF)) { + return new RowFilter(branches, nextResult); + } + branches.add(new FilterBranch(nextResult, parseExpression())); + } + } + + private ColumnMask parseColumnMask() { + expect(TokenType.MASK, "expected mask"); + expect(TokenType.ASSIGN, "expected := after mask"); + MaskAction action = parseMaskAction(); + if (!match(TokenType.IF)) { + return new ColumnMask(new ArrayList<>(), action); + } + + List<MaskBranch> branches = new ArrayList<>(); + branches.add(new MaskBranch(action, parseExpression())); + while (true) { + expect(TokenType.ELSE, "expected final else branch"); + expect(TokenType.ASSIGN, "expected := after else"); + MaskAction nextAction = parseMaskAction(); + if (!match(TokenType.IF)) { + return new ColumnMask(branches, nextAction); + } + branches.add(new MaskBranch(nextAction, parseExpression())); + } + } + + private MaskAction parseMaskAction() { + expect(TokenType.ACTION, "expected action"); + expect(TokenType.LPAREN, "expected ( after action"); + Token actionToken = expect(TokenType.STRING, "expected mask action string"); + String action = decodeString(actionToken); + expect(TokenType.RPAREN, "expected ) after mask action"); + try { + return MaskAction.fromValue(action); + } catch (IllegalArgumentException exception) { + throw error(actionToken, exception.getMessage()); + } + } + + private CanonicalExpression parseExpression() { + return parseOrExpression(); + } + + private CanonicalExpression parseOrExpression() { + List<CanonicalExpression> operands = new ArrayList<>(); + operands.add(parseAndExpression()); + while (match(TokenType.OR)) { + operands.add(parseAndExpression()); + } + return LogicalExpression.of(LogicalOperator.OR, operands); + } + + private CanonicalExpression parseAndExpression() { + List<CanonicalExpression> operands = new ArrayList<>(); + operands.add(parseNotExpression()); + while (match(TokenType.AND)) { + operands.add(parseNotExpression()); + } + return LogicalExpression.of(LogicalOperator.AND, operands); + } + + private CanonicalExpression parseNotExpression() { + int notCount = 0; + while (match(TokenType.NOT)) { + notCount++; + } Review Comment: Fixed in bd09cf0398. `parseNotExpression` now rejects as soon as the eighth `not` is consumed, before constructing a recursively deep AST. A parser-level regression test supplies 4090 `not` tokens within the 16 KiB source limit and asserts the normal source-depth error; the existing depth-eight test continues to pin that seven negations around an atomic predicate are accepted. ########## core/src/main/java/org/apache/gravitino/policy/rego/ExpressionValidator.java: ########## @@ -0,0 +1,146 @@ +/* + * 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.gravitino.policy.rego; + +import com.google.common.base.Preconditions; + +/** Semantic validation helpers for unresolved restricted Rego expressions. */ +final class ExpressionValidator { + private ExpressionValidator() {} + + static boolean isPredicate(CanonicalExpression expression) { + if (expression instanceof Comparison + || expression instanceof Not + || expression instanceof LogicalExpression + || expression instanceof GroupMembership) { + return true; + } + return expression instanceof Literal + && ((Literal) expression).literalType() == LiteralType.BOOLEAN; + } + + static boolean isContextOnly(CanonicalExpression expression) { + if (expression instanceof Column) { + return false; + } + if (expression instanceof Comparison) { + Comparison comparison = (Comparison) expression; + return isContextOnly(comparison.left()) && isContextOnly(comparison.right()); + } + if (expression instanceof Not) { + return isContextOnly(((Not) expression).operand()); + } + if (expression instanceof LogicalExpression) { + for (CanonicalExpression child : ((LogicalExpression) expression).operands()) { + if (!isContextOnly(child)) { + return false; + } + } + } + return true; + } + + static void validateComparison( + ComparisonOperator operator, CanonicalExpression left, CanonicalExpression right) { + if (operator == ComparisonOperator.IN) { + Preconditions.checkArgument( + right instanceof LiteralArray, "right operand of in must be a literal array"); + Preconditions.checkArgument( + left instanceof Column || left instanceof SessionUser, + "left operand of in must be col(...) or session_user()"); + if (left instanceof SessionUser) { + LiteralArray array = (LiteralArray) right; + Preconditions.checkArgument( + array.elementType() == LiteralType.STRING, + "session_user() can only be tested against a string array"); + } + return; + } + + Preconditions.checkArgument( + !(left instanceof LiteralArray) && !(right instanceof LiteralArray), + "%s does not support array operands", + operator.sourceToken()); + + boolean equality = operator == ComparisonOperator.EQ || operator == ComparisonOperator.NEQ; + if (isColumnLiteralPair(left, right)) { + Literal literal = left instanceof Literal ? (Literal) left : (Literal) right; + Preconditions.checkArgument( + literal.literalType() != LiteralType.NULL || equality, "null supports only == and !="); + Preconditions.checkArgument( + literal.literalType() != LiteralType.BOOLEAN || equality, + "boolean literal supports only == and !="); + return; + } + + if (isColumnSessionUserPair(left, right)) { + Preconditions.checkArgument(equality, "session_user() supports only == and !="); + return; + } + + if (isSessionUserStringPair(left, right)) { + Preconditions.checkArgument(equality, "session_user() supports only == and !="); + return; + } + + throw new IllegalArgumentException( + String.format("Unsupported operands for %s in restricted-rego-v1", operator.sourceToken())); + } + + static void validateUnicodeScalars(String value, String description) { + for (int index = 0; index < value.length(); index++) { + char current = value.charAt(index); + Preconditions.checkArgument(current != '\0', "%s cannot contain NUL", description); Review Comment: Clarified in bd09cf0398. Escaped control characters other than NUL are intentionally decoded and preserved to retain JSON string semantics and exact authored values; U+007F is also preserved in literal or escaped form. Both documents now state that contract explicitly, and the parser test pins an escaped newline in a column name, an escaped tab in a group name, and U+007F in a string value. Raw U+0000 through U+001F and decoded NUL remain rejected. -- 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]
