24 Jun
2009
24 Jun
'09
2:07 p.m.
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 <andrei(a)iptel.org>
Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org>
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) {