Browse Source

Ticket 48752: Page result search should return empty cookie if there is no returned entry

Bug Description:
  When there is no more entry to return the cookie should be empty
  (see https://www.ietf.org/rfc/rfc2696.txt).
  This is done when current_search_count=-1 but in case current_search_count=0
  the cookie is returned. This let the ldap client think it has to continue
  to send PR searches

Fix Description:
  This fix is an hardening of the case there is no more entry to return.
  Plus for the troubleshooting, the cookie value is additionally logged in
  the access log:
  [..] conn=# op=# RESULT err=0 tag=101 nentries=5 etime=0 notes=P pr_idx=2 pr_cookie=2

https://fedorahosted.org/389/ticket/48752

Reviewed by: [email protected]

Platforms tested: F23

Flag Day: no

Doc impact: no
Thierry Bordaz 9 years ago
parent
commit
f215fb63a4

+ 6 - 2
ldap/servers/slapd/opshared.c

@@ -477,10 +477,12 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
       if ( slapi_control_present (ctrlp, LDAP_CONTROL_PAGEDRESULTS,
                                   &ctl_value, &iscritical) )
       {
+          int pr_cookie = -1;
           /* be is set only when this request is new. otherwise, prev be is honored. */
           rc = pagedresults_parse_control_value(pb, ctl_value, &pagesize, &pr_idx, be);
           /* Let's set pr_idx even if it fails; in case, pr_idx == -1. */
           slapi_pblock_set(pb, SLAPI_PAGED_RESULTS_INDEX, &pr_idx);
+          slapi_pblock_set(pb, SLAPI_PAGED_RESULTS_COOKIE, &pr_cookie);
           if ((LDAP_SUCCESS == rc) || (LDAP_CANCELLED == rc) || (0 == pagesize)) {
               unsigned int opnote = SLAPI_OP_NOTE_SIMPLEPAGED;
               op_set_pagedresults(operation);
@@ -700,7 +702,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
       }
       pagedresults_unlock(pb->pb_conn, pr_idx);
 
-      if (PAGEDRESULTS_SEARCH_END == pr_stat) {
+      if ((PAGEDRESULTS_SEARCH_END == pr_stat) || (0 == pnentries)) {
         /* no more entries to send in the backend */
         if (NULL == next_be) {
           /* no more entries && no more backends */
@@ -709,6 +711,7 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
           curr_search_count = pnentries;
         }
         estimate = 0;
+        pr_stat = PAGEDRESULTS_SEARCH_END; /* make sure stat is SEARCH_END */
       } else {
         curr_search_count = pnentries;
         estimate -= estimate?curr_search_count:0;
@@ -870,13 +873,14 @@ op_shared_search (Slapi_PBlock *pb, int send_result)
   
             curr_search_count = pnentries;
             slapi_pblock_get(pb, SLAPI_SEARCH_RESULT_SET, &sr);
-            if (PAGEDRESULTS_SEARCH_END == pr_stat) {
+            if ((PAGEDRESULTS_SEARCH_END == pr_stat) || (0 == pnentries)) {
               /* no more entries, but at least another backend */
               PR_EnterMonitor(pb->pb_conn->c_mutex);
               pagedresults_set_search_result(pb->pb_conn, operation, NULL, 1, pr_idx);
               be->be_search_results_release(&sr);
               rc = pagedresults_set_current_be(pb->pb_conn, next_be, pr_idx, 1);
               PR_ExitMonitor(pb->pb_conn->c_mutex);
+              pr_stat = PAGEDRESULTS_SEARCH_END; /* make sure stat is SEARCH_END */
               if (NULL == next_be) {
                   /* no more entries && no more backends */
                   curr_search_count = -1;

+ 4 - 0
ldap/servers/slapd/pagedresults.c

@@ -235,6 +235,7 @@ pagedresults_set_response_control( Slapi_PBlock *pb, int iscritical,
     char *cookie_str = NULL;
     int found = 0;
     int i;
+    int cookie = 0;
 
     LDAPDebug1Arg(LDAP_DEBUG_TRACE,
                   "--> pagedresults_set_response_control: idx=%d\n", index);
@@ -246,10 +247,13 @@ pagedresults_set_response_control( Slapi_PBlock *pb, int iscritical,
 
     /* begin sequence, payload, end sequence */
     if (current_search_count < 0) {
+        cookie = 0;
         cookie_str = slapi_ch_strdup("");
     } else {
+        cookie = index;
         cookie_str = slapi_ch_smprintf("%d", index);
     }
+    slapi_pblock_set ( pb, SLAPI_PAGED_RESULTS_COOKIE, &cookie );
     ber_printf ( ber, "{io}", estimate, cookie_str, strlen(cookie_str) );
     if ( ber_flatten ( ber, &berval ) != LDAP_SUCCESS )
     {

+ 13 - 0
ldap/servers/slapd/pblock.c

@@ -1946,6 +1946,15 @@ slapi_pblock_get( Slapi_PBlock *pblock, int arg, void *value )
 		}
 		break;
 
+	case SLAPI_PAGED_RESULTS_COOKIE:
+		if (op_is_pagedresults(pblock->pb_op)) {
+			/* search req is simple paged results */
+			(*(int *)value) = pblock->pb_paged_results_cookie;
+		} else {
+			(*(int *)value) = 0;
+		}
+		break;
+
 	/* ACI Target Check */	
 	case SLAPI_ACI_TARGET_CHECK:
 		(*(int *)value) = pblock->pb_aci_target_check;
@@ -3540,6 +3549,10 @@ slapi_pblock_set( Slapi_PBlock *pblock, int arg, void *value )
 		pblock->pb_paged_results_index = *(int *)value;
 		break;
 
+	case SLAPI_PAGED_RESULTS_COOKIE:
+		pblock->pb_paged_results_cookie = *(int *)value;
+		break;
+
 	/* ACI Target Check */
 	case SLAPI_ACI_TARGET_CHECK:
 		pblock->pb_aci_target_check = *((int *) value);

+ 7 - 4
ldap/servers/slapd/result.c

@@ -26,6 +26,7 @@
 #include "pratom.h"
 #include "fe.h"
 #include "vattr_spi.h"
+#include "slapi-plugin.h"
 
 #include <ssl.h>
 
@@ -1938,8 +1939,10 @@ log_result( Slapi_PBlock *pb, Operation *op, int err, ber_tag_t tag, int nentrie
 	char csn_str[CSN_STRSIZE + 5];
 	char etime[ETIME_BUFSIZ];
 	int pr_idx = -1;
+	int pr_cookie = -1;
 
 	slapi_pblock_get(pb, SLAPI_PAGED_RESULTS_INDEX, &pr_idx);
+	slapi_pblock_get(pb, SLAPI_PAGED_RESULTS_COOKIE, &pr_cookie);
 
 	internal_op = operation_is_flag_set( op, OP_FLAG_INTERNAL );
 
@@ -2040,24 +2043,24 @@ log_result( Slapi_PBlock *pb, Operation *op, int err, ber_tag_t tag, int nentrie
 				slapi_log_access( LDAP_DEBUG_STATS,
 								  "conn=%" NSPRIu64 " op=%d RESULT err=%d"
 								  " tag=%" BERTAG_T " nentries=%d etime=%s%s%s"
-								  " pr_idx=%d\n",
+								  " pr_idx=%d pr_cookie=%d\n",
 								  op->o_connid, 
 								  op->o_opid,
 								  err, tag, nentries, 
 								  etime, 
-								  notes_str, csn_str, pr_idx );
+								  notes_str, csn_str, pr_idx, pr_cookie );
 			}
 			else
 			{
 				slapi_log_access( LDAP_DEBUG_ARGS,
 								  "conn=%s op=%d RESULT err=%d"
 								  " tag=%" BERTAG_T " nentries=%d etime=%s%s%s"
-								  " pr_idx=%d\n",
+								  " pr_idx=%d pr_cookie=%d \n",
 								  LOG_INTERNAL_OP_CON_ID,
 								  LOG_INTERNAL_OP_OP_ID,
 								  err, tag, nentries, 
 								  etime, 
-								  notes_str, csn_str, pr_idx );
+								  notes_str, csn_str, pr_idx, pr_cookie );
 			}
 		}
 		else if ( !internal_op )

+ 1 - 0
ldap/servers/slapd/slap.h

@@ -1735,6 +1735,7 @@ typedef struct slapi_pblock {
 	int		pb_syntax_filter_normalized; /* the syntax filter types/values are already normalized */
 	void		*pb_syntax_filter_data; /* extra data to pass to a syntax plugin function */
 	int	pb_paged_results_index;    /* stash SLAPI_PAGED_RESULTS_INDEX */
+        int	pb_paged_results_cookie;   /* stash SLAPI_PAGED_RESULTS_COOKIE */
 	passwdPolicy *pwdpolicy;
 	void *op_stack_elem;
 

+ 1 - 0
ldap/servers/slapd/slapi-plugin.h

@@ -7314,6 +7314,7 @@ typedef struct slapi_plugindesc {
 
 /* Simple paged results index */
 #define SLAPI_PAGED_RESULTS_INDEX   1945
+#define SLAPI_PAGED_RESULTS_COOKIE  1949
 
 /* ACI Target Check */
 #define SLAPI_ACI_TARGET_CHECK      1946