This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo-access.git
The following commit(s) were added to refs/heads/main by this push:
new 5d7a944 improves validation in unquote() (#104)
5d7a944 is described below
commit 5d7a94492832121f507029d9d7e7627fd88e95ba
Author: Keith Turner <[email protected]>
AuthorDate: Tue Mar 10 16:06:38 2026 -0700
improves validation in unquote() (#104)
Co-authored-by: Christopher Tubbs <[email protected]>
---
.../accumulo/access/impl/AccessEvaluatorImpl.java | 3 +--
.../accumulo/access/impl/AccessExpressionImpl.java | 12 +++++++--
.../apache/accumulo/access/impl/CharsWrapper.java | 2 +-
.../accumulo/access/tests/AccessEvaluatorTest.java | 29 ++++++++++++++++++++--
4 files changed, 39 insertions(+), 7 deletions(-)
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
b/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
index 403872a..cc888d3 100644
---
a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
+++
b/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessEvaluatorImpl.java
@@ -99,8 +99,7 @@ public final class AccessEvaluatorImpl implements
AccessEvaluator {
}
} else if (isQuoteSymbol(c)) {
// should only see quote after a slash
- throw new IllegalArgumentException(
- "Illegal character after slash in auth String : " + auth);
+ throw new IllegalArgumentException("Unescaped quote in auth : " +
auth);
}
unescapedCopy[pos++] = c;
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java
b/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java
index 42fd3dc..2668152 100644
---
a/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java
+++
b/modules/core/src/main/java/org/apache/accumulo/access/impl/AccessExpressionImpl.java
@@ -78,8 +78,16 @@ public final class AccessExpressionImpl extends
AccessExpression {
public static CharSequence unquote(CharSequence term) {
final int len = term.length();
- if (len >= 2 && term.charAt(0) == '"' && term.charAt(len - 1) == '"') {
- term = len == 2 ? "" : AccessEvaluatorImpl.unescape(term.subSequence(1,
len - 1));
+ if (len >= 1) {
+ final boolean firstIsQuote = term.charAt(0) == '"';
+ final boolean lastIsQuote = term.charAt(len - 1) == '"';
+ if (firstIsQuote || lastIsQuote) {
+ if (len == 1 || (firstIsQuote != lastIsQuote)) {
+ throw new IllegalArgumentException("Unbalanced quotes : " + term);
+ }
+
+ term = len == 2 ? "" :
AccessEvaluatorImpl.unescape(term.subSequence(1, len - 1));
+ }
}
if (term.isEmpty()) {
throw new IllegalArgumentException("Empty strings are not legal
authorizations.");
diff --git
a/modules/core/src/main/java/org/apache/accumulo/access/impl/CharsWrapper.java
b/modules/core/src/main/java/org/apache/accumulo/access/impl/CharsWrapper.java
index 1929bd9..17c47c6 100644
---
a/modules/core/src/main/java/org/apache/accumulo/access/impl/CharsWrapper.java
+++
b/modules/core/src/main/java/org/apache/accumulo/access/impl/CharsWrapper.java
@@ -21,7 +21,7 @@ package org.apache.accumulo.access.impl;
import java.util.Arrays;
import java.util.Objects;
-public final class CharsWrapper implements CharSequence {
+final class CharsWrapper implements CharSequence {
private char[] wrapped;
private int offset;
private int len;
diff --git
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
b/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
index 8df473a..a3b0e3b 100644
---
a/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
+++
b/modules/core/src/test/java/org/apache/accumulo/access/tests/AccessEvaluatorTest.java
@@ -26,6 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.lang.reflect.Type;
+import java.nio.CharBuffer;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
@@ -38,6 +39,7 @@ import org.apache.accumulo.access.AccessEvaluator;
import org.apache.accumulo.access.AuthorizationValidator;
import org.apache.accumulo.access.InvalidAccessExpressionException;
import org.apache.accumulo.access.impl.AccessEvaluatorImpl;
+import org.apache.accumulo.access.impl.AccessExpressionImpl;
import org.junit.jupiter.api.Test;
import com.google.gson.Gson;
@@ -200,8 +202,31 @@ public class AccessEvaluatorTest {
assertEquals("九", access.unquote(access.quote("九")));
assertEquals("\"五十\"", access.quote("五十"));
assertEquals("五十", access.unquote(access.quote("五十")));
- assertThrows(IllegalArgumentException.class, () -> access.quote(""));
- assertThrows(IllegalArgumentException.class, () -> access.unquote(""));
+
+ var e = assertThrows(IllegalArgumentException.class, () ->
access.quote(""));
+ assertTrue(e.getMessage().contains("Empty string"));
+
+ testUnquoteError(access, "\"\"\"\"", "Unescaped quote");
+
+ for (var illegalInput : List.of("", "\"\"")) {
+ testUnquoteError(access, illegalInput, "Empty string");
+ }
+
+ for (var illegalInput : List.of("\"", "AB\"", "\"AB", "\"A", "B\"")) {
+ testUnquoteError(access, illegalInput, "Unbalanced quotes");
+ }
+ }
+
+ private static void testUnquoteError(Access access, String illegalInput,
+ String expectedErrorMsg) {
+ var e = assertThrows(IllegalArgumentException.class, () ->
access.unquote(illegalInput),
+ illegalInput);
+ assertTrue(e.getMessage().contains(expectedErrorMsg));
+ // test with an input that is not a string
+ CharSequence charSeq = CharBuffer.wrap(illegalInput);
+ e = assertThrows(IllegalArgumentException.class, () ->
AccessExpressionImpl.unquote(charSeq),
+ illegalInput);
+ assertTrue(e.getMessage().contains(expectedErrorMsg));
}
private static String unescape(String s) {
