From f215fb63a4ba2b57c44b29d6a88d655fb98917d1 Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: May 28 2016 00:34:02 +0000 Subject: 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 --- diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c index 29a1d84..7eaa19a 100644 --- a/ldap/servers/slapd/opshared.c +++ b/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; 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 d48c2d0..f6d3af0 100644 --- a/ldap/servers/slapd/pblock.c +++ b/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); diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c index b5681ec..cb465bf 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 @@ -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 ) diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index 20a2a3a..a594a29 100644 --- a/ldap/servers/slapd/slap.h +++ b/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; diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index d13aae9..294f2c3 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/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