Ticket #47571 - targetattr ACIs ignore subtype

Description:
Subtypes in targetattr, userattr in aci as well as filter and attribute list
in the search are supported.
* If targetattr contains subtypes, the base type only as well as other subtypes
  are not allowed to access (or denied to access).
* If userattr contains subtypes, the base type as well as other subtypes in
  entries do not match the userattr value.
* If attribute list in search has a base type attribute, and a targetattr has
  a type with subtypes, then only the subtyped value is returned.  E.g.,
    attribute list: sn
    targetattr: sn;en
      ==>
    sn;en: <sn-en-value> and
    sn;en;phonetic: <sn-en-phonetic-value> are returned
    but
    sn or sn;fr is not.
  If attribute list has a type with subtype, then if the targetattr allows the
  subtype, the value is returned.  E.g.,
    attribute list: sn;en
    targetattr: sn;en
      ==>
    sn;en: <sn-en-value> and
    sn;en;phonetic: <sn-en-phonetic-value> are returned
    but
    sn or sn;fr is not.
1) slapd/attr.c
   * slapi_attr_type_cmp assumed the subtype order in 2 args are identical,
     but it is not always guaranteed.  Removed the assumption.
   * Added another compare type SLAPI_TYPE_CMP_SUBTYPES to comp_cmp which is
     called by slapi_attr_type_cmp to support full subtypes comparison.
2) plugin/acl.c:
   * Changed to call slapi_attr_type_cmp with human readable macros, e.g.,
     SLAPI_TYPE_CMP_BASE, SLAPI_TYPE_CMP_SUBTYPE, etc.
   * Replaced strcasecmp with slapi_attr_type_cmp for attribute type comparison.
   * Changed to call slapi_attr_type_cmp with SLAPI_TYPE_CMP_SUBTYPES (full
     subtype comparison) in acl__get_attrEval, where the next attribute to
	 compare is determined.
3) slapd/search.c,result.c
   send_all_attrs/send_specific_attrs use a dontsendattr array to control the
   duplicate attribute types.  Replaced the logic with a simpler one by creating
   an charray with no duplicates.

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

Reviewed by [email protected] (Thank you, Thierry!)

ldap/servers/plugins/acl/acl.c

