# HG changeset patch # User Sebastien Decugis # Date 1299057719 -32400 # Node ID d666051658bd9ea2a2ca0791d8af4dcabe8e7b03 # Parent 03bcd4870f4441dfa10a0cdea14e7fbc0f9f8c85 Fix broken 'almostcasecmp' logic diff -r 03bcd4870f44 -r d666051658bd extensions/app_radgw/rgw_clients.c --- 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 { diff -r 03bcd4870f44 -r d666051658bd extensions/app_redirect/ard_rules.c --- 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; diff -r 03bcd4870f44 -r d666051658bd extensions/rt_redirect/redir_fwd.c --- 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); diff -r 03bcd4870f44 -r d666051658bd extensions/rt_redirect/redir_out.c --- 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; } } diff -r 03bcd4870f44 -r d666051658bd include/freeDiameter/libfdproto.h --- 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. diff -r 03bcd4870f44 -r d666051658bd libfdcore/p_ce.c --- 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; diff -r 03bcd4870f44 -r d666051658bd libfdcore/peers.c --- 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 ); diff -r 03bcd4870f44 -r d666051658bd libfdcore/routing_dispatch.c --- 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; diff -r 03bcd4870f44 -r d666051658bd libfdproto/ostr.c --- 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; diff -r 03bcd4870f44 -r d666051658bd libfdproto/rt_data.c --- 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 */ diff -r 03bcd4870f44 -r d666051658bd tests/testostr.c --- 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(); }