ctubbsii commented on code in PR #102:
URL: https://github.com/apache/accumulo-access/pull/102#discussion_r2913579269
##########
modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java:
##########
@@ -77,15 +77,13 @@ public static CharSequence quote(CharSequence term) {
}
public static CharSequence unquote(CharSequence term) {
- if (term.equals("\"\"") || term.isEmpty()) {
- throw new IllegalArgumentException("Empty strings are not legal
authorizations.");
+ if (term.length() >= 2 && term.charAt(0) == '"' &&
term.charAt(term.length() - 1) == '"') {
Review Comment:
Yes, it executes an additional `AccessEvaluatorImpl.unescape("\"\"")`
instead of short-circuiting that, but that returns an empty string, so
`term.isEmpty()` is true. This was simpler than handling that case separately,
and the extra trivial execution in the rare, exceptional case of a quoted empty
string isn't worth adding an extra `if` case.
--
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]
