ldap/servers/slapd/opshared.c | 8 ++++++-- ldap/servers/slapd/pagedresults.c | 4 ++++ ldap/servers/slapd/pblock.c | 13 +++++++++++++ ldap/servers/slapd/result.c | 11 +++++++---- ldap/servers/slapd/slap.h | 1 + ldap/servers/slapd/slapi-plugin.h | 1 + 6 files changed, 32 insertions(+), 6 deletions(-)
New commits: commit ca50d1ca577099e3f14778a20a2fc5ca27bc365d Author: Thierry Bordaz tbordaz@redhat.com Date: Fri May 27 17:25:53 2016 -0700
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: nhosoi@redhat.com
Platforms tested: F23
Flag Day: no
Doc impact: no
(cherry picked from commit f215fb63a4ba2b57c44b29d6a88d655fb98917d1)
diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c index e76ca0f..84e4c71 100644 --- a/ldap/servers/slapd/opshared.c +++ b/ldap/servers/slapd/opshared.c @@ -476,10 +476,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); @@ -699,7 +701,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 */ @@ -708,6 +710,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; @@ -869,13 +872,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; diff --git a/ldap/servers/slapd/pagedresults.c b/ldap/servers/slapd/pagedresults.c index 52d2158..07a7b69 100644 --- a/ldap/servers/slapd/pagedresults.c +++ b/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 ) { diff --git a/ldap/servers/slapd/pblock.c b/ldap/servers/slapd/pblock.c index d373d99..f268f72 100644 --- a/ldap/servers/slapd/pblock.c +++ b/ldap/servers/slapd/pblock.c @@ -1936,6 +1936,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; @@ -3520,6 +3529,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); diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c index cd58f50..34c4d41 100644 --- a/ldap/servers/slapd/result.c +++ b/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>
@@ -1931,8 +1932,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 );
@@ -2033,24 +2036,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 ) diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index 657f16d..6615868 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -1724,6 +1724,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;
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 72f3920..a193aad 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -7287,6 +7287,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
389-commits@lists.fedoraproject.org