@@ -549,15 +549,14 @@ acl_access_allowed(
 	** Check if we can use any cached information to determine 
 	** access to this resource
 	*/
-	if (  (access & SLAPI_ACL_SEARCH) &&
-	 (ret_val =  acl__match_handlesFromCache ( aclpb , attr, access))  != -1) {
+	if ((access & SLAPI_ACL_SEARCH) &&
+	    (ret_val = acl__match_handlesFromCache(aclpb, attr, access)) != -1) {
 		/* means got a result: allowed or not*/
 
 		if (ret_val == LDAP_SUCCESS ) {
-				decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_ALLOW;
+			decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_ALLOW;
 		} else if (ret_val == LDAP_INSUFFICIENT_ACCESS) {
-				decision_reason.reason =
-					ACL_REASON_EVALCONTEXT_CACHED_NOT_ALLOWED;
+			decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_NOT_ALLOWED;
 		}
 		goto cleanup_and_ret;
 	}
@@ -1058,7 +1057,8 @@ acl_read_access_allowed_on_entry (
 				slapi_ch_free ( (void **) &aclpb->aclpb_Evalattr);
 				aclpb->aclpb_Evalattr = slapi_ch_malloc(len+1);
 			}
-			PL_strncpyz (aclpb->aclpb_Evalattr, attr_type, len);
+			/* length needs to have 1 for '\0' */
+			PL_strncpyz (aclpb->aclpb_Evalattr, attr_type, len+1);
 #ifdef DETERMINE_ACCESS_BASED_ON_REQUESTED_ATTRIBUTES
 			if ( attr_index >= 0 ) {
 				/*
@@ -1191,38 +1191,36 @@ acl_read_access_allowed_on_attr (
 	}
 	
 	/*
-	 * Am I	a anonymous dude ? then	we can use our anonympous profile
+	 * Am I anonymous? then we can use our anonympous profile
 	 * We don't require the aclpb to have been initialized for anom stuff
-	 *
-	*/
+	 */
 	slapi_pblock_get (pb, SLAPI_REQUESTOR_DN ,&clientDn );
-	if ( clientDn && *clientDn == '\0' ) {		
-		ret_val =  aclanom_match_profile ( pb, aclpb, e, attr, 
-						SLAPI_ACL_READ );
+	if (clientDn && (*clientDn == '\0')) {
+		ret_val = aclanom_match_profile (pb, aclpb, e, attr, SLAPI_ACL_READ);
 		TNF_PROBE_1_DEBUG(acl_read_access_allowed_on_attr_end ,"ACL","",
-							tnf_string,anon_decision,"");
-		if (ret_val != -1 ) return ret_val;
+		                  tnf_string,anon_decision,"");
+		if (ret_val != -1) {
+			return ret_val;
+		}
 	}
 
 	/* Then I must have a access to the entry. */
 	aclpb->aclpb_state |= ACLPB_ACCESS_ALLOWED_ON_ENTRY;
 
-	if ( aclpb->aclpb_state & ACLPB_MATCHES_ALL_ACLS ) {		
+	if (aclpb->aclpb_state & ACLPB_MATCHES_ALL_ACLS) {
 
 		ret_val = acl__attr_cached_result (aclpb, attr, SLAPI_ACL_READ);
-		if (ret_val != -1 ) {
+		if (ret_val != -1) {
 			slapi_log_error(SLAPI_LOG_ACL, plugin_name, 
-				 "MATCHED HANDLE:dn:%s attr: %s val:%d\n", 
-				n_edn, attr, ret_val );
-			if ( ret_val == LDAP_SUCCESS) {
-				decision_reason.reason =
-						ACL_REASON_EVALCONTEXT_CACHED_ALLOW;
+			                "MATCHED HANDLE:dn:%s attr: %s val:%d\n", 
+			                n_edn, attr, ret_val );
+			if (ret_val == LDAP_SUCCESS) {
+				decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_ALLOW;
 			} else {
-				decision_reason.reason =
-							ACL_REASON_EVALCONTEXT_CACHED_NOT_ALLOWED;
-			}		
+				decision_reason.reason = ACL_REASON_EVALCONTEXT_CACHED_NOT_ALLOWED;
+			}
 			goto acl_access_allowed_on_attr_Exit;
-		 } else  {
+		} else {
 			aclpb->aclpb_state |= ACLPB_COPY_EVALCONTEXT;
 		}
 	}
@@ -1258,50 +1256,54 @@ acl_read_access_allowed_on_attr (
 		**  -- access  is allowed on phone
 		**  -- Don't know about the rest. Need to evaluate.
 		*/
-
-		if ( slapi_attr_type_cmp (attr, aclpb->aclpb_Evalattr, 1) == 0) {
+		if (slapi_attr_type_cmp(aclpb->aclpb_Evalattr, attr, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
 			/* from now on we need to evaluate access on
 			** rest of the attrs.
 			*/
-			aclpb->aclpb_state &= ~ACLPB_ACCESS_ALLOWED_ON_A_ATTR;
 			TNF_PROBE_1_DEBUG(acl_read_access_allowed_on_attr_end ,"ACL","",
-								tnf_string,aclp_Evalattr1,"");
-
+			                  tnf_string,aclp_Evalattr1,"");
 			return LDAP_SUCCESS;
-		} else {
-			/*
-			 * Here, the attr that implied access to the entry (aclpb_Evalattr),
-			 *  is not
-			 * the one we currently want evaluated--so
-			 * we need to evaluate access to attr--so fall through.
-			*/	 					
 		}
 
-	}  else if (aclpb->aclpb_state & ACLPB_ACCESS_ALLOWED_USERATTR) {
+		/*
+		 * Here, the attr that implied access to the entry (aclpb_Evalattr),
+		 *  is not
+		 * the one we currently want evaluated--so
+		 * we need to evaluate access to attr--so fall through.
+		 */
+
+	} else if (aclpb->aclpb_state & ACLPB_ACCESS_ALLOWED_USERATTR) {
 		/* Only skip evaluation on the user attr on which we have
 		** evaluated before.
 		*/
-		if ( slapi_attr_type_cmp (attr, aclpb->aclpb_Evalattr, 1) == 0) {
+		if (slapi_attr_type_cmp(aclpb->aclpb_Evalattr, attr, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
 			aclpb->aclpb_state &= ~ACLPB_ACCESS_ALLOWED_USERATTR;
 			TNF_PROBE_1_DEBUG(acl_read_access_allowed_on_attr_end ,"ACL","",
-								tnf_string,aclp_Evalattr2,"");
+			                  tnf_string,aclp_Evalattr2,"");
 			return LDAP_SUCCESS;
 		}
 	}
 
 	/* we need to evaluate the access on this attr */
+	/*
+	 * targetattr=sn;en
+	 * search attribute list: cn sn
+	 * ==>
+	 * attr: cn sn;en
+	 * aclpb_Evalattr: sn;en
+	 */
 	return ( acl_access_allowed(pb, e, attr, val, access) );
 
 	/* This exit point prints a summary and returns ret_val */
 acl_access_allowed_on_attr_Exit:
 
-	/* print summary if loglevel set */			
+	/* print summary if loglevel set */
 	if ( slapi_is_loglevel_set(loglevel) ) {
 		
 		print_access_control_summary( "on attr",
 										ret_val, clientDn, aclpb,
 										acl_access2str(SLAPI_ACL_READ),
-										attr, n_edn,											&decision_reason);
+										attr, n_edn, &decision_reason);
 	}
 	TNF_PROBE_0_DEBUG(acl_read_access_allowed_on_attr_end ,"ACL","");
 
@@ -1697,8 +1699,9 @@ acl_modified (Slapi_PBlock *pb, int optype, Slapi_DN *e_sdn, void *change)
 
 		mods = (LDAPMod **) change;
 		
-		for (j=0;  mods[j] != NULL; j++) {
-			if (strcasecmp(mods[j]->mod_type, aci_attr_type) == 0) {
+		for (j=0; mods && mods[j]; j++) {
+			if (slapi_attr_type_cmp(mods[j]->mod_type, aci_attr_type,
+			                        SLAPI_TYPE_CMP_SUBTYPE) == 0) {
 
 				/* Got an aci to mod in this list of mods, so
 				 * take the acicache lock for the whole list of mods,
@@ -2165,7 +2168,7 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 		} else {
 			dn_matched = ACL_TRUE;
 		}
-														
+
 		if (aci->aci_type & ACI_TARGET_NOT) {
 			matches = (dn_matched ? ACL_FALSE : ACL_TRUE);
 		} else {
@@ -2347,7 +2350,7 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 					 * of the attribute in the entry--otherwise you
 					 * could satisfy the filter and then put loads of other
 					 * values in on the back of it.
-				 	*/
+					 */
 
 					sval=NULL;
 					attrVal=NULL;
@@ -2356,18 +2359,16 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 					while(k != -1 && !done) {
 						attrVal = slapi_value_get_berval(sval);
 
-						if ( acl__make_filter_test_entry(
-                            	&aclpb->aclpb_filter_test_entry,
-								attrFilter->attr_str,
-								(struct berval *)attrVal) == LDAP_SUCCESS ) {                      
-				
-							attr_matched= acl__test_filter( 
-                            	    	aclpb->aclpb_filter_test_entry,
-						   	   			attrFilter->filter,
-						   	   			1 /* Do filter sense evaluation below */
-							 			);
+						if (acl__make_filter_test_entry(&aclpb->aclpb_filter_test_entry,
+						                                attrFilter->attr_str,
+						                                (struct berval *)attrVal) == LDAP_SUCCESS ) {
+
+							attr_matched = acl__test_filter(aclpb->aclpb_filter_test_entry,
+							                                attrFilter->filter,
+							                                1 /* Do filter sense evaluation below */
+							                               );
 							done = !attr_matched;
-							slapi_entry_free( aclpb->aclpb_filter_test_entry );			
+							slapi_entry_free( aclpb->aclpb_filter_test_entry );
 						}                                           
 						
 						k= slapi_attr_next_value(attr_ptr, k, &sval);
@@ -2379,8 +2380,8 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 					 * of the attribute in the entry satisfied the filter.
 					 * Otherwise, attr_matched is ACL_FALSE and not every 
 					 * value satisfied the filter, so we will teminate the
-					 * scan of the filter list.					
-					*/
+					 * scan of the filter list.
+					 */
 
 				}
 
@@ -2391,9 +2392,9 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 		 * Here, we've applied all the applicable filters to the entry.
 		 * Each one must have been satisfied by all the values of the attribute.
 		 * The result of this is stored in attr_matched.
-		*/		
+		 */
 
-#if 0		
+#if 0
 		/*
 		 * Don't support a notion of "add != " or "del != "
 		 * at the moment.
@@ -2415,12 +2416,10 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 			goto acl__resource_match_aci_EXIT;	
 		}
 
-	} else  if ( ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_ADD) &&
-                  (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) ||
-                 ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_DEL) &&
-                  (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)) ) {
-
-            
+	} else if ( ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_ADD) &&
+                 (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) ||
+                ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_DEL) &&
+                 (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)) ) {
 		/*     
 		 * Here, it's a modify add/del and we have attr filters.  
 		 * So, we need to scan the add/del filter list to find the filter
@@ -2436,15 +2435,11 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 		int found = 0;
 
 		if ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_ADD) &&
-			(aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) {
-
+		    (aci->aci_type & ACI_TARGET_ATTR_ADD_FILTERS)) {
 			attrFilterArray = aci->targetAttrAddFilters;
-
 		} else if ((aclpb->aclpb_access & ACLPB_SLAPI_ACL_WRITE_DEL) && 
-				   (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)) {
-				
+		           (aci->aci_type & ACI_TARGET_ATTR_DEL_FILTERS)) {
 			attrFilterArray = aci->targetAttrDelFilters;
-
 		}
 		
 
@@ -2456,12 +2451,12 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 		num_attrs = 0;
 
 		while (attrFilterArray[num_attrs] && !found) {
-				attrFilter = attrFilterArray[num_attrs];
+			attrFilter = attrFilterArray[num_attrs];
 
 			/* If this filter applies to the attribute, stop. */
 			if ((aclpb->aclpb_curr_attrEval) &&
-				slapi_attr_type_cmp ( aclpb->aclpb_curr_attrEval->attrEval_name,
-									  attrFilter->attr_str, 1) == 0) {
+				slapi_attr_type_cmp(aclpb->aclpb_curr_attrEval->attrEval_name,
+				                    attrFilter->attr_str, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
 				found = 1;
 			}
 			num_attrs++;
@@ -2475,17 +2470,16 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 
 		if (found) {
 
-			if ( acl__make_filter_test_entry(
-                            	&aclpb->aclpb_filter_test_entry,
-								aclpb->aclpb_curr_attrEval->attrEval_name,
-								aclpb->aclpb_curr_attrVal) == LDAP_SUCCESS ) {                      
+			if (acl__make_filter_test_entry(&aclpb->aclpb_filter_test_entry,
+			                                aclpb->aclpb_curr_attrEval->attrEval_name,
+			                                aclpb->aclpb_curr_attrVal) == LDAP_SUCCESS ) {
 				
 				attr_matched= acl__test_filter(aclpb->aclpb_filter_test_entry,
 										attrFilter->filter,
 										1 /* Do filter sense evaluation below */
-										);                            
-				slapi_entry_free( aclpb->aclpb_filter_test_entry );			
-			}           
+										);
+				slapi_entry_free( aclpb->aclpb_filter_test_entry );
+			}
 
 			/* No need to look further */
 			if (attr_matched == ACL_FALSE) {
@@ -2500,7 +2494,7 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 			*/
 		
 			attr_matched_in_targetattrfilters = 1;
-		}                                
+		}
 	} /* targetvaluefilters */ 
 	
 
@@ -2572,7 +2566,7 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
      * bother to look at the attrlist.
     */
     
-    if (!attr_matched_in_targetattrfilters) {                
+    if (!attr_matched_in_targetattrfilters) {
             
 	/* match target attr */
 	if ((c_attrEval) && 
@@ -2587,10 +2581,13 @@ acl__resource_match_aci( Acl_PBlock *aclpb, aci_t *aci, int skip_attrEval, int *
 			num_attrs = 0;
 
 			while (attrArray[num_attrs] && !attr_matched) {
-				attr = attrArray[num_attrs];           
-	            if (attr->attr_type & ACL_ATTR_STRING) { 
-					if (slapi_attr_type_cmp ( res_attr, 
-					      		attr->u.attr_str, 1) == 0) {
+				attr = attrArray[num_attrs];
+				if (attr->attr_type & ACL_ATTR_STRING) { 
+					/*
+					 * res_attr: attr type to eval (e.g., filter "(sn;en=*)")
+					 * attr->u.attr_str: targetattr value (e.g., sn;en)
+					 */
+					if (slapi_attr_type_cmp(attr->u.attr_str, res_attr, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
 						attr_matched = ACL_TRUE;
 						*a_matched = ACL_TRUE;
 					} 
@@ -3518,7 +3515,7 @@ acl__attr_cached_result (struct acl_pblock *aclpb, char *attr, int access )
 
 		if ( a_eval == NULL ) continue;
 
-		if (strcasecmp ( attr, a_eval->attrEval_name ) == 0 ) {
+		if (slapi_attr_type_cmp(a_eval->attrEval_name, attr, SLAPI_TYPE_CMP_SUBTYPE) == 0) {
 			if ( access & SLAPI_ACL_SEARCH ) {
 				if (a_eval->attrEval_s_status < ACL_ATTREVAL_DETERMINISTIC ) {
 					if ( a_eval->attrEval_s_status & ACL_ATTREVAL_SUCCESS)
@@ -3708,8 +3705,9 @@ acl_copyEval_context ( struct acl_pblock *aclpb, aclEvalContext *src,
 			continue;
 
 		for ( j = 0; j < dest->acle_numof_attrs; j++ ) {
-			if ( strcasecmp ( src->acle_attrEval[i].attrEval_name,
-					  dest->acle_attrEval[j].attrEval_name ) == 0 ) {
+			if (slapi_attr_type_cmp(src->acle_attrEval[i].attrEval_name,
+			                        dest->acle_attrEval[j].attrEval_name,
+			                        SLAPI_TYPE_CMP_SUBTYPE) == 0) {
 				/* We have it. skip it. */
 				attr_exists = 1;
 				dd_slot = j;
@@ -3751,8 +3749,7 @@ acl_copyEval_context ( struct acl_pblock *aclpb, aclEvalContext *src,
 			(size_t)src->acle_numof_tmatched_handles, sizeof( int ), acl__cmp );
 
 	for (i=0; i < src->acle_numof_tmatched_handles; i++ )  {
-		dest->acle_handles_matched_target[i]  =
-				src->acle_handles_matched_target[i];
+		dest->acle_handles_matched_target[i] = src->acle_handles_matched_target[i];
 	}
 
 	if ( src->acle_numof_tmatched_handles ) {
@@ -3917,9 +3914,12 @@ acl__get_attrEval ( struct acl_pblock *aclpb, char *attr )
 	/* Go thru and see if we have the attr already */
 	for (j=0; j < c_ContextEval->acle_numof_attrs; j++) {
 		c_attrEval = &c_ContextEval->acle_attrEval[j];
-			
-		if ( c_attrEval && 
-			slapi_attr_type_cmp ( c_attrEval->attrEval_name, attr, 1) == 0 ) {
+
+		if (c_attrEval && 
+		    /* attr: e.g., filter "(sn;en=*)" / attr list / attr in entry */
+		    /* This compare must check all subtypes.  "sn" vs. "sn;fr" should return 1 */
+		    slapi_attr_type_cmp(c_attrEval->attrEval_name,
+		                        attr, SLAPI_TYPE_CMP_SUBTYPES) == 0) {
 			aclpb->aclpb_curr_attrEval = c_attrEval;
 			break;
 		}

ldap/servers/plugins/acl/aclparse.c

@@ -2224,8 +2224,7 @@ static int type_compare( Slapi_Filter *f, void *arg) {
 	if (slapi_filter_get_attribute_type( f, &filter_type) == 0) {
 		t = slapi_attr_syntax_normalize(t);
 		filter_type = slapi_attr_syntax_normalize(filter_type);
-
-		if (slapi_attr_type_cmp(filter_type, t, 1) == 0) {
+		if (slapi_attr_type_cmp(filter_type, t, SLAPI_TYPE_CMP_BASE) == 0) {
 			rc = SLAPI_FILTER_SCAN_CONTINUE;
 		}
 

ldap/servers/plugins/acl/aclplugin.c

@@ -374,11 +374,11 @@ acl_access_allowed_main ( Slapi_PBlock *pb, Slapi_Entry *e, char **attrs,
 
 	if (attrs && *attrs) attr = attrs[0];
 
-	if (ACLPLUGIN_ACCESS_READ_ON_ENTRY == flags)
+	if (ACLPLUGIN_ACCESS_READ_ON_ENTRY == flags) {
 		rc = acl_read_access_allowed_on_entry ( pb, e, attrs, access);
-	else if ( ACLPLUGIN_ACCESS_READ_ON_ATTR == flags) {
+	} else if ( ACLPLUGIN_ACCESS_READ_ON_ATTR == flags) {
 		if (attr == NULL) {
-        		slapi_log_error( SLAPI_LOG_PLUGIN, plugin_name, "Missing attribute\n" );
+			slapi_log_error( SLAPI_LOG_PLUGIN, plugin_name, "Missing attribute\n" );
 			rc = LDAP_OPERATIONS_ERROR;
 		} else {
 			rc = acl_read_access_allowed_on_attr ( pb, e, attr, val, access);

ldap/servers/slapd/attr.c

@@ -76,8 +76,12 @@ typedef struct slapi_attr_value_index {
  * find the next component of an attribute type.
  */
 static const char *
-next_comp( const char *s )
+next_comp( const char *sp )
 {
+	char *s = (char *)sp;
+	if (NULL == s) {
+		return( NULL );
+	}
 	while ( *s && *s != ';' ) {
 		s++;
 	}
@@ -93,14 +97,30 @@ next_comp( const char *s )
  * compare two components of an attribute type.
  */
 static int
-comp_cmp( const char *s1, const char *s2 )
+comp_cmp( const char *s1p, const char *s2p )
 {
-	while ( *s1 && *s1 != ';' && tolower( *s1 ) == tolower( *s2 ) ) {
+	char *s1 = (char *)s1p;
+	char *s2 = (char *)s2p;
+	if (NULL == s1) {
+		if (s2) {
+			return 1;
+		}
+		return 0;
+	} 
+	if (NULL == s2) {
+		if (s1) {
+			return 1;
+		}
+		return 0;
+	} 
+	while ( *s1 && (*s1 != ';') &&
+	        *s2 && (*s2 != ';') &&
+	        tolower( *s1 ) == tolower( *s2 ) ) {
 		s1++, s2++;
 	}
 	if ( *s1 != *s2 ) {
-		if ( (*s1 == '\0' || *s1 == ';') &&
-		    (*s2 == '\0' || *s2 == ';') ) {
+		if ((*s1 == '\0' || *s1 == ';') &&
+		    (*s2 == '\0' || *s2 == ';')) {
 			return( 0 );
 		} else {
 			return( 1 );
@@ -115,16 +135,18 @@ slapi_attr_type_cmp( const char *a1, const char *a2, int opt )
 {
 	int	rc= 0;
 
-    switch ( opt ) {
-    case SLAPI_TYPE_CMP_EXACT: /* compare base name + options as given */
-        rc = strcasecmp( a1, a2 );
+	switch ( opt ) {
+	case SLAPI_TYPE_CMP_EXACT: /* compare base name + options as given */
+		rc = strcasecmp( a1, a2 );
 		break;
 
-    case SLAPI_TYPE_CMP_BASE: /* ignore options on both names - compare base names only */
+	case SLAPI_TYPE_CMP_BASE: /* ignore options on both names - compare base names only */
 		rc = comp_cmp( a1, a2 );
-        break;
+		break;
 
-    case SLAPI_TYPE_CMP_SUBTYPE: /* ignore options on second name that are not in first name */
+	case SLAPI_TYPE_CMP_SUBTYPE: /* ignore options on second name that are not in first name */
+	{
+		const char *b2 = a2;
 		/*
 		 * first, check that the base types match
 		 */
@@ -134,24 +156,73 @@ slapi_attr_type_cmp( const char *a1, const char *a2, int opt )
 		}
 		/*
 		 * next, for each component in a1, make sure there is a
-		 * matching component in a2. the order must be the same,
-		 * so we can keep looking where we left off each time in a2
+		 * matching component in a2. 
 		 */
 		rc = 0;
-		for ( a1 = next_comp( a1 ); a1 != NULL; a1 = next_comp( a1 ) ) {
-			for ( a2 = next_comp( a2 ); a2 != NULL;
-			    a2 = next_comp( a2 ) ) {
-				if ( comp_cmp( a1, a2 ) == 0 ) {
+		for ( a1 = next_comp( a1 ); a1; a1 = next_comp( a1 ) ) {
+			for ( b2 = next_comp( b2 ); b2; b2 = next_comp( b2 ) ) {
+				if ( comp_cmp( a1, b2 ) == 0 ) {
 					break;
 				}
 			}
-			if ( a2 == NULL ) {
+			if ( b2 == NULL ) {
 				rc = 1;
 				break;
 			}
+			b2 = a2;
 		}
-        break;
-    }
+		break;
+	}
+	case SLAPI_TYPE_CMP_SUBTYPES: /* Both share the same subtypes */
+	{
+		const char *b1 = a1;
+		const char *b2 = a2;
+		/*
+		 * first, check that the base types match
+		 */
+		if (comp_cmp(a1, a2) != 0) {
+			rc = 1;
+			break;
+		}
+		/*
+		 * next, for each component in a1, make sure there is a
+		 * matching component in a2. 
+		 */
+		rc = 0;
+		for (b1 = next_comp(b1); b1; b1 = next_comp(b1)) {
+			for (b2 = next_comp(b2); b2; b2 = next_comp(b2)) {
+				if (comp_cmp(b1, b2) == 0) {
+					break;
+				}
+			}
+			if (b2 == NULL) {
+				rc = 1;
+				break;
+			}
+			b2 = a2;
+		}
+		if (0 == rc) {
+			b1 = a1;
+			b2 = a2;
+			/*
+			 * next, for each component in a2, make sure there is a
+			 * matching component in a1. 
+			 */
+			for (b2 = next_comp(b2); b2; b2 = next_comp(b2)) {
+				for (b1 = next_comp(b1); b1; b1 = next_comp(b1)) {
+					if (comp_cmp(b1, b2) == 0) {
+						break;
+					}
+				}
+				if (b1 == NULL) {
+					rc = 1;
+					break;
+				}
+				b1 = a1;
+			}
+		}
+	} /* SLAPI_TYPE_CMP_SUBTYPES */
+	} /* switch (opt) */
 
 	return( rc );
 }

ldap/servers/slapd/result.c

@@ -1002,17 +1002,17 @@ encode_attr_2(
 	attrs[0] = (char*)attribute_type;
 
 #if !defined(DISABLE_ACL_CHECK)
-	if ( plugin_call_acl_plugin (pb, e, attrs, NULL, SLAPI_ACL_READ, 
-				     ACLPLUGIN_ACCESS_READ_ON_ATTR, NULL ) != LDAP_SUCCESS ) {
+	if (plugin_call_acl_plugin(pb, e, attrs, NULL, SLAPI_ACL_READ, 
+	                           ACLPLUGIN_ACCESS_READ_ON_ATTR, NULL) != LDAP_SUCCESS) {
 		return( 0 );
 	}
 #endif
 
-	if ( ber_printf( ber, "{s[", returned_type ) == -1 ) {
+	if (ber_printf(ber, "{s[", returned_type?returned_type:attribute_type) == -1) {
 		LDAPDebug( LDAP_DEBUG_ANY, "ber_printf failed\n", 0, 0, 0 );
 		ber_free( ber, 1 );
-		send_ldap_result( pb, LDAP_OPERATIONS_ERROR, NULL,
-		    "ber_printf type", 0, NULL );
+		send_ldap_result(pb, LDAP_OPERATIONS_ERROR, NULL,
+		                 "ber_printf type", 0, NULL);
 		return( -1 );
 	}
 
@@ -1146,7 +1146,7 @@ static const char *idds_map_attrt_v3(
 
 /* Helper functions */
 
-static int send_all_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_PBlock *pb,BerElement *ber,int attrsonly,int ldapversion,int *dontsendattr, int real_attrs_only, int some_named_attrs)
+static int send_all_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_PBlock *pb,BerElement *ber,int attrsonly,int ldapversion, int real_attrs_only, int some_named_attrs)
 {
 	int i = 0;
 	int rc = 0;
@@ -1259,10 +1259,11 @@ static int send_all_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_
 					&attr_free_flags, 
 					&item_count
 					);
-			if (0 == rc && item_count > 0) {
+			if ((0 == rc) && (item_count > 0)) {
 
 				for(iter=0; iter<item_count; iter++)
 				{
+					int skipit;
 					if ( rc != 0 ) {
 						/* we hit an error - we need to free all of the stuff allocated by
 						   slapi_vattr_namespace_values_get_sp */
@@ -1274,31 +1275,28 @@ static int send_all_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_
 						name_to_return = actual_type_name[iter]; 
 					}
 
-					/*
-					* The dontsendattr array is used to track whether attributes
-					* that were explicitly requested by the client have been
-					* returned. Check here to see if the attribute we just
-					* arranged to send back was explicitly requested, and if so,
-					* set its dontsendattr flag so the send_specific_attrs()
-					* function does not return it a second time.
-					*/					
-					for ( i = 0; attrs != NULL && attrs[i] != NULL; i++ ) {
-						if ( !dontsendattr[i] && slapi_attr_type_cmp( current_type_name, attrs[i], SLAPI_TYPE_CMP_SUBTYPE ) == 0 ) {
-						/* Client is also asking for an attr which is in '*', zap it. */
-							dontsendattr[i]= 1;
+					/* If current_type_name is in attrs, we could rely on send_specific_attrs. */
+					skipit = 0;
+					for (i = 0; attrs && attrs[i]; i++) {
+						if (slapi_attr_type_cmp(current_type_name, attrs[i], SLAPI_TYPE_CMP_SUBTYPE) == 0) {
+							skipit = 1;
+							break;
 						}
 					}
 
-					rc = encode_attr_2( pb, ber, e, values[iter], attrsonly, current_type_name, name_to_return );
-
-					if (rewrite_rfc1274 != 0) {
-						v2name = idds_map_attrt_v3(current_type_name);
-						if (v2name != NULL) {
-							/* also return values with RFC1274 attr name */
-							rc = encode_attr_2(pb, ber, e, values[iter], 
-								   attrsonly, 
-								   current_type_name, 
-								   v2name);
+					if (!skipit) {
+						rc = encode_attr_2(pb, ber, e, values[iter], attrsonly, 
+						                   current_type_name, name_to_return );
+
+						if (rewrite_rfc1274 != 0) {
+							v2name = idds_map_attrt_v3(current_type_name);
+							if (v2name != NULL) {
+								/* also return values with RFC1274 attr name */
+								rc = encode_attr_2(pb, ber, e, values[iter], 
+								                   attrsonly, 
+								                   current_type_name, 
+								                   v2name);
+							}
 						}
 					}
 
@@ -1329,137 +1327,143 @@ exit:
 	return rc;
 }
 
-int send_specific_attrs(Slapi_Entry *e,char **attrs,Slapi_Operation *op,Slapi_PBlock *pb,BerElement *ber,int attrsonly,int ldapversion,int *dontsendattr, int real_attrs_only)
+/*
+ * attrs need to expand including the subtypes found in the entry
+ * e.g., if "sn" is no the attrs and 'e' has sn, sn;en, and sn;fr,
+ * attrs should have sn, sn;en, and sn;fr, as well.
+ */
+int
+send_specific_attrs(Slapi_Entry *e, char **attrs, Slapi_Operation *op,
+                    Slapi_PBlock *pb, BerElement *ber, int attrsonly,
+                    int ldapversion, int real_attrs_only)
 {
-	int i,j = 0;
+	int i = 0;
 	int rc = 0;
 	int vattr_flags = 0;
 	vattr_context *ctx;
+	char **attrs_ext = NULL;
+	char **my_searchattrs = NULL;
 
-	if(real_attrs_only == SLAPI_SEND_VATTR_FLAG_REALONLY)
+	if (real_attrs_only == SLAPI_SEND_VATTR_FLAG_REALONLY) {
 		vattr_flags = SLAPI_REALATTRS_ONLY;
-	else
-	{
+	} else {
 		vattr_flags = SLAPI_VIRTUALATTRS_REQUEST_POINTERS;
 		if(real_attrs_only == SLAPI_SEND_VATTR_FLAG_VIRTUALONLY)
 			vattr_flags |= SLAPI_VIRTUALATTRS_ONLY;
 	}
+
+	/* Create a copy of attrs with no duplicates */
+	if (attrs) {
+		for (i = 0; attrs[i]; i++) {
+			if (!charray_inlist(attrs_ext, attrs[i])) {
+				slapi_ch_array_add(&attrs_ext, slapi_ch_strdup(attrs[i]));
+				slapi_ch_array_add(&my_searchattrs, slapi_ch_strdup(op->o_searchattrs[i]));
+			}
+		}
+	}
+	if (attrs_ext) {
+		attrs = attrs_ext;
+	}
 	
-	for ( i = 0; attrs != NULL && attrs[i] != NULL; i++ )
-	{
+	for ( i = 0; attrs && attrs[i] != NULL; i++ ) {
 		char *current_type_name = attrs[i];
-        if(!dontsendattr[i]) {
-			Slapi_ValueSet **values = NULL;
-			int attr_free_flags = 0;
-			char *name_to_return = NULL;
-			char **actual_type_name= NULL;
-			int *type_name_disposition = 0;
-			int item_count = 0;
-			int iter = 0;
-			Slapi_DN *namespace_dn;
-			Slapi_Backend *backend=0;
-
-    		/*
-    		 * Here we call the computed attribute code to see whether
-    		 * the requested attribute is to be computed. 
-    		 * The subroutine compute_attribute calls encode_attr on our behalf, in order
-    		 * to avoid the inefficiency of returning a complex structure
-    		 * which we'd have to free
-    		 */
-    		rc = compute_attribute(attrs[i],pb,ber,e,attrsonly,op->o_searchattrs[i]);
-    		if (0 == rc) {
-    			continue; /* Means this was a computed attr and we prcessed it OK. */
-    		}
-    		if (-1 != rc) {
-    			/* Means that some error happened */
-    			return rc;
-    		}
-    		else {
-    			rc = 0; /* Means that we just didn't recognize this as a computed attr */
-    		}
-
-			/* get the namespace dn */
-			slapi_pblock_get( pb, SLAPI_BACKEND, (void *)&backend);
-			namespace_dn = (Slapi_DN*)slapi_be_getsuffix(backend, 0);
+		Slapi_ValueSet **values = NULL;
+		int attr_free_flags = 0;
+		char *name_to_return = NULL;
+		char **actual_type_name= NULL;
+		int *type_name_disposition = 0;
+		int item_count = 0;
+		int iter = 0;
+		Slapi_DN *namespace_dn;
+		Slapi_Backend *backend=0;
 
-			/* Get the attribute value from the vattr service */
-			/* ctx will be freed by attr_context_ungrok() */
-			ctx = vattr_context_new ( pb );
-			rc = slapi_vattr_namespace_values_get_sp(
-					ctx,
-					e,
-					namespace_dn,
-					current_type_name,
-					&values,
-					&type_name_disposition,
-					&actual_type_name,
-					vattr_flags,
-					&attr_free_flags, 
-					&item_count
-					);
-			if (0 == rc && item_count > 0) {
+		/*
+		 * Here we call the computed attribute code to see whether
+		 * the requested attribute is to be computed. 
+		 * The subroutine compute_attribute calls encode_attr on our behalf, in order
+		 * to avoid the inefficiency of returning a complex structure
+		 * which we'd have to free
+		 */
+		rc = compute_attribute(attrs[i], pb, ber, e, attrsonly, my_searchattrs[i]);
+		if (0 == rc) {
+			continue; /* Means this was a computed attr and we prcessed it OK. */
+		}
+		if (-1 != rc) {
+			/* Means that some error happened */
+			return rc;
+		}
+		else {
+			rc = 0; /* Means that we just didn't recognize this as a computed attr */
+		}
 
-				for(iter=0; iter<item_count; iter++)
-				{
-					if ( rc != 0 ) {
-						/* we hit an error - we need to free all of the stuff allocated by
-						   slapi_vattr_namespace_values_get_sp */
-						slapi_vattr_values_free(&(values[iter]), &(actual_type_name[iter]), attr_free_flags);
+		/* get the namespace dn */
+		slapi_pblock_get( pb, SLAPI_BACKEND, (void *)&backend);
+		namespace_dn = (Slapi_DN*)slapi_be_getsuffix(backend, 0);
+
+		/* Get the attribute value from the vattr service */
+		/* This call handles subtype, as well.
+		 * e.g., current_type_name: cn 
+		 * ==>
+		 * item_count: 5; actual_type_name: cn;sub0, ..., cn;sub4 */
+		/* ctx will be freed by attr_context_ungrok() */
+		ctx = vattr_context_new ( pb );
+		rc = slapi_vattr_namespace_values_get_sp(
+				ctx,
+				e,
+				namespace_dn,
+				current_type_name,
+				&values,
+				&type_name_disposition,
+				&actual_type_name,
+				vattr_flags,
+				&attr_free_flags, 
+				&item_count
+				);
+		if ((0 == rc) && (item_count > 0)) {
+			for (iter = 0; iter < item_count; iter++) {
+				if ( rc != 0 ) {
+					/* we hit an error - we need to free all of the stuff allocated by
+					   slapi_vattr_namespace_values_get_sp */
+					slapi_vattr_values_free(&(values[iter]), &(actual_type_name[iter]), attr_free_flags);
+					continue;
+				}
+				if (SLAPI_VIRTUALATTRS_TYPE_NAME_MATCHED_SUBTYPE == type_name_disposition[iter]) {
+					name_to_return = actual_type_name[iter]; 
+					if ((iter > 0) && charray_inlist(attrs, name_to_return)) {
+						/* subtype retrieved by slapi_vattr_namespace_values_get_sp is
+						 * included in the attr list.  Skip the dup. */
 						continue;
 					}
-					if (SLAPI_VIRTUALATTRS_TYPE_NAME_MATCHED_SUBTYPE == type_name_disposition[iter]) {
-						name_to_return = actual_type_name[iter]; 
-					} else {
-						name_to_return = op->o_searchattrs[i];
-					}
-
-					/*
-					 * The client may have specified a list of attributes
-					 * with duplicates, 'cn cn cn'.
-					 * We need to determine which of any duplicates take precedence
-					 * For subtypes, the attribute which is most generic should be
-					 * returned (since it will also trigger the return of the less
-					 * generic attribute subtypes.
-					 */
-					for ( j = i+1; attrs[j] != NULL && dontsendattr[i]==0; j++ )
-					{
-						if ( !dontsendattr[j] && slapi_attr_type_cmp( attrs[j], actual_type_name[iter], SLAPI_TYPE_CMP_SUBTYPE ) == 0 )
-						{
-							/* discover which is the more generic attribute and cancel the other*/
-							int attrbase = slapi_attr_type_cmp( attrs[j], current_type_name, SLAPI_TYPE_CMP_EXACT );
-
-							if(attrbase >= 0)
-								dontsendattr[j]= 1;
-							else
-								dontsendattr[i]= 1; /* the current value is superceeded later */
-						}
-					}
-
-					/* we may have just cancelled ourselves so check */
-					if(!dontsendattr[i])
-						rc = encode_attr_2( pb, ber, e, values[iter], attrsonly, current_type_name, name_to_return );
-					
-					slapi_vattr_values_free(&(values[iter]), &(actual_type_name[iter]), attr_free_flags);
+				} else {
+					name_to_return = my_searchattrs[i];
 				}
 
-				slapi_ch_free((void**)&actual_type_name);
-				slapi_ch_free((void**)&type_name_disposition);
-				slapi_ch_free((void**)&values);
-				if ( rc != 0 ) {
-					goto exit;
-				}
+				/* need to pass actual_type_name (e.g., sn;en to evaluate the ACL */
+				rc = encode_attr_2(pb, ber, e, values[iter], attrsonly, 
+				                   actual_type_name[iter], name_to_return);
+				
+				slapi_vattr_values_free(&(values[iter]), &(actual_type_name[iter]), attr_free_flags);
+			}
 
-			} else {
-				/* if we got here, then either values is NULL or values contains no elements
-				   either way we can free it */
-				slapi_ch_free((void**)&values);
-				slapi_ch_free((void**)&actual_type_name);
-				slapi_ch_free((void**)&type_name_disposition);
-				rc = 0;
+			slapi_ch_free((void**)&actual_type_name);
+			slapi_ch_free((void**)&type_name_disposition);
+			slapi_ch_free((void**)&values);
+			if ( rc != 0 ) {
+				goto exit;
 			}
-        } 	
+
+		} else {
+			/* if we got here, then either values is NULL or values contains no elements
+			   either way we can free it */
+			slapi_ch_free((void**)&values);
+			slapi_ch_free((void**)&actual_type_name);
+			slapi_ch_free((void**)&type_name_disposition);
+			rc = 0;
+		}
 	}
 exit:
+	slapi_ch_array_free(attrs_ext);
+	slapi_ch_array_free(my_searchattrs);
 	return rc;
 
 }
@@ -1482,7 +1486,6 @@ send_ldap_search_entry_ext(
 	BerElement	*ber = NULL;
 	int		i, rc = 0, logit = 0;
 	int		alluserattrs, noattrs, some_named_attrs;
-	int *dontsendattr= NULL;
 	Slapi_Operation *operation;
 	int real_attrs_only = 0;
 	LDAPControl		**ctrlp = 0;
@@ -1596,11 +1599,6 @@ send_ldap_search_entry_ext(
 			    "Accepting illegal other attributes specified with "
 			    "special \"1.1\" attribute\n", 0, 0, 0 );
 		}
-        /*
-         * We maintain a flag array so that we can remove requests
-         * for duplicate attributes.
-         */
-    	dontsendattr = (int*) slapi_ch_calloc( i+1, sizeof(int) );
 	}
 
 
@@ -1625,12 +1623,13 @@ send_ldap_search_entry_ext(
 
 	/* look through each attribute in the entry */
 	if ( alluserattrs ) {
-		rc = send_all_attrs(e,attrs,op,pb,ber,attrsonly,conn->c_ldapversion,dontsendattr, real_attrs_only, some_named_attrs);
+		rc = send_all_attrs(e, attrs, op, pb, ber, attrsonly, conn->c_ldapversion,
+		                    real_attrs_only, some_named_attrs);
 	}
 	
 	/* if the client explicitly specified a list of attributes look through each attribute requested */
 	if( (rc == 0) && (attrs!=NULL) && !noattrs) {
-		rc = send_specific_attrs(e,attrs,op,pb,ber,attrsonly,conn->c_ldapversion,dontsendattr,real_attrs_only);
+		rc = send_specific_attrs(e,attrs,op,pb,ber,attrsonly,conn->c_ldapversion,real_attrs_only);
 	}
 
 	/* Append effective rights to the stream of attribute list */
@@ -1738,7 +1737,6 @@ cleanup:
 		ldap_controls_free(searchctrlp);
 	}
 	ber_free( ber, 1 );
-	slapi_ch_free( (void **) &dontsendattr );
 	LDAPDebug( LDAP_DEBUG_TRACE, "<= send_ldap_search_entry\n", 0, 0, 0 );
 
 	return( rc );
@@ -2153,7 +2151,6 @@ encode_read_entry (Slapi_PBlock *pb, Slapi_Entry *e, char **attrs, int alluserat
     LDAPControl **ctrlp = NULL;
     struct berval *bv = NULL;
     BerElement *ber = NULL;
-    int *dontsendattr = NULL;
     int real_attrs_only = 0;
     int rc = 0;
 
@@ -2191,13 +2188,12 @@ encode_read_entry (Slapi_PBlock *pb, Slapi_Entry *e, char **attrs, int alluserat
      * for duplicate attributes.  We also need to set o_searchattrs
      * for the attribute processing, as modify op's don't have search attrs.
      */
-    dontsendattr = (int*) slapi_ch_calloc( attr_count+1, sizeof(int) );
     op->o_searchattrs = attrs;
 
     /* Send all the attributes */
     if ( alluserattrs ) {
         rc = send_all_attrs(e, attrs, op, pb, ber, 0, conn->c_ldapversion,
-                            dontsendattr, real_attrs_only, attr_count);
+                            real_attrs_only, attr_count);
         if(rc){
             goto cleanup;
         }
@@ -2205,8 +2201,7 @@ encode_read_entry (Slapi_PBlock *pb, Slapi_Entry *e, char **attrs, int alluserat
 
     /* Send a specified list of attributes */
     if( attrs != NULL) {
-        rc = send_specific_attrs(e, attrs, op, pb, ber, 0, conn->c_ldapversion,
-                                 dontsendattr, real_attrs_only);
+        rc = send_specific_attrs(e, attrs, op, pb, ber, 0, conn->c_ldapversion, real_attrs_only);
         if(rc){
             goto cleanup;
         }
@@ -2222,7 +2217,6 @@ encode_read_entry (Slapi_PBlock *pb, Slapi_Entry *e, char **attrs, int alluserat
 cleanup:
 
     ber_free( ber, 1 );
-    slapi_ch_free( (void **) &dontsendattr );
     if(rc != 0){
         return NULL;
     } else {

ldap/servers/slapd/search.c

@@ -246,7 +246,15 @@ do_search( Slapi_PBlock *pb )
 	}
 
 	if ( attrs != NULL ) {
-		int aciin = 0;
+		char *normaci = slapi_attr_syntax_normalize("aci");
+		int replace_aci = 0;
+		if (0 == strcasecmp(normaci, "aci")) {
+			/* normaci is identical to "aci" */
+			slapi_ch_free_string(&normaci);
+			normaci = slapi_ch_strdup("aci");
+		} else {
+			replace_aci = 1;
+		}
 		/* 
 		 * . store gerattrs if any
 		 * . add "aci" once if "*" is given
@@ -274,13 +282,25 @@ do_search( Slapi_PBlock *pb )
 				charray_merge_nodup(&gerattrs, dummyary, 1);
 				/* null terminate the attribute name at the @ after it has been copied */
 				*p = '\0';
-			}
-			else if ( !aciin && strcasecmp(attrs[i], LDAP_ALL_USER_ATTRS) == 0 )
-			{
-				charray_add(&attrs, slapi_attr_syntax_normalize("aci"));
-				aciin = 1;
+			} else if (strcmp(attrs[i], LDAP_ALL_USER_ATTRS /* '*' */) == 0) {
+				if (!charray_inlist(attrs, normaci)) {
+					charray_add(&attrs, normaci); /* consumed */
+					normaci = NULL;
+				}
+			} else if (replace_aci && (strcasecmp(attrs[i], "aci") == 0)) {
+				slapi_ch_free_string(&attrs[i]);
+				if (charray_inlist(attrs, normaci)) { /* attrlist: "*" "aci" */
+					int j = i;
+					for (; attrs[j]; j++) { /* Shift the rest by 1 */
+						attrs[j] = attrs[j+1];
+					}
+				} else { /* attrlist: "aci" "*" */
+					attrs[i] = normaci; /* consumed */
+					normaci = NULL;
+				}
 			}
 		}
+		slapi_ch_free_string(&normaci);
 
 		if (config_get_return_orig_type_switch()) {
 			/* return the original type, e.g., "sn (surname)" */
@@ -376,6 +396,7 @@ free_and_return:;
 	if ( !psearch || rc != 0 ) {
 		slapi_ch_free_string(&fstr);
 		slapi_filter_free( filter, 1 );
+		slapi_pblock_get( pb, SLAPI_SEARCH_ATTRS, &attrs );
 		charray_free( attrs );	/* passing NULL is fine */
 		charray_free( gerattrs );	/* passing NULL is fine */
 		/* 

ldap/servers/slapd/slapi-plugin.h

@@ -4091,6 +4091,7 @@ int slapi_attr_value_find( const Slapi_Attr *a, const struct berval *v );
  *        \arg #SLAPI_TYPE_CMP_EXACT
  *        \arg #SLAPI_TYPE_CMP_BASE
  *        \arg #SLAPI_TYPE_CMP_SUBTYPE
+ *        \arg #SLAPI_TYPE_CMP_SUBTYPES
  * \return \c 0 if the type names are equal.
  * \return A non-zero value if the type names are not equal.
  * \see slapi_attr_type2plugin()
@@ -4122,6 +4123,13 @@ int slapi_attr_type_cmp( const char *t1, const char *t2, int opt );
  */
 #define SLAPI_TYPE_CMP_SUBTYPE	2
 
+/**
+ * Compare types including subtypes in the both args.
+ *
+ * \see slapi_attr_type_cmp()
+ */
+#define SLAPI_TYPE_CMP_SUBTYPES	3
+
 /**
  * Compare two attribute names to determine if they represent the same value.
  *