Module: sip-router Branch: master Commit: 8632598cac189adb557ad0c65f15744ef4256e45 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=8632598… Author: Andrei Pelinescu-Onciul &lt;andrei(a)iptel.org&gt; Committer: Andrei Pelinescu-Onciul &lt;andrei(a)iptel.org&gt; Date: Wed Jun 24 20:01:46 2009 +0200 tm: safety checks for possible escaped neg. ACKs In normal operation looking up a transaction corresponding to an ACK to a neg. reply or to a local transaction should end up in script termination, so when t_relay_to() is called for a neg. ACK, the transaction should not have been looked up previously. If this assumption fails, the ACK will be processed normally (resulting at worst in calling the TMCB_ACK_NEG_IN callback multiple times for the same ACK) and a warning message will be logged. --- modules/tm/t_funcs.c | 28 +++++++++++++++++++++++----- 1 files changed, 23 insertions(+), 5 deletions(-) diff --git a/modules/tm/t_funcs.c b/modules/tm/t_funcs.c index d60c99e..aa7d1f4 100644 --- a/modules/tm/t_funcs.c +++ b/modules/tm/t_funcs.c @@ -271,11 +271,10 @@ int t_relay_to( struct sip_msg *p_msg , struct proxy_l *proxy, int proto, /* parsing error, memory alloc, whatever ... if via is bad and we are forced to reply there, return with 0 (->break), pass error status otherwise - - MMA: return value E_SCRIPT means that transaction was already started from the script - so continue with that transaction + MMA: return value E_SCRIPT means that transaction was already started + from the script so continue with that transaction */ - if (new_tran!=E_SCRIPT) { + if (likely(new_tran!=E_SCRIPT)) { if (new_tran<0) { ret = (ser_error==E_BAD_VIA && reply_to_via) ? 0 : new_tran; goto done; @@ -285,11 +284,30 @@ int t_relay_to( struct sip_msg *p_msg , struct proxy_l *proxy, int proto, ret = 1; goto done; } + }else if (unlikely(p_msg->REQ_METHOD==METHOD_ACK)) { + /* transaction previously found (E_SCRIPT) and msg==ACK + => ack to neg. reply or ack to local trans. + => process it and exit */ + /* FIXME: there's no way to distinguish here between acks to + local trans. and neg. acks */ + /* in normal operation we should never reach this point, if we + do WARN(), it might hide some real bug (apart from possibly + hiding a bug the most harm done is calling the TMCB_ACK_NEG + callbacks twice) */ + WARN("negative or local ACK caught, please report\n"); + t=get_t(); + if (unlikely(has_tran_tmcbs(t, TMCB_ACK_NEG_IN))) + run_trans_callbacks(TMCB_ACK_NEG_IN, t, p_msg, 0, + p_msg->REQ_METHOD); + t_release_transaction(t); + ret=1; + goto done; } /* new transaction */ - /* ACKs do not establish a transaction and are fwd-ed statelessly */ + /* at this point if the msg is an ACK it is an e2e ACK and + e2e ACKs do not establish a transaction and are fwd-ed statelessly */ if ( p_msg->REQ_METHOD==METHOD_ACK) { DBG( "SER: forwarding ACK statelessly \n"); if (proxy==0) {