Mercurial > hg > freeDiameter

changeset 738:d666051658bd

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix broken 'almostcasecmp' logic
author Sebastien Decugis <sdecugis@nict.go.jp>
date Wed, 02 Mar 2011 18:21:59 +0900
parents 03bcd4870f44
children a47f7b36a309
files extensions/app_radgw/rgw_clients.c extensions/app_redirect/ard_rules.c extensions/rt_redirect/redir_fwd.c extensions/rt_redirect/redir_out.c include/freeDiameter/libfdproto.h libfdcore/p_ce.c libfdcore/peers.c libfdcore/routing_dispatch.c libfdproto/ostr.c libfdproto/rt_data.c tests/testostr.c
diffstat 11 files changed, 169 insertions(+), 59 deletions(-) [+]
line wrap: on
line diff
--- a/extensions/app_radgw/rgw_clients.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/extensions/app_radgw/rgw_clients.c	Wed Mar 02 18:21:59 2011 +0900
@@ -731,8 +731,8 @@
 		*/
 		
 		/* first, check if the nas_id is the fqdn of the peer or a known alias */
-		if (!fd_os_almostcasecmp(nas_id + 1, nas_id_len, 
-						cli->fqdn, cli->fqdn_len)) {
+		if (!fd_os_almostcasesrch(nas_id + 1, nas_id_len, 
+						cli->fqdn, cli->fqdn_len, NULL)) {
 			TRACE_DEBUG(FULL, "NAS-Identifier contains the fqdn of the client");
 			found = 1;
 		} else {
--- a/extensions/app_redirect/ard_rules.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/extensions/app_redirect/ard_rules.c	Wed Mar 02 18:21:59 2011 +0900
@@ -93,7 +93,7 @@
 	*match = 0;
 	
 	if (c->is_regex == 0) {
-		if ( ! fd_os_almostcasecmp(c->s, c->sl, s, l) )
+		if ( ! fd_os_almostcasesrch(c->s, c->sl, s, l, NULL) )
 			*match = 1;
 	} else {
 		int err;
--- a/extensions/rt_redirect/redir_fwd.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/extensions/rt_redirect/redir_fwd.c	Wed Mar 02 18:21:59 2011 +0900
@@ -149,7 +149,7 @@
 						/* The list is kept ordered by id so that it is faster to compare to candidates later */
 						for (li = task.rh.next; li != &task.rh; li = li->next) {
 							struct redir_host * nhost = li->o;
-							if ( fd_os_almostcasecmp(id, len, nhost->id, nhost->len) <= 0 )
+							if ( fd_os_cmp(id, len, nhost->id, nhost->len) <= 0 )
 								break;
 						}
 						fd_list_insert_before(li, &h->chain);
--- a/extensions/rt_redirect/redir_out.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/extensions/rt_redirect/redir_out.c	Wed Mar 02 18:21:59 2011 +0900
@@ -170,7 +170,7 @@
 		
 		/* Special case: ALL_HOST rules: we decrease the score of the Origin-Host if present */
 		if (e->type == ALL_HOST) {
-			cmp = fd_os_almostcasecmp(cand->diamid, cand->diamidlen, e->data.host.s, e->data.host.l);
+			cmp = fd_os_almostcasesrch(cand->diamid, cand->diamidlen, e->data.host.s, e->data.host.l, NULL);
 			if (!cmp) {
 				TRACE_DEBUG(FULL, "Redirect msg %p: peer '%.*s' += %d (previous ALL_HOST Redirect originated from this peer)", msg, cand->diamidlen, cand->diamid, FD_SCORE_SENT_REDIRECT);
 				cand->score += FD_SCORE_SENT_REDIRECT;
@@ -197,19 +197,20 @@
 		/* the candidates list is not guaranteed to be ordered at this time, so we cannot avoid the two imbricated loops */
 		struct rtd_candidate * cand = (struct rtd_candidate *) lic;
 		
-		/* Is this candidate in the "Redirect-Host" list ? */
+		/* Is this candidate in the "Redirect-Host" list ? We must search caseinsentive here. */
 		for (lirh = e->target_peers_list.next; lirh != &e->target_peers_list; lirh = lirh->next) {
 			struct redir_host * host = lirh->o;
-			
-			cmp = fd_os_cmp( cand->diamid, cand->diamidlen, host->id, host->len );
+			int cont;
 			
-			if (cmp < 0)
-				break; /* targets are ordered */
+			cmp = fd_os_almostcasesrch( cand->diamid, cand->diamidlen, host->id, host->len, &cont );
 			
 			if (cmp == 0) {
 				TRACE_DEBUG(FULL, "Redirect msg %p: peer '%.*s' += %d (rule t:%d @%p)", msg, cand->diamidlen, cand->diamid, redirects_usages[e->type].score, e->type, e);
 				cand->score += redirects_usages[e->type].score;
+				break;
 			}
+			if (!cont)
+				break;
 		}
 	}
 	
--- a/include/freeDiameter/libfdproto.h	Wed Mar 02 16:28:14 2011 +0900
+++ b/include/freeDiameter/libfdproto.h	Wed Mar 02 18:21:59 2011 +0900
@@ -620,14 +620,21 @@
 #define fd_os_cmp(_o1, _l1, _o2, _l2)  fd_os_cmp_int((os0_t)(_o1), _l1, (os0_t)(_o2), _l2)
 
 /* A roughly case-insensitive variant, which actually only compares ASCII chars (0-127) in a case-insentitive maneer 
-  -- this should be OK with (old) DNS names. Not sure how it works with IDN...
-  -- it also clearly does not support locales where a lowercase letter uses more space than upper case, such as ß -> ss
- It is slower than fd_os_cmp... 
- This function should give the same order as fd_os_cmp, except when it finds 2 strings to be equal.
+  -- it does not support locales where a lowercase letter uses more space than upper case, such as ß -> ss
+ It is slower than fd_os_cmp.
  Note that the result is NOT the same as strcasecmp !!!
+ 
+ This function gives the same order as fd_os_cmp, except when it finds 2 strings to be equal.
+ However this is not always sufficient:
+ 	for example fd_os_cmp gives: "Ac" < "aB" < "aa"
+	if you attempt to fd_os_almostcasesrch "Aa" you will actually have to go past "aB" which is > "Aa".
+	Therefore you can use the maybefurther parameter.
+	This parameter is 1 on return if os1 may have been stored further that os2 (assuming os2 values are ordered by fd_os_cmp)
+	and 0 if we are sure that it is not the case.
+	When looping through a list of fd_os_cmp classified values, this parameter must be used to stop looping, in addition to the comp result.
  */
-int fd_os_almostcasecmp_int(uint8_t * os1, size_t os1sz, uint8_t * os2, size_t os2sz);
-#define fd_os_almostcasecmp(_o1, _l1, _o2, _l2)  fd_os_almostcasecmp_int((os0_t)(_o1), _l1, (os0_t)(_o2), _l2)
+int fd_os_almostcasesrch_int(uint8_t * os1, size_t os1sz, uint8_t * os2, size_t os2sz, int * maybefurther);
+#define fd_os_almostcasesrch(_o1, _l1, _o2, _l2, _mb)  fd_os_almostcasesrch_int((os0_t)(_o1), _l1, (os0_t)(_o2), _l2, _mb)
 
 /* Analyze a DiameterURI and return its components. 
   Return EINVAL if the URI is not valid. 
--- a/libfdcore/p_ce.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/libfdcore/p_ce.c	Wed Mar 02 18:21:59 2011 +0900
@@ -303,8 +303,8 @@
 				}
 				
 				/* We check that the value matches what we know, otherwise disconnect the peer */
-				if (fd_os_almostcasecmp(hdr->avp_value->os.data, hdr->avp_value->os.len, 
-							peer->p_hdr.info.pi_diamid, peer->p_hdr.info.pi_diamidlen)) {
+				if (fd_os_almostcasesrch(hdr->avp_value->os.data, hdr->avp_value->os.len, 
+							peer->p_hdr.info.pi_diamid, peer->p_hdr.info.pi_diamidlen, NULL)) {
 					TRACE_DEBUG(INFO, "Received a message with Origin-Host set to '%.*s' while expecting '%s'\n", 
 							hdr->avp_value->os.len, hdr->avp_value->os.data, peer->p_hdr.info.pi_diamid);
 					error->pei_errcode = "ER_DIAMETER_AVP_NOT_ALLOWED";
@@ -812,7 +812,7 @@
 	/* Validate the realm if needed */
 	if (peer->p_hdr.info.config.pic_realm) {
 		size_t len = strlen(peer->p_hdr.info.config.pic_realm);
-		if (fd_os_almostcasecmp(peer->p_hdr.info.config.pic_realm, len, peer->p_hdr.info.runtime.pir_realm, peer->p_hdr.info.runtime.pir_realmlen)) {
+		if (fd_os_almostcasesrch(peer->p_hdr.info.config.pic_realm, len, peer->p_hdr.info.runtime.pir_realm, peer->p_hdr.info.runtime.pir_realmlen, NULL)) {
 			TRACE_DEBUG(INFO, "Rejected CER from peer '%s', realm mismatch with configured value (returning DIAMETER_UNKNOWN_PEER).\n", peer->p_hdr.info.pi_diamid);
 			pei.pei_errcode = "DIAMETER_UNKNOWN_PEER"; /* maybe AVP_NOT_ALLOWED would be better fit? */
 			goto error_abort;
--- a/libfdcore/peers.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/libfdcore/peers.c	Wed Mar 02 18:21:59 2011 +0900
@@ -93,7 +93,7 @@
 int fd_peer_add ( struct peer_info * info, char * orig_dbg, void (*cb)(struct peer_info *, void *), void * cb_data )
 {
 	struct fd_peer *p = NULL;
-	struct fd_list * li;
+	struct fd_list * li, *li_inf;
 	int ret = 0;
 	
 	TRACE_ENTRY("%p %p %p %p", info, orig_dbg, cb, cb_data);
@@ -142,16 +142,22 @@
 	
 	/* Ok, now check if we don't already have an entry with the same Diameter Id, and insert this one */
 	CHECK_POSIX( pthread_rwlock_wrlock(&fd_g_peers_rw) );
-	
+	li_inf = &fd_g_peers;
 	for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
 		struct fd_peer * next = (struct fd_peer *)li;
-		int cmp = fd_os_almostcasecmp( p->p_hdr.info.pi_diamid, p->p_hdr.info.pi_diamidlen, 
-						next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen );
-		if (cmp > 0)
-			continue;
-		if (cmp == 0)
+		int cont;
+		int cmp = fd_os_almostcasesrch( p->p_hdr.info.pi_diamid, p->p_hdr.info.pi_diamidlen, 
+						next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen,
+						&cont );
+		if (cmp < 0)
+			li_inf = li;
+		
+		if (cmp == 0) {
 			ret = EEXIST;
-		break;
+			break;
+		}
+		if (!cont)
+			break;
 	}
 	
 	/* We can insert the new peer object */
@@ -161,7 +167,7 @@
 			CHECK_FCT_DO( ret = fd_p_expi_update( p ), break );
 
 			/* Insert the new element in the list */
-			fd_list_insert_before( li, &p->p_hdr.chain );
+			fd_list_insert_after( li_inf, &p->p_hdr.chain );
 		} while (0);
 
 	CHECK_POSIX( pthread_rwlock_unlock(&fd_g_peers_rw) );
@@ -184,19 +190,28 @@
 	
 	/* Search in the list */
 	CHECK_POSIX( pthread_rwlock_rdlock(&fd_g_peers_rw) );
-	for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
-		struct fd_peer * next = (struct fd_peer *)li;
-		int cmp;
-		if (igncase)
-			cmp = fd_os_almostcasecmp( diamid, diamidlen, next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen );
-		else
-			cmp = fd_os_cmp( diamid, diamidlen, next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen );
-		if (cmp > 0)
-			continue;
-		if (cmp == 0)
-			*peer = &next->p_hdr;
-		break;
-	}
+	if (igncase)
+		for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
+			struct fd_peer * next = (struct fd_peer *)li;
+			int cmp, cont;
+			cmp = fd_os_almostcasesrch( diamid, diamidlen, next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen, &cont );
+			if (cmp == 0) {
+				*peer = &next->p_hdr;
+				break;
+			}
+			if (!cont)
+				break;
+		}
+	else
+		for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
+			struct fd_peer * next = (struct fd_peer *)li;
+			int cmp = fd_os_cmp( diamid, diamidlen, next->p_hdr.info.pi_diamid, next->p_hdr.info.pi_diamidlen );
+			if (cmp > 0)
+				continue;
+			if (cmp == 0)
+				*peer = &next->p_hdr;
+			break;
+		}
 	CHECK_POSIX( pthread_rwlock_unlock(&fd_g_peers_rw) );
 	
 	return 0;
@@ -428,7 +443,7 @@
 	struct msg * msg;
 	struct avp *avp_oh;
 	struct avp_hdr * avp_hdr;
-	struct fd_list * li;
+	struct fd_list * li, *li_inf;
 	int found = 0;
 	int ret = 0;
 	struct fd_peer * peer;
@@ -469,15 +484,21 @@
 	 */
 	CHECK_POSIX( pthread_rwlock_wrlock(&fd_g_peers_rw) );
 	
+	li_inf = &fd_g_peers;
 	for (li = fd_g_peers.next; li != &fd_g_peers; li = li->next) {
-		int cmp;
+		int cmp, cont;
 		peer = (struct fd_peer *)li->o;
-		cmp = fd_os_almostcasecmp( avp_hdr->avp_value->os.data, avp_hdr->avp_value->os.len, peer->p_hdr.info.pi_diamid, peer->p_hdr.info.pi_diamidlen );
-		if (cmp > 0)
+		cmp = fd_os_almostcasesrch( avp_hdr->avp_value->os.data, avp_hdr->avp_value->os.len, peer->p_hdr.info.pi_diamid, peer->p_hdr.info.pi_diamidlen, &cont );
+		if (cmp < 0) {
+			li_inf = li;
 			continue;
-		if (cmp == 0)
+		}
+		if (cmp == 0) {
 			found = 1;
-		break;
+			break;
+		}
+		if (!cont)
+			break;
 	}
 	
 	if (!found) {
@@ -499,7 +520,7 @@
 		-- RFC3539 states that this must not be inferior to BRINGDOWN_INTERVAL = 5 minutes */
 		
 		/* Insert the new peer in the list (the PSM will take care of setting the expiry after validation) */
-		fd_list_insert_before( li, &peer->p_hdr.chain );
+		fd_list_insert_after( li_inf, &peer->p_hdr.chain );
 		
 		/* Start the PSM, which will receive the event bellow */
 		CHECK_FCT_DO( ret = fd_psm_begin(peer), goto out );
--- a/libfdcore/routing_dispatch.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/libfdcore/routing_dispatch.c	Wed Mar 02 18:21:59 2011 +0900
@@ -273,11 +273,11 @@
 	    #endif /* 0 */
 		
 		/* In the AVPs, the value comes from the network, so let's be case permissive */
-		if (dh && !fd_os_almostcasecmp(dh->os.data, dh->os.len, c->diamid, c->diamidlen) ) {
+		if (dh && !fd_os_almostcasesrch(dh->os.data, dh->os.len, c->diamid, c->diamidlen, NULL) ) {
 			/* The candidate is the Destination-Host */
 			c->score += FD_SCORE_FINALDEST;
 		} else {
-			if (dr && !fd_os_almostcasecmp(dr->os.data, dr->os.len, c->realm, c->realmlen) ) {
+			if (dr && !fd_os_almostcasesrch(dr->os.data, dr->os.len, c->realm, c->realmlen, NULL) ) {
 				/* The candidate's realm matchs the Destination-Realm */
 				c->score += FD_SCORE_REALM;
 			}
@@ -591,7 +591,7 @@
 							} );
 						ASSERT( ahdr->avp_value );
 						/* Compare the Destination-Host AVP of the message with our identity */
-						if (!fd_os_almostcasecmp(ahdr->avp_value->os.data, ahdr->avp_value->os.len, fd_g_config->cnf_diamid, fd_g_config->cnf_diamid_len)) {
+						if (!fd_os_almostcasesrch(ahdr->avp_value->os.data, ahdr->avp_value->os.len, fd_g_config->cnf_diamid, fd_g_config->cnf_diamid_len, NULL)) {
 							is_dest_host = YES;
 						} else {
 							is_dest_host = NO;
@@ -612,7 +612,7 @@
 						ASSERT( ahdr->avp_value );
 						dr_val = ahdr->avp_value;
 						/* Compare the Destination-Realm AVP of the message with our identity */
-						if (!fd_os_almostcasecmp(dr_val->os.data, dr_val->os.len, fd_g_config->cnf_diamrlm, fd_g_config->cnf_diamrlm_len)) {
+						if (!fd_os_almostcasesrch(dr_val->os.data, dr_val->os.len, fd_g_config->cnf_diamrlm, fd_g_config->cnf_diamrlm_len, NULL)) {
 							is_dest_realm = YES;
 						} else {
 							is_dest_realm = NO;
--- a/libfdproto/ostr.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/libfdproto/ostr.c	Wed Mar 02 18:21:59 2011 +0900
@@ -68,13 +68,22 @@
 	return a;
 }
 
-/* a little less sensitive to case, slower. */
-int fd_os_almostcasecmp_int(uint8_t * os1, size_t os1sz, uint8_t * os2, size_t os2sz)
+/* less sensitive to case, slower. */
+int fd_os_almostcasesrch_int(uint8_t * os1, size_t os1sz, uint8_t * os2, size_t os2sz, int *maybefurther)
 {
 	int i;
+	int res = 0;
+	
 	ASSERT( os1 && os2);
+	if (maybefurther)
+		*maybefurther = 0;
+	
 	if (os1sz < os2sz)
 		return -1;
+	
+	if (maybefurther)
+		*maybefurther = 1;
+	
 	if (os1sz > os2sz)
 		return 1;
 	
@@ -82,10 +91,13 @@
 		if (os1[i] == os2[i])
 			continue;
 		
+		if (!res) 
+			res = os1[i] < os2[i] ? -1 : 1;
+		
 		if (asciitolower(os1[i]) == asciitolower(os2[i])) 
 			continue;
 		
-		return os1[i] < os2[i] ? -1 : 1;
+		return res;
 	}
 	
 	return 0;
--- a/libfdproto/rt_data.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/libfdproto/rt_data.c	Wed Mar 02 18:21:59 2011 +0900
@@ -159,8 +159,8 @@
 	
 	for (li = rtd->candidates.next; li != &rtd->candidates; li = li->next) {
 		struct rtd_candidate * c = (struct rtd_candidate *) li;
-		
-		int cmp = fd_os_almostcasecmp(id, idsz, c->diamid, c->diamidlen);
+		int cont;
+		int cmp = fd_os_almostcasesrch(id, idsz, c->diamid, c->diamidlen, &cont);
 		
 		if (!cmp) {
 			/* Found it! Remove it */
@@ -171,7 +171,7 @@
 			break;
 		}
 		
-		if (cmp > 0)
+		if (cont)
 			continue;
 		
 		/* The list is guaranteed to be ordered only if not extracted */
--- a/tests/testostr.c	Wed Mar 02 16:28:14 2011 +0900
+++ b/tests/testostr.c	Wed Mar 02 18:21:59 2011 +0900
@@ -98,7 +98,7 @@
 		CHECK( 0, fd_os_validate_DiameterIdentity(&res, &len, 1) );
 		CHECK( 0, strcasecmp(res, TEST_IDN_UTF8) );
 		CHECK( 0, fd_os_cmp(res, len, TEST_IDN_UTF8, CONSTSTRLEN(TEST_IDN_UTF8)) );
-		CHECK( 0, fd_os_almostcasecmp(res, len, TEST_IDN_UTF8, CONSTSTRLEN(TEST_IDN_UTF8)) );
+		CHECK( 0, fd_os_almostcasesrch(res, len, TEST_IDN_UTF8, CONSTSTRLEN(TEST_IDN_UTF8), NULL) );
 		CHECK( 0, fd_os_validate_DiameterIdentity(&res, &len, 0) );
 		CHECK( 0, strcasecmp(res, TEST_IDN_UTF8) );
 		CHECK( CONSTSTRLEN(TEST_IDN_UTF8), len );
@@ -127,6 +127,75 @@
 
 	}
 	
+	{
+		/* test fd_os_cmp and fd_os_almostcasesrch and that they are compatible */
+		char *t1 = "a";
+		char *t2 = "b";
+		char *t3 = "C";
+		char *t4 = "d";
+		char *t5 = "aa";
+		char *t6 = "aB";
+		char *t7 = "Ac";
+		char *t8 = "aD";
+		char *t9 = "AAA";
+		
+		char *t5b = "Aa";
+		char *t6b = "ab";
+		
+		/* First, create a list with all the elements in order given by fd_os_cmp */
+		char *t[] = { t1, t2, t3, t4, t5, t6,t7, t8, t9 };
+		int i;
+		struct fd_list *li, l = FD_LIST_INITIALIZER(l);
+		for (i = 0; i < sizeof(t) / sizeof(t[0]); i++) {
+			/* insert t[i] */
+			struct fd_list *n = malloc(sizeof(struct fd_list));
+			CHECK( 1, n ? 1 : 0 );
+			fd_list_init(n, t[i]);
+			for (li = l.next; li != &l; li = li->next) {
+				if ( fd_os_cmp(t[i], strlen(t[i]), li->o, strlen(li->o)) < 0 )
+					break;
+			} 
+			fd_list_insert_before(li, n);
+		}
+		/* in principle the result is: [ "C", "a", "b", "d", "Ac", "aB", "aD", "aa", "AAA" ] */
+		
+		/* Since there is no equal value in the list (even case-insensitive), check that the order is valid also for the caseinsensitive variant */
+		for (li = l.next; li != l.prev; li = li->next) {
+			CHECK( 1, fd_os_almostcasesrch(li->o, strlen(li->o), li->next->o, strlen(li->next->o), NULL) < 0 ? 1 : 0 );
+		}
+		
+		/* Now check that we can case-insentively find t5b and t6b to be equal to t5 and t6 resp. (this is how we use it in the daemon) */
+		for (li = l.next; li != &l; li = li->next) {
+			int cont, cmp;
+			cmp = fd_os_almostcasesrch(t5b, strlen(t5b), li->o, strlen(li->o), &cont);
+			TRACE_DEBUG(FULL, "Comp '%s' : %d, %d", (char *)li->o, cmp, cont);
+			if (cmp == 0)
+				break;
+			if (!cont)
+				break;
+		}
+		CHECK( li->o, t5 );
+		
+		for (li = l.next; li != &l; li = li->next) {
+			int cont, cmp;
+			cmp = fd_os_almostcasesrch(t6b, strlen(t6b), li->o, strlen(li->o), &cont);
+			TRACE_DEBUG(FULL, "Comp '%s' : %d, %d", (char *)li->o, cmp, cont);
+			if (cmp == 0)
+				break;
+			if (!cont)
+				break;
+		}
+		CHECK( li->o, t6 );
+		
+		
+		/* done */
+		while (!FD_IS_LIST_EMPTY(&l)) {
+			li = l.next;
+			fd_list_unlink(li);
+			free(li);
+		}
+	}
+	
 	/* That's all for the tests yet */
 	PASSTEST();
 }
"Welcome to our mercurial repository"