- consistently honor multiple referrals

- return those that failed as referrals
- fix handling of some operational attributes
- fix URI rebuilding (use default scope)
- make copies of o_req_dn/o_req_ndn in case other overlays muck with them

servers/slapd/overlays/chain.c

@@ -41,6 +41,54 @@ ldap_chain_chk_referrals( Operation *op, SlapReply *rs )
 	return LDAP_SUCCESS;
 }
 
+static int
+ldap_chain_operational( Operation *op, SlapReply *rs )
+{
+	/* trap entries generated by back-ldap.
+	 * FIXME: we need a better way to recognize them; a cleaner
+	 * solution would be to be able to intercept the response
+	 * of be_operational(), so that we can divert only those
+	 * calls that fail because operational attributes were
+	 * requested for entries that do not belong to the underlying
+	 * database.  This fix is likely to intercept also entries
+	 * generated by back-perl and so. */
+	if ( rs->sr_entry->e_private == NULL ) {
+		return 0;
+	}
+
+	return SLAP_CB_CONTINUE;
+}
+
+static int
+ldap_chain_cb_response( Operation *op, SlapReply *rs )
+{
+	assert( op->o_tag == LDAP_REQ_SEARCH );
+
+	if ( rs->sr_type == REP_SEARCH ) {
+		Attribute	**ap = &rs->sr_entry->e_attrs;
+
+		for ( ; *ap != NULL; ap = &(*ap)->a_next ) {
+			/* will be generated later by frontend
+			 * (a cleaner solution would be that
+			 * the frontend checks if it already exists */
+			if ( ad_cmp( (*ap)->a_desc, slap_schema.si_ad_entryDN ) == 0 )
+			{
+				Attribute *a = *ap;
+
+				*ap = (*ap)->a_next;
+				attr_free( a );
+
+				/* there SHOULD be one only! */
+				break;
+			}
+		}
+		
+		return SLAP_CB_CONTINUE;
+	}
+
+	return 0;
+}
+
 static int
 ldap_chain_response( Operation *op, SlapReply *rs )
 {
@@ -66,6 +114,9 @@ ldap_chain_response( Operation *op, SlapReply *rs )
 	op->o_callback = NULL;
 
 	if ( lip->url == NULL ) {
+		/* FIXME: we're setting the URI of the first referral;
+		 * what if there are more?  Is this something we should
+		 * worry about? */
 		li = *lip;
 		op->o_bd->be_private = &li;
 
@@ -73,9 +124,10 @@ ldap_chain_response( Operation *op, SlapReply *rs )
 			LDAPURLDesc	*srv;
 			char		*save_dn;
 
-			/* parse reference and use proto://[host][:port]/ only */
+			/* parse reference and use 
+			 * proto://[host][:port]/ only */
 			rc = ldap_url_parse_ext( ref[0].bv_val, &srv );
-			if ( rc != LDAP_SUCCESS) {
+			if ( rc != LDAP_URL_SUCCESS) {
 				/* error */
 				return 1;
 			}
@@ -85,16 +137,15 @@ ldap_chain_response( Operation *op, SlapReply *rs )
 			 * as a comma-separated URL list */
 			save_dn = srv->lud_dn;
 			srv->lud_dn = "";
+			srv->lud_scope = LDAP_SCOPE_DEFAULT;
 			li.url = ldap_url_desc2str( srv );
-			if ( li.url == NULL ) {
-				/* error */
-				srv->lud_dn = save_dn;
-				ldap_free_urldesc( srv );
-				return 1;
-			}
-
 			srv->lud_dn = save_dn;
 			ldap_free_urldesc( srv );
+
+			if ( li.url == NULL ) {
+				/* error */
+				return 1;
+			}
 		}
 
 	} else {
@@ -106,34 +157,38 @@ ldap_chain_response( Operation *op, SlapReply *rs )
 	 * Binds are done separately, on an anonymous session.
 	 */
 	if ( op->o_tag != LDAP_REQ_BIND ) {
-		for (i=0; prev && prev[i]; i++);
+		for ( i = 0; prev && prev[i]; i++ )
+			/* count and set prev to the last one */ ;
 		nctrls = i;
 
 		/* Add an extra NULL slot */
-		if (!prev) i++;
+		if ( !prev ) {
+			i++;
+		}
 
-		ctrls = op->o_tmpalloc((i+1)*sizeof(LDAPControl *),
+		ctrls = op->o_tmpalloc((i + 1)*sizeof(LDAPControl *),
 			op->o_tmpmemctx);
-		for (i=0; i <nctrls; i++)
+		for ( i = 0; i < nctrls; i++ ) {
 			ctrls[i] = prev[i];
+		}
 		ctrls[nctrls] = &authz;
-		ctrls[nctrls+1] = NULL;
+		ctrls[nctrls + 1] = NULL;
 		authz.ldctl_oid = LDAP_CONTROL_PROXY_AUTHZ;
 		authz.ldctl_iscritical = 1;
 		authz.ldctl_value = op->o_dn;
-		if ( op->o_dn.bv_len ) {
+		if ( !BER_BVISEMPTY( &op->o_dn ) ) {
-			authzid = op->o_tmpalloc( op->o_dn.bv_len + sizeof("dn:") - 1,
+			authzid = op->o_tmpalloc( op->o_dn.bv_len + STRLENOF("dn:"),
 				op->o_tmpmemctx );
 			strcpy(authzid, "dn:");
-			strcpy(authzid + sizeof("dn:") - 1, op->o_dn.bv_val);
+			strcpy(authzid + STRLENOF("dn:"), op->o_dn.bv_val);
-			authz.ldctl_value.bv_len = op->o_dn.bv_len + sizeof("dn:") - 1;
+			authz.ldctl_value.bv_len = op->o_dn.bv_len + STRLENOF("dn:");
 			authz.ldctl_value.bv_val = authzid;
 		}
 		op->o_ctrls = ctrls;
 		op->o_ndn = op->o_bd->be_rootndn;
 	}
 
-	switch( op->o_tag ) {
+	switch ( op->o_tag ) {
 	case LDAP_REQ_BIND: {
 		struct berval	rndn = op->o_req_ndn;
 		Connection	*conn = op->o_conn;
@@ -166,59 +221,107 @@ ldap_chain_response( Operation *op, SlapReply *rs )
 			struct berval	*curr = ref,
 					odn = op->o_req_dn,
 					ondn = op->o_req_ndn;
+			slap_callback	sc2 = { 0 };
+			int		tmprc = 0;
+			ber_len_t	refcnt = 0;
+			BerVarray	newref = NULL;
+
+			sc2.sc_response = ldap_chain_cb_response;
+			op->o_callback = &sc2;
 
 			rs->sr_type = REP_SEARCH;
 
 			/* copy the private info because we need to modify it */
-			for ( ; curr[0].bv_val; curr++ ) {
+			for ( ; !BER_BVISNULL( &curr[0] ); curr++ ) {
 				LDAPURLDesc	*srv;
+				char		*save_dn;
 
-				/* parse reference and use proto://[host][:port]/ only */
+				/* parse reference and use
-				rc = ldap_url_parse_ext( curr[0].bv_val, &srv );
+				 * proto://[host][:port]/ only */
-				if ( rc != LDAP_SUCCESS) {
+				tmprc = ldap_url_parse_ext( curr[0].bv_val, &srv );
+				if ( tmprc != LDAP_URL_SUCCESS ) {
 					/* error */
 					rc = 1;
 					goto end_of_searchref;
 				}
 
-				ber_str2bv(srv->lud_dn, 0, 0, &op->o_req_dn);
-				op->o_req_ndn = op->o_req_dn;
-
 				/* remove DN essentially because later on 
 				 * ldap_initialize() will parse the URL 
 				 * as a comma-separated URL list */
+				save_dn = srv->lud_dn;
 				srv->lud_dn = "";
+				srv->lud_scope = LDAP_SCOPE_DEFAULT;
 				li.url = ldap_url_desc2str( srv );
+				if ( li.url != NULL ) {
+					ber_str2bv_x( save_dn, 0, 1, &op->o_req_dn,
+							op->o_tmpmemctx );
+					ber_dupbv_x( &op->o_req_ndn, &op->o_req_dn,
+							op->o_tmpmemctx );
+				}
+
+				srv->lud_dn = save_dn;
+				ldap_free_urldesc( srv );
+
 				if ( li.url == NULL ) {
 					/* error */
-					srv->lud_dn = op->o_req_dn.bv_val;
-					ldap_free_urldesc( srv );
 					rc = 1;
 					goto end_of_searchref;
 				}
 
+
 				/* FIXME: should we also copy filter and scope?
 				 * according to RFC3296, no */
-
+				tmprc = lback->bi_op_search( op, rs );
-				rc = lback->bi_op_search( op, rs );
 
 				ldap_memfree( li.url );
 				li.url = NULL;
 
-				srv->lud_dn = op->o_req_dn.bv_val;
+				op->o_tmpfree( op->o_req_dn.bv_val,
-				ldap_free_urldesc( srv );
+						op->o_tmpmemctx );
+				op->o_tmpfree( op->o_req_ndn.bv_val,
+						op->o_tmpmemctx );
 
-				if ( rc ) {
+				if ( tmprc ) {
 					/* error */
 					rc = 1;
 					goto end_of_searchref;
 				}
+
+				if ( rs->sr_err != LDAP_SUCCESS ) {
+					/* if search was not successful,
+					 * at least return the referral! */
+					/* FIXME: assumes referrals 
+					 * are always created via
+					 * referral_rewrite() and freed via
+					 * ber_bvarray_free( rs->sr_ref ) */
+					newref = ch_realloc( newref, sizeof( struct berval ) * (refcnt + 2) );
+					ber_dupbv( &newref[ refcnt ], &curr[ 0 ] );
+					refcnt++;
+					BER_BVZERO( &newref[ refcnt ] );
+				}
 			}
 
 end_of_searchref:;
 			op->o_req_dn = odn;
 			op->o_req_ndn = ondn;
 			rs->sr_type = REP_SEARCHREF;
+			rs->sr_entry = NULL;
+
+			/* if the error was bad, it was already returned
+			 * by back-ldap; destroy the referrals left;
+			 * otherwise, let the frontend return them. */
+			if ( newref ) {
+				if ( rc == 0 ) {
+					rc = SLAP_CB_CONTINUE;
+					if ( ref != default_referral ) {
+						ber_bvarray_free( ref );
+					}
+					ref = newref;
+
+				} else {
+					ber_bvarray_free( newref );
+				}
+			}
 			
 		} else {
 			rc = lback->bi_op_search( op, rs );
@@ -236,22 +339,27 @@ end_of_searchref:;
 	op->o_bd->be_private = private;
 	op->o_callback = sc;
 	op->o_ndn = ndn;
-	if ( ctrls ) op->o_tmpfree( ctrls, op->o_tmpmemctx );
+	if ( ctrls ) {
-	if ( authzid ) op->o_tmpfree( authzid, op->o_tmpmemctx );
+		op->o_tmpfree( ctrls, op->o_tmpmemctx );
+	}
+	if ( authzid ) {
+		op->o_tmpfree( authzid, op->o_tmpmemctx );
+	}
 	rs->sr_ref = ref;
-	if ( lip->url == NULL && li.url ) {
+	if ( lip->url == NULL && li.url != NULL ) {
 		ldap_memfree( li.url );
 	}
 
 	return rc;
 }
 
-static int ldap_chain_config(
+static int
-    BackendDB	*be,
+ldap_chain_config(
-    const char	*fname,
+	BackendDB	*be,
-    int		lineno,
+	const char	*fname,
-    int		argc,
+	int		lineno,
-    char	**argv
+	int		argc,
+	char	**argv
 )
 {
 	slap_overinst	*on = (slap_overinst *) be->bd_info;
@@ -273,7 +381,8 @@ static int ldap_chain_config(
 	return rc;
 }
 
-static int ldap_chain_init(
+static int
+ldap_chain_init(
 	BackendDB *be
 )
 {
@@ -289,7 +398,8 @@ static int ldap_chain_init(
 	return rc;
 }
 
-static int ldap_chain_destroy(
+static int
+ldap_chain_destroy(
 	BackendDB *be
 )
 {
@@ -306,18 +416,34 @@ static int ldap_chain_destroy(
 
 static slap_overinst ldapchain;
 
-int chain_init()
+int
+chain_init()
 {
-	lback = backend_info("ldap");
+	lback = backend_info( "ldap" );
 
-	if ( !lback ) return -1;
+	if ( !lback ) {
+		return -1;
+	}
 
 	ldapchain.on_bi.bi_type = "chain";
 	ldapchain.on_bi.bi_db_init = ldap_chain_init;
 	ldapchain.on_bi.bi_db_config = ldap_chain_config;
 	ldapchain.on_bi.bi_db_destroy = ldap_chain_destroy;
+	
+	/* ... otherwise the underlying backend's function would be called,
+	 * likely passing an invalid entry; on the contrary, the requested
+	 * operational attributes should have been returned while chasing
+	 * the referrals.  This all in all is a bit messy, because part
+	 * of the operational attributes are generated by they backend;
+	 * part by the frontend; back-ldap should receive all the available
+	 * ones from the remote server, but then, on it own, it strips those
+	 * it assumes will be (re)generated by the frontend (e.g.
+	 * subschemaSubentry.) */
+	ldapchain.on_bi.bi_operational = ldap_chain_operational;
+	
 	ldapchain.on_response = ldap_chain_response;
 
+
 	ldapchain.on_bi.bi_chk_referrals = ldap_chain_chk_referrals;
 
 	return overlay_register( &ldapchain );