From d06f22e6132ee3459b692aecd8876b8788b9143c Mon Sep 17 00:00:00 2001
From: coulbeck <coulbeck@localhost>
Date: Thu, 26 Apr 2007 19:01:03 +0000
Subject: [PATCH] Fix issue 1531: Improper filter allowed in targattrfilters argument.

---
 opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java                             |   41 ++++++++++---
 opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java |    4 +
 opends/src/server/org/opends/server/messages/ProtocolMessages.java                             |   16 +++++
 opends/tests/unit-tests-testng/src/server/org/opends/server/types/SearchFilterTests.java       |    5 +
 opends/src/server/org/opends/server/types/SearchFilter.java                                    |   55 +++++++++++++-----
 opends/src/server/org/opends/server/messages/AciMessages.java                                  |    4 
 opends/src/server/org/opends/server/messages/CoreMessages.java                                 |   14 ++++
 7 files changed, 110 insertions(+), 29 deletions(-)

diff --git a/opends/src/server/org/opends/server/messages/AciMessages.java b/opends/src/server/org/opends/server/messages/AciMessages.java
index 58cb756..d140858 100644
--- a/opends/src/server/org/opends/server/messages/AciMessages.java
+++ b/opends/src/server/org/opends/server/messages/AciMessages.java
@@ -1092,7 +1092,7 @@
              MSGID_ACI_SYNTAX_INVALID_TARGATTRFILTERS_FILTER_LISTS_FILTER,
              "The provided Access Control Instruction (ACI) " +
              "targattrfilters expression value " +
-             "%s is invalid because the one or more of the specified" +
+             "%s is invalid because one or more of the specified " +
              "filters are invalid for the following reason: " +
              "%s");
 
@@ -1100,7 +1100,7 @@
              MSGID_ACI_SYNTAX_INVALID_TARGATTRFILTERS_FILTER_LISTS_ATTR_FILTER,
              "The provided Access Control Instruction (ACI) " +
              "targattrfilters expression value " +
-             "%s is invalid because the one or more of the specified" +
+             "%s is invalid because one or more of the specified " +
              "filters are invalid because of non-matching attribute" +
              "type names in the filter");
 
diff --git a/opends/src/server/org/opends/server/messages/CoreMessages.java b/opends/src/server/org/opends/server/messages/CoreMessages.java
index fac6e6f..f91a29c 100644
--- a/opends/src/server/org/opends/server/messages/CoreMessages.java
+++ b/opends/src/server/org/opends/server/messages/CoreMessages.java
@@ -6031,6 +6031,16 @@
 
 
   /**
+   * The message ID for the message that will be used if a NOT filter does not
+   * contain exactly one filter component.  This takes three arguments, which
+   * are the filter string and the start and end position of the NOT filter.
+   */
+  public static final int MSGID_SEARCH_FILTER_NOT_EXACTLY_ONE =
+       CATEGORY_MASK_PROTOCOL | SEVERITY_MASK_MILD_ERROR | 602;
+
+
+
+  /**
    * Associates a set of generic messages with the message IDs defined
    * in this class.
    */
@@ -6539,6 +6549,10 @@
                     "because the extensible match component starting at " +
                     "position %d did not have a colon to denote the end of " +
                     "the attribute type name");
+    registerMessage(MSGID_SEARCH_FILTER_NOT_EXACTLY_ONE,
+                    "The provided search filter \"%s\" could not be decoded " +
+                    "because the NOT filter between positions %d and %d " +
+                    "did not contain exactly one filter component");
     registerMessage(MSGID_SEARCH_FILTER_INVALID_FILTER_TYPE,
                     "Unable to determine whether entry \"%s\" matches filter " +
                     "\"%s\" because it contained an unknown filter type %s");
diff --git a/opends/src/server/org/opends/server/messages/ProtocolMessages.java b/opends/src/server/org/opends/server/messages/ProtocolMessages.java
index 23889c4..9b76f6e 100644
--- a/opends/src/server/org/opends/server/messages/ProtocolMessages.java
+++ b/opends/src/server/org/opends/server/messages/ProtocolMessages.java
@@ -4387,6 +4387,18 @@
   public static final int MSGID_LDAPS_CONNHANDLER_DESCRIPTION_ENABLE =
        CATEGORY_MASK_PROTOCOL | SEVERITY_MASK_INFORMATIONAL | 404;
 
+
+
+  /**
+   * The message ID for the message that will be used if a NOT filter does not
+   * contain exactly one filter component.  This takes three arguments, which
+   * are the filter string and the start and end position of the NOT filter.
+   */
+  public static final int MSGID_LDAP_FILTER_NOT_EXACTLY_ONE =
+       CATEGORY_MASK_PROTOCOL | SEVERITY_MASK_MILD_ERROR | 405;
+
+
+
   /**
    * Associates a set of generic messages with the message IDs defined in this
    * class.
@@ -5168,6 +5180,10 @@
                     "because the extensible match component starting at " +
                     "position %d did not have a colon to denote the end of " +
                     "the attribute type name");
+    registerMessage(MSGID_LDAP_FILTER_NOT_EXACTLY_ONE,
+                    "The provided search filter \"%s\" could not be decoded " +
+                    "because the NOT filter between positions %d and %d " +
+                    "did not contain exactly one filter component");
 
 
     registerMessage(MSGID_LDAP_CLIENT_SEND_RESPONSE_NO_RESULT_CODE,
diff --git a/opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java b/opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java
index bee304b..fe8fd84 100644
--- a/opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java
+++ b/opends/src/server/org/opends/server/protocols/ldap/LDAPFilter.java
@@ -395,9 +395,8 @@
     }
     else if (c == '!')
     {
-      LDAPFilter notComponent = decode(filterString, startPos+1, endPos);
-      return new LDAPFilter(FilterType.NOT, null, notComponent, null, null,
-                              null, null, null, null, false);
+      return decodeCompoundFilter(FilterType.NOT, filterString, startPos+1,
+                                  endPos);
     }
 
 
@@ -657,7 +656,7 @@
    * indicated range.
    *
    * @param  filterType    The filter type for this compound filter.  It must be
-   *                       either an AND or an OR filter.
+   *                       an AND, OR or NOT filter.
    * @param  filterString  The string containing the filter information to
    *                       decode.
    * @param  startPos      The position of the first character in the set of
@@ -680,11 +679,20 @@
 
 
     // If the end pos is equal to the start pos, then there are no components.
-    // This is valid and will be treated as a TRUE/FALSE filter.
     if (startPos == endPos)
     {
-      return new LDAPFilter(filterType, filterComponents, null, null, null,
-                            null, null, null, null, false);
+      if (filterType == FilterType.NOT)
+      {
+        int    msgID   = MSGID_LDAP_FILTER_NOT_EXACTLY_ONE;
+        String message = getMessage(msgID, filterString, startPos, endPos);
+        throw new LDAPException(LDAPResultCode.PROTOCOL_ERROR, msgID, message);
+      }
+      else
+      {
+        // This is valid and will be treated as a TRUE/FALSE filter.
+        return new LDAPFilter(filterType, filterComponents, null, null, null,
+                              null, null, null, null, false);
+      }
     }
 
 
@@ -752,8 +760,23 @@
 
 
     // We should have everything we need, so return the list.
-    return new LDAPFilter(filterType, filterComponents, null, null, null, null,
-                          null, null, null, false);
+    if (filterType == FilterType.NOT)
+    {
+      if (filterComponents.size() != 1)
+      {
+        int    msgID   = MSGID_LDAP_FILTER_NOT_EXACTLY_ONE;
+        String message = getMessage(msgID, filterString, startPos, endPos);
+        throw new LDAPException(LDAPResultCode.PROTOCOL_ERROR, msgID, message);
+      }
+      RawFilter notComponent = filterComponents.get(0);
+      return new LDAPFilter(filterType, null, notComponent, null, null,
+                            null, null, null, null, false);
+    }
+    else
+    {
+      return new LDAPFilter(filterType, filterComponents, null, null, null,
+                            null, null, null, null, false);
+    }
   }
 
 
diff --git a/opends/src/server/org/opends/server/types/SearchFilter.java b/opends/src/server/org/opends/server/types/SearchFilter.java
index b7048ba..d3844a8 100644
--- a/opends/src/server/org/opends/server/types/SearchFilter.java
+++ b/opends/src/server/org/opends/server/types/SearchFilter.java
@@ -694,11 +694,8 @@
     }
     else if (c == '!')
     {
-      SearchFilter notComponent = createFilterFromString(filterString,
-                                       startPos+1, endPos);
-      return new SearchFilter(FilterType.NOT, null, notComponent,
-                              null, null, null, null, null, null,
-                              null, false);
+      return decodeCompoundFilter(FilterType.NOT, filterString,
+                                  startPos+1, endPos);
     }
 
 
@@ -1014,7 +1011,7 @@
    * the indicated range.
    *
    * @param  filterType    The filter type for this compound filter.
-   *                       It must be either an AND or an OR filter.
+   *                       It must be an AND, OR or NOT filter.
    * @param  filterString  The string containing the filter
    *                       information to decode.
    * @param  startPos      The position of the first character in the
@@ -1039,13 +1036,24 @@
 
 
     // If the end pos is equal to the start pos, then there are no
-    // components.  This is valid and will be treated as a TRUE/FALSE
-    // filter.
+    // components.
     if (startPos == endPos)
     {
-      return new SearchFilter(filterType, filterComponents, null,
-                              null, null, null, null, null, null,
-                              null, false);
+      if (filterType == FilterType.NOT)
+      {
+        int    msgID   = MSGID_SEARCH_FILTER_NOT_EXACTLY_ONE;
+        String message = getMessage(msgID, filterString, startPos,
+                                    endPos);
+        throw new DirectoryException(ResultCode.PROTOCOL_ERROR,
+                                     message, msgID);
+      }
+      else
+      {
+        // This is valid and will be treated as a TRUE/FALSE filter.
+        return new SearchFilter(filterType, filterComponents, null,
+                                null, null, null, null, null, null,
+                                null, false);
+      }
     }
 
 
@@ -1123,13 +1131,30 @@
 
 
     // We should have everything we need, so return the list.
-    return new SearchFilter(filterType, filterComponents, null, null,
-                            null, null, null, null, null, null,
-                            false);
+    if (filterType == FilterType.NOT)
+    {
+      if (filterComponents.size() != 1)
+      {
+        int    msgID   = MSGID_SEARCH_FILTER_NOT_EXACTLY_ONE;
+        String message = getMessage(msgID, filterString, startPos,
+                                    endPos);
+        throw new DirectoryException(ResultCode.PROTOCOL_ERROR,
+                                     message, msgID);
+      }
+      SearchFilter notComponent = filterComponents.get(0);
+      return new SearchFilter(filterType, null, notComponent, null,
+                              null, null, null, null, null, null,
+                              false);
+    }
+    else
+    {
+      return new SearchFilter(filterType, filterComponents, null,
+                              null, null, null, null, null, null,
+                              null, false);
+    }
   }
 
 
-
   /**
    * Decodes a substring search filter component based on the provided
    * information.
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java
index 427a9bb..c7be60c 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/protocols/ldap/TestLDAPFilter.java
@@ -67,7 +67,9 @@
       { "(&(givenname=bob)|(sn=pep)dob=12))", null },
       { "(:=bob)", null },
       { "(=sally)", null },
-      { "(cn=billy bob", null }
+      { "(cn=billy bob", null },
+      { "(|(!(title=sweep*)(l=Paris*)))", null },
+      { "(|(!))", null },
 
     };
   }
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/types/SearchFilterTests.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/types/SearchFilterTests.java
index 69883ec..4ef6643 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/types/SearchFilterTests.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/types/SearchFilterTests.java
@@ -281,11 +281,12 @@
             {"(!(sn=test)"},
             {"(&(sn=test)))"},
             {"(|(sn=test)))"},
-            // TODO: open a bug for this.
-//            {"(!(sn=test)))"},
+            {"(!(sn=test)))"},
             {"(sn=\\A)"},
             {"(sn=\\1H)"},
             {"(sn=\\H1)"},
+            {"(!(sn=test)(cn=test))"},
+            {"(!)"},
     };
   }
 

--
Gitblit v1.10.0