This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 396ff13db6 Follow-up to "Report malformed JNDIRealm user patterns..."
396ff13db6 is described below
commit 396ff13db6e32fc9d4b2fe08e54c64d729a63da6
Author: Mark Thomas <[email protected]>
AuthorDate: Sun Oct 11 17:50:20 2026 +0100
Follow-up to "Report malformed JNDIRealm user patterns..."
Further tighten-up the parsing.
Co-authored-by: Claude Sonnet 5.5
---
java/org/apache/catalina/realm/JNDIRealm.java | 111 ++++++++++++++--------
test/org/apache/catalina/realm/TestJNDIRealm.java | 73 +++++++++++++-
2 files changed, 142 insertions(+), 42 deletions(-)
diff --git a/java/org/apache/catalina/realm/JNDIRealm.java
b/java/org/apache/catalina/realm/JNDIRealm.java
index bdb4a2ffb3..ec3474b4de 100644
--- a/java/org/apache/catalina/realm/JNDIRealm.java
+++ b/java/org/apache/catalina/realm/JNDIRealm.java
@@ -2976,52 +2976,83 @@ public class JNDIRealm extends RealmBase {
*/
protected String[] parseUserPatternString(String userPatternString) {
- if (userPatternString != null) {
- List<String> pathList = new ArrayList<>();
- int startParenLoc = userPatternString.indexOf('(');
- if (startParenLoc == -1) {
- // no parens here; return whole thing
- return new String[] { userPatternString };
- }
- while (startParenLoc > -1) {
- // weed out escaped open parens and parens enclosing the
- // whole statement (in the case of valid LDAP search
- // strings: (|(something)(somethingelse))
- while (startParenLoc > -1 && startParenLoc <
userPatternString.length() - 1 &&
- ((userPatternString.charAt(startParenLoc + 1) == '|')
||
- (startParenLoc != 0 &&
userPatternString.charAt(startParenLoc - 1) == '\\'))) {
- startParenLoc = userPatternString.indexOf('(',
startParenLoc + 1);
- }
- if (startParenLoc == -1) {
- break;
- }
- if (startParenLoc == userPatternString.length() - 1) {
- // an open paren at the end cannot start a pattern
- throw new
IllegalArgumentException(sm.getString("jndiRealm.invalidUserPattern",
- userPatternString));
+ if (userPatternString == null) {
+ return null;
+ }
+
+ List<String> pathList = new ArrayList<>();
+ // Number of open "(|" wrappers around the patterns
+ int wrapperDepth = 0;
+ boolean unescapedOpenParen = false;
+ // A close paren without a matching open paren
+ boolean strayCloseParen = false;
+ // Text other than whitespace outside of any pattern
+ boolean strayText = false;
+ int length = userPatternString.length();
+ int i = 0;
+ while (i < length) {
+ char c = userPatternString.charAt(i);
+ if (c == '\\') {
+ // Skip the escaped character
+ strayText = true;
+ i += 2;
+ } else if (c == '(') {
+ unescapedOpenParen = true;
+ if (i + 1 < length && userPatternString.charAt(i + 1) == '|') {
+ // Parens enclosing the whole statement. Valid LDAP search
strings: (|(something)(somethingelse))
+ wrapperDepth++;
+ i += 2;
+ } else {
+ int endParenLoc =
findUnescapedCloseParen(userPatternString, i + 1);
+ if (endParenLoc == -1) {
+ // no matching close paren for this pattern
+ throw new IllegalArgumentException(
+ sm.getString("jndiRealm.invalidUserPattern",
userPatternString));
+ }
+ pathList.add(userPatternString.substring(i + 1,
endParenLoc));
+ i = endParenLoc + 1;
}
- int endParenLoc = userPatternString.indexOf(')', startParenLoc
+ 1);
- // weed out escaped end-parens
- while (endParenLoc > 0 && userPatternString.charAt(endParenLoc
- 1) == '\\') {
- endParenLoc = userPatternString.indexOf(')', endParenLoc +
1);
+ } else if (c == ')') {
+ if (wrapperDepth == 0) {
+ strayCloseParen = true;
+ } else {
+ wrapperDepth--;
}
- if (endParenLoc == -1) {
- // no matching close paren for this pattern
- throw new
IllegalArgumentException(sm.getString("jndiRealm.invalidUserPattern",
- userPatternString));
+ i++;
+ } else {
+ if (!Character.isWhitespace(c)) {
+ strayText = true;
}
- String nextPathPart =
userPatternString.substring(startParenLoc + 1, endParenLoc);
- pathList.add(nextPathPart);
- startParenLoc = userPatternString.indexOf('(', endParenLoc +
1);
+ i++;
}
- if (pathList.isEmpty()) {
- // every paren was escaped, nothing to separate: treat the
- // whole string as the pattern
- return new String[] { userPatternString };
+ }
+
+ if (!unescapedOpenParen) {
+ // No open parens (or only escaped open parens) here, so this is a
plain pattern. A close paren is not
+ // special in a distinguished name; return whole thing
+ return new String[] { userPatternString };
+ }
+ if (wrapperDepth != 0 || strayCloseParen || pathList.isEmpty() ||
strayText) {
+ // unclosed wrapper, extra close paren, nothing but wrappers or
text between the patterns
+ throw new
IllegalArgumentException(sm.getString("jndiRealm.invalidUserPattern",
userPatternString));
+ }
+ return pathList.toArray(new String[0]);
+ }
+
+
+ private static int findUnescapedCloseParen(String value, int start) {
+ int i = start;
+ while (i < value.length()) {
+ char c = value.charAt(i);
+ if (c == '\\') {
+ i += 2;
+ } else if (c == ')') {
+ return i;
+ } else {
+ i++;
}
- return pathList.toArray(new String[0]);
}
- return null;
+ return -1;
}
diff --git a/test/org/apache/catalina/realm/TestJNDIRealm.java
b/test/org/apache/catalina/realm/TestJNDIRealm.java
index c8a870dc95..33323d9faa 100644
--- a/test/org/apache/catalina/realm/TestJNDIRealm.java
+++ b/test/org/apache/catalina/realm/TestJNDIRealm.java
@@ -168,10 +168,79 @@ public class TestJNDIRealm {
realm.setUserPattern("(");
}
- @Test(expected = IllegalArgumentException.class)
+ @Test
public void testParseUserPatternStringEscapedOpenParenOnly() throws
Exception {
JNDIRealm realm = new JNDIRealm();
- realm.setUserPattern("cn=\\(");
+ // Escaped parens are treated as a single pattern, wherever they appear
+ Assert.assertArrayEquals(new String[] { "cn=\\(" },
realm.parseUserPatternString("cn=\\("));
+ Assert.assertArrayEquals(new String[] { "cn=\\(x\\)" },
realm.parseUserPatternString("cn=\\(x\\)"));
+ }
+
+ @Test
+ public void testParseUserPatternStringEscapedCloseParenInPattern() throws
Exception {
+ JNDIRealm realm = new JNDIRealm();
+ Assert.assertArrayEquals(new String[] { "cn={0}\\)x" },
realm.parseUserPatternString("(cn={0}\\)x)"));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testParseUserPatternStringWrapperWithoutPatterns() throws
Exception {
+ // Previously looped forever
+ new JNDIRealm().parseUserPatternString("(|x)");
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testParseUserPatternStringUnclosedWrapper() throws Exception {
+ new JNDIRealm().parseUserPatternString("(|(cn={0})");
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testParseUserPatternStringExtraCloseParen() throws Exception {
+ new JNDIRealm().parseUserPatternString("(cn={0}))");
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testParseUserPatternStringTextBetweenPatterns() throws
Exception {
+ new JNDIRealm().parseUserPatternString("(cn={0})junk(cn={0},o=myorg)");
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testParseUserPatternStringTextBeforePattern() throws Exception
{
+ new JNDIRealm().parseUserPatternString("junk(cn={0})");
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testParseUserPatternStringTextAfterPattern() throws Exception {
+ new JNDIRealm().parseUserPatternString("(cn={0})junk");
+ }
+
+ @Test
+ public void testParseUserPatternStringWhitespaceBetweenPatterns() throws
Exception {
+ JNDIRealm realm = new JNDIRealm();
+ Assert.assertArrayEquals(new String[] { "cn={0}", "cn={0},o=myorg" },
+ realm.parseUserPatternString("(cn={0}) (cn={0},o=myorg)"));
+ Assert.assertArrayEquals(new String[] { "cn={0}", "cn={0},o=myorg" },
+ realm.parseUserPatternString("(|(cn={0}) (cn={0},o=myorg) )"));
+ }
+
+ @Test
+ public void testParseUserPatternStringCloseParenWithoutOpenParen() throws
Exception {
+ // A close paren is not special in a DN so a pattern with no open
paren is a single pattern
+ Assert.assertArrayEquals(new String[] {
"cn={0},ou=Users),dc=example,dc=com" },
+ new
JNDIRealm().parseUserPatternString("cn={0},ou=Users),dc=example,dc=com"));
+ }
+
+ @Test
+ public void testSetUserPatternRejectedPatternKeepsPrevious() throws
Exception {
+ JNDIRealm realm = new JNDIRealm();
+ realm.setUserPattern("(cn={0})(cn={0},o=myorg)");
+ try {
+ realm.setUserPattern("(cn={0}");
+ Assert.fail("Unbalanced pattern must be rejected");
+ } catch (IllegalArgumentException e) {
+ // Expected
+ }
+ Assert.assertEquals("(cn={0})(cn={0},o=myorg)",
realm.getUserPattern());
+ Assert.assertArrayEquals(new String[] { "cn={0}", "cn={0},o=myorg" },
realm.userPatternArray);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
