From b7084e32bb89655ead73bd0d50616fa1f5bb8491 Mon Sep 17 00:00:00 2001
From: sin <sin@localhost>
Date: Tue, 14 Apr 2009 15:56:32 +0000
Subject: [PATCH] issue#2623:ldapsearch: wrong error message when a wrong sort order is specified

---
 opends/src/server/org/opends/server/backends/jeb/EntryContainer.java                                    |   71 +++++++++++++++++------
 opends/src/server/org/opends/server/controls/ServerSideSortRequestControl.java                          |   33 +++++++++--
 opends/tests/unit-tests-testng/src/server/org/opends/server/tools/LDAPSearchTestCase.java               |    4 
 opends/tests/unit-tests-testng/src/server/org/opends/server/controls/ServerSideSortControlTestCase.java |   56 +++++++++++++++++-
 4 files changed, 134 insertions(+), 30 deletions(-)

diff --git a/opends/src/server/org/opends/server/backends/jeb/EntryContainer.java b/opends/src/server/org/opends/server/backends/jeb/EntryContainer.java
index 71e3171..834889b 100644
--- a/opends/src/server/org/opends/server/backends/jeb/EntryContainer.java
+++ b/opends/src/server/org/opends/server/backends/jeb/EntryContainer.java
@@ -875,6 +875,22 @@
     .getRequestControl(PagedResultsControl.DECODER);
     ServerSideSortRequestControl sortRequest = searchOperation
     .getRequestControl(ServerSideSortRequestControl.DECODER);
+    if(sortRequest != null && !sortRequest.containsSortKeys()
+            && sortRequest.isCritical())
+    {
+      /**
+         If the control's criticality field is true then the server SHOULD do
+         the following: return unavailableCriticalExtension as a return code
+         in the searchResultDone message; include the sortKeyResponseControl in
+         the searchResultDone message, and not send back any search result
+         entries.
+       */
+      searchOperation.addResponseControl(
+            new ServerSideSortResponseControl(
+                LDAPResultCode.NO_SUCH_ATTRIBUTE, null));
+      searchOperation.setResultCode(ResultCode.UNAVAILABLE_CRITICAL_EXTENSION);
+      return;
+    }
     VLVRequestControl vlvRequest = searchOperation
     .getRequestControl(VLVRequestControl.DECODER);
 
@@ -1043,28 +1059,47 @@
 
       if (sortRequest != null)
       {
-        try
-        {
-          entryIDList = EntryIDSetSorter.sort(this, entryIDList,
-              searchOperation,
-              sortRequest.getSortOrder(),
-              vlvRequest);
-          searchOperation.addResponseControl(
-              new ServerSideSortResponseControl(LDAPResultCode.SUCCESS, null));
-        }
-        catch (DirectoryException de)
-        {
-          searchOperation.addResponseControl(
-              new ServerSideSortResponseControl(
-                  de.getResultCode().getIntValue(), null));
-
-          if (sortRequest.isCritical())
+          try
           {
-            throw de;
+            //If the sort key is not present, the sorting will generate the
+            //default ordering. VLV search request goes through as if
+            //this sort key was not found in the user entry.
+            entryIDList = EntryIDSetSorter.sort(this, entryIDList,
+                searchOperation,
+                sortRequest.getSortOrder(),
+                vlvRequest);
+            if(sortRequest.containsSortKeys())
+            {
+              searchOperation.addResponseControl(
+                new ServerSideSortResponseControl(
+                                              LDAPResultCode.SUCCESS, null));
+            }
+            else
+            {
+              /*
+                There is no sort key associated with the sort control. Since it
+                came here it means that the critificality is false so let the
+                server return all search results unsorted and include the
+                sortKeyResponseControl inthe searchResultDone message.
+              */
+              searchOperation.addResponseControl(
+                      new ServerSideSortResponseControl
+                                (LDAPResultCode.NO_SUCH_ATTRIBUTE, null));
+            }
+          }
+          catch (DirectoryException de)
+          {
+            searchOperation.addResponseControl(
+                new ServerSideSortResponseControl(
+                    de.getResultCode().getIntValue(), null));
+
+            if (sortRequest.isCritical())
+            {
+              throw de;
+            }
           }
         }
       }
-    }
 
     // If requested, construct and return a fictitious entry containing
     // debug information, and no other entries.
diff --git a/opends/src/server/org/opends/server/controls/ServerSideSortRequestControl.java b/opends/src/server/org/opends/server/controls/ServerSideSortRequestControl.java
index 2c71b03..a9c1361 100644
--- a/opends/src/server/org/opends/server/controls/ServerSideSortRequestControl.java
+++ b/opends/src/server/org/opends/server/controls/ServerSideSortRequestControl.java
@@ -22,7 +22,7 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2008 Sun Microsystems, Inc.
+ *      Copyright 2008-2009 Sun Microsystems, Inc.
  */
 package org.opends.server.controls;
 import org.opends.messages.Message;
@@ -115,8 +115,11 @@
               DirectoryServer.getAttributeType(attrName, false);
           if (attrType == null)
           {
-            Message message = INFO_SORTREQ_CONTROL_UNDEFINED_ATTR.get(attrName);
-            throw new DirectoryException(ResultCode.PROTOCOL_ERROR, message);
+            //This attribute is not defined in the schema. There is no point
+            //iterating over the next attribute and return a partially sorted
+            //result.
+            return new ServerSideSortRequestControl(isCritical,
+            new SortOrder(sortKeys.toArray(new SortKey[0])));
           }
 
           OrderingMatchingRule orderingRule = null;
