Changeset 738:d666051658bd in freeDiameter

Timestamp:

Mar 2, 2011, 6:21:59 PM (13 years ago)

Author:

Sebastien Decugis <sdecugis@nict.go.jp>

Branch:

default

Phase:

public

Message:

Fix broken 'almostcasecmp' logic

Files:

• extensions/app_radgw/rgw_clients.c (modified) (1 diff)
• extensions/app_redirect/ard_rules.c (modified) (1 diff)
• extensions/rt_redirect/redir_fwd.c (modified) (1 diff)
• extensions/rt_redirect/redir_out.c (modified) (2 diffs)
• include/freeDiameter/libfdproto.h (modified) (1 diff)
• libfdcore/p_ce.c (modified) (2 diffs)
• libfdcore/peers.c (modified) (7 diffs)
• libfdcore/routing_dispatch.c (modified) (3 diffs)
• libfdproto/ostr.c (modified) (2 diffs)
• libfdproto/rt_data.c (modified) (2 diffs)
• tests/testostr.c (modified) (2 diffs)

extensions/app_radgw/rgw_clients.c

@@ -732 +732 @@
                 
                 /* 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;

extensions/app_redirect/ard_rules.c

@@ -94 +94 @@
         
         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 {

extensions/rt_redirect/redir_fwd.c

@@ -150 +150 @@
                                                 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;
                                                 }

extensions/rt_redirect/redir_out.c

@@ -171 +171 @@
                 /* 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);
@@ -198 +198 @@
                 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 );
-                        
-                        if (cmp < 0)
-                                break; /* targets are ordered */
+                        int cont;
+                        
+                        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;
                 }
         }

include/freeDiameter/libfdproto.h

@@ -621 +621 @@
 
 /* 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 !!!
- */
-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)
+ 
+ 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_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. 

libfdcore/p_ce.c

@@ -304 +304 @@
                                 
                                 /* 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);
@@ -813 +813 @@
         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? */

libfdcore/peers.c

@@ -94 +94 @@
 {
         struct fd_peer *p = NULL;
-        struct fd_list * li;
+        struct fd_list * li, *li_inf;
         int ret = 0;
         
@@ -143 +143 @@
         /* 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;
         }
         
@@ -162 +168 @@
 
                         /* 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);
 
@@ -185 +191 @@
         /* 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) );
         
@@ -429 +444 @@
         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;
@@ -470 +485 @@
         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;
         }
         
@@ -500 +521 @@
                 
                 /* 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 */

libfdcore/routing_dispatch.c

@@ -274 +274 @@
                 
                 /* 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;
@@ -592 +592 @@
                                                 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 {
@@ -613 +613 @@
                                                 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 {

libfdproto/ostr.c

@@ -69 +69 @@
 }
 
-/* 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;
@@ -83 +92 @@
                         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;
         }
         

libfdproto/rt_data.c

@@ -160 +160 @@
         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) {
@@ -172 +172 @@
                 }
                 
-                if (cmp > 0)
+                if (cont)
                         continue;
                 

tests/testostr.c

@@ -99 +99 @@
                 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) );
@@ -128 +128 @@
         }
         
+        {
+                /* 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();