Module: sip-router Branch: sr_3.0 Commit: 1bf989c36a9972389c033b87df0e88c04e56b9c3 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=1bf989c3...
Author: Jan Janak jan@ryngle.com Committer: Jan Janak jan@ryngle.com Date: Mon Oct 26 16:25:02 2009 +0100
registrar: Fix handling of cases where contacts > max_contacts
Registrar should not report an error to syslog if a user exceeds the maximum number of allowed contacts per user. The registrar module sends a reply back with a description of what happened so there is no reason to write this to syslog, max_contacts > 0 is a configuration choice rather than an error.
This patch also changes all affected functions in registrar module to indicate that the maximum number of allowed contacts was exceeded by returning a positive number, instead of a negative number to indicate that an error occurred.
---
modules_s/registrar/save.c | 38 +++++++++++++++++++++----------------- 1 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/modules_s/registrar/save.c b/modules_s/registrar/save.c index 27d134c..fee4548 100644 --- a/modules_s/registrar/save.c +++ b/modules_s/registrar/save.c @@ -257,10 +257,11 @@ static int create_rcv_uri(str** uri, struct sip_msg* m)
/* - * Message contained some contacts, but record with same address - * of record was not found so we have to create a new record - * and insert all contacts from the message that have expires - * > 0 + * Message contained some contacts, but record with same address of record was + * not found so we have to create a new record and insert all contacts from + * the message that have expires > 0. The function returns a negative number + * on error, a positive number if the number of contacts would exceed + * max_contacts and 0 on success. */ static inline int insert(struct sip_msg* _m, str* aor, contact_t* _c, udomain_t* _d, str* _u, str *ua, str* aor_filter, int sid) { @@ -289,7 +290,7 @@ static inline int insert(struct sip_msg* _m, str* aor, contact_t* _c, udomain_t* if (max_contacts && (num >= max_contacts)) { rerrno = R_TOO_MANY; ul.delete_urecord(_d, _u); - return -1; + return 1; } num++; @@ -394,7 +395,7 @@ static int test_max_contacts(struct sip_msg* _m, urecord_t* _r, contact_t* _c) _c = get_next_contact(_c); } - DBG("test_max_contacts: %d contacts after commit\n", num); + DBG("test_max_contacts: %d contacts after commit, max_contacts=%d\n", num, max_contacts); if (num > max_contacts) { rerrno = R_TOO_MANY; return 1; @@ -414,6 +415,11 @@ static int test_max_contacts(struct sip_msg* _m, urecord_t* _r, contact_t* _c) * > 0, update the contact * 3) If contact in usrloc exists and expires * == 0, delete contact + * + * The function returns a negative number on error, a positive number if + * max_contacts is set and the number of contacts after the change would + * exceed that maximum number of allowed contacts. On success the function + * returns 0. */ static inline int update(struct sip_msg* _m, urecord_t* _r, str* aor, contact_t* _c, str* _ua, str* aor_filter, int sid) { @@ -434,8 +440,12 @@ static inline int update(struct sip_msg* _m, urecord_t* _r, str* aor, contact_t* if (max_contacts) { ret = test_max_contacts(_m, _r, _c); if (ret != 0) { + /* test_max_contacts returns a negative number on error and a + * positive number if the number of contacts after the update + * exceeds the configured max_contacts. In both cases we return + * here. */ build_contact(_r->contacts, aor_filter); - return -1; + return ret; } }
@@ -607,24 +617,18 @@ static inline int contacts(struct sip_msg* _m, contact_t* _c, udomain_t* _d, str }
if (res == 0) { /* Contacts found */ - if (update(_m, r, aor, _c, _ua, aor_filter, sid) < 0) { + if ((res = update(_m, r, aor, _c, _ua, aor_filter, sid) < 0)) { LOG(L_ERR, "contacts(): Error while updating record\n"); - build_contact(r->contacts, aor_filter); - ul.release_urecord(r); - ul.unlock_udomain(_d); - return -3; } build_contact(r->contacts, aor_filter); ul.release_urecord(r); } else { - if (insert(_m, aor, _c, _d, _u, _ua, aor_filter, sid) < 0) { + if ((res = insert(_m, aor, _c, _d, _u, _ua, aor_filter, sid) < 0)) { LOG(L_ERR, "contacts(): Error while inserting record\n"); - ul.unlock_udomain(_d); - return -4; } } ul.unlock_udomain(_d); - return 0; + return res; }
#define UA_DUMMY_STR "Unknown" @@ -684,7 +688,7 @@ static inline int save_real(struct sip_msg* _m, udomain_t* _t, char* aor_filt, i if (no_contacts(_t, &uid, &aor_filter) < 0) goto error; } } else { - if (contacts(_m, c, _t, &uid, &ua, &aor_filter) < 0) goto error; + if (contacts(_m, c, _t, &uid, &ua, &aor_filter) != 0) goto error; }
if (doreply) {
jan,
i once suggested to k folks that k should report an error only when the error is caused by the proxy itself, not by anything user can do. the idea was not accepted.
-- juha
Juha,
On Mon, Oct 26, 2009 at 4:17 PM, Juha Heinanen jh@tutpro.com wrote:
jan,
i once suggested to k folks that k should report an error only when the error is caused by the proxy itself, not by anything user can do. the idea was not accepted.
We may need to revisit how we log messages into syslog in sip-router. I remember we already discussed this (with Andrei and Daniel iirc), but decided not to do it because we were just starting to merge modules and it would have been too many changes at once.
Ondrej, a colleague of mine, already worked on this and prepared scripts (sip-router/scripts/logging) and some documentation (sip-router/doc/logging-api.txt) which can be used to unify log messages.
We may also need to change the log level of some messages, for example, lots of memory allocation errors are now logged on L_ERR, together with many less-critical error messages, such as parsing errors. This makes it difficult to mute some of them unless you change the code.
Another nice improvement would be log-level configurable per-module, so that you can selectively enable/disable more logging for individual modules when you are debugging them. The sip-router core has the config framework which could be used to implement this efficiently.
In my previous commits I changed those error messages that generate too much traffic in syslog by default. We run sip-router on iptel.org, that's a public SIP service and we cannot control what user agents people use. Lots of them are broken and generate bogus SIP messages which then trigger syslog errors.
-- Jan
-
Jan Janak -
jh＠tutpro.com