@@ -351,6 +354,23 @@
   }
 
   /**
+   * Indicates whether the sort control contains Sort keys.
+   *
+   * <P> A Sort control may not contain sort keys if the attribute type
+   * is not recognized by the server </P>
+   *
+   * @return  <CODE>true</CODE> if the control contains sort keys
+   *          or <CODE>false</CODE> if it does not.
+   *
+   * @throws  DirectoryException  If a problem occurs while trying to make the
+   *                              determination.
+   */
+  public boolean  containsSortKeys() throws DirectoryException
+  {
+    return getSortOrder().getSortKeys().length!=0;
+  }
+
+  /**
    * Writes this control's value to an ASN.1 writer. The value (if any) must
    * be written as an ASN1OctetString.
    *
@@ -438,9 +458,10 @@
           DirectoryServer.getAttributeType(decodedKey[0].toLowerCase(), false);
       if (attrType == null)
       {
-        Message message =
-            INFO_SORTREQ_CONTROL_UNDEFINED_ATTR.get(decodedKey[0]);
-        throw new DirectoryException(ResultCode.PROTOCOL_ERROR, message);
+        //This attribute is not defined in the schema. There is no point
+        //iterating over the next attribute and return a partially sorted
+        //result.
+        return new SortOrder(sortKeys.toArray(new SortKey[0]));
       }
 
       OrderingMatchingRule orderingRule = null;
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/controls/ServerSideSortControlTestCase.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/controls/ServerSideSortControlTestCase.java
index 46fe9e0..6c9ff77 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/controls/ServerSideSortControlTestCase.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/controls/ServerSideSortControlTestCase.java
@@ -22,7 +22,7 @@
  * CDDL HEADER END
  *
  *
- *      Copyright 2008 Sun Microsystems, Inc.
+ *      Copyright 2008-2009 Sun Microsystems, Inc.
  */
 package org.opends.server.controls;
 
@@ -718,13 +718,13 @@
 
 
   /**
-   * Tests performing an internal search using the server-side sort control with
+   * Tests performing an internal search using the CRITICAL server-side sort control with
    * an undefined attribute type.
    *
    * @throws  Exception  If an unexpected problem occurred.
    */
   @Test()
-  public void testInternalSearchUndefinedAttribute()
+  public void testCriticalSortWithUndefinedAttribute()
          throws Exception
   {
     populateDB();
@@ -744,7 +744,7 @@
                   null, null);
 
     internalSearch.run();
-    assertFalse(internalSearch.getResultCode() == ResultCode.SUCCESS);
+    assertEquals(internalSearch.getResultCode(), ResultCode.UNAVAILABLE_CRITICAL_EXTENSION);
   }
 
 
@@ -779,5 +779,53 @@
     internalSearch.run();
     assertFalse(internalSearch.getResultCode() == ResultCode.SUCCESS);
   }
+  
+  
+  /**
+   * Tests performing an internal search using the non-critical server-side
+   * sort control to sort the entries
+   *
+   * @throws  Exception  If an unexpected problem occurred.
+   */
+  @Test()
+  public void testNonCriticalSortWithUndefinedAttribute()
+         throws Exception
+  {
+    populateDB();
+    InternalClientConnection conn =
+    InternalClientConnection.getRootConnection();
+
+    ArrayList<Control> requestControls = new ArrayList<Control>();
+    requestControls.add(new ServerSideSortRequestControl(false,
+                                 "bad_sort:caseExactOrderingMatch"));
+
+    InternalSearchOperation internalSearch =
+         new InternalSearchOperation(conn, conn.nextOperationID(),
+                  conn.nextMessageID(), requestControls,
+                  DN.decode("dc=example,dc=com"), SearchScope.WHOLE_SUBTREE,
+                  DereferencePolicy.NEVER_DEREF_ALIASES, 0, 0, false,
+                  SearchFilter.createFilterFromString("(objectClass=person)"),
+                  null, null);
+
+    internalSearch.run();
+    assertEquals(internalSearch.getResultCode(),
+            ResultCode.SUCCESS);
+    List<Control> responseControls = internalSearch.getResponseControls();
+    assertNotNull(responseControls);
+    assertEquals(responseControls.size(), 1);
+
+    ServerSideSortResponseControl responseControl;
+    Control c = responseControls.get(0);
+    if(c instanceof ServerSideSortResponseControl)
+    {
+      responseControl = (ServerSideSortResponseControl)c;
+    }
+    else
+    {
+      responseControl = ServerSideSortResponseControl.DECODER.decode(
+              c.isCritical(), ((LDAPControl)c).getValue());
+    }
+    assertEquals(responseControl.getResultCode(), 16);
+  }
 }
 
diff --git a/opends/tests/unit-tests-testng/src/server/org/opends/server/tools/LDAPSearchTestCase.java b/opends/tests/unit-tests-testng/src/server/org/opends/server/tools/LDAPSearchTestCase.java
index 09f93a0..6de452a 100644
--- a/opends/tests/unit-tests-testng/src/server/org/opends/server/tools/LDAPSearchTestCase.java
+++ b/opends/tests/unit-tests-testng/src/server/org/opends/server/tools/LDAPSearchTestCase.java
@@ -2311,7 +2311,7 @@
          throws Exception
   {
     TestCaseUtils.clearJEBackend(true, "userRoot", "dc=example,dc=com");
-
+   
     String[] args =
     {
       "-h", "127.0.0.1",
@@ -2325,7 +2325,7 @@
       "(objectClass=*)"
     };
 
-    assertFalse(LDAPSearch.mainSearch(args, false, null, null) == 0);
+    assertTrue(LDAPSearch.mainSearch(args, false, null, null) == 0);
   }
 
 

--
Gitblit v1.10.0