From 7545eb11f040923bde9eff55f8ff697c1dc3e525 Mon Sep 17 00:00:00 2001 From: Raphael Coeffic Date: Thu, 24 Mar 2011 20:11:24 +0100 Subject: [PATCH] b/f: several minor bugs in session creation. - the session pointer should not be touched after the session has been started (could be immediately destroyed). - return 481 on CANCEL if the transaction cannot be found. - reply properly with 482 if the UAS dialog already exists. --- apps/dsm/mods/mod_dlg/ModDlg.cpp | 16 ++-- apps/examples/call_gen/CallGen.cpp | 2 +- apps/examples/di_dialer/DIDial.cpp | 24 ++--- apps/ivr/IvrUAC.cpp | 1 - apps/webconference/WebConference.cpp | 13 ++- core/AmSessionContainer.cpp | 135 ++++++++++++++++----------- core/AmSessionContainer.h | 22 +++-- core/AmSipDialog.cpp | 20 +++- core/AmSipDispatcher.cpp | 23 +++-- core/AmSipHeaders.h | 3 + core/AmUAC.cpp | 18 ++-- core/AmUAC.h | 18 ++-- 12 files changed, 173 insertions(+), 122 deletions(-) diff --git a/apps/dsm/mods/mod_dlg/ModDlg.cpp b/apps/dsm/mods/mod_dlg/ModDlg.cpp index 97990b27..9eace5fd 100644 --- a/apps/dsm/mods/mod_dlg/ModDlg.cpp +++ b/apps/dsm/mods/mod_dlg/ModDlg.cpp @@ -282,13 +282,13 @@ EXEC_ACTION_START(DLGDialoutAction) { DBG("sess_params: '%s'\n", AmArg::print(*sess_params).c_str()); - AmSession* new_sess = AmUAC::dialout(user, app_name, r_uri, from, from_uri, to, ltag, hdrs, sess_params); - - if (NULL != new_sess) { - sc_sess->var[arrayname + "_ltag"] = new_sess->getLocalTag(); - } else { - sc_sess->var[arrayname + "_ltag"] = ""; - sc_sess->SET_ERRNO(DSM_ERRNO_GENERAL); - } + string new_sess_tag = AmUAC::dialout(user, app_name, r_uri, from, from_uri, to, ltag, hdrs, sess_params); + + if (!new_sess_tag.empty()) { + sc_sess->var[arrayname + "_ltag"] = new_sess_tag; + } else { + sc_sess->var[arrayname + "_ltag"] = ""; + sc_sess->SET_ERRNO(DSM_ERRNO_GENERAL); + } } EXEC_ACTION_END; diff --git a/apps/examples/call_gen/CallGen.cpp b/apps/examples/call_gen/CallGen.cpp index 7eb088a4..aca140e9 100644 --- a/apps/examples/call_gen/CallGen.cpp +++ b/apps/examples/call_gen/CallGen.cpp @@ -255,7 +255,7 @@ void CallGenFactory::createCall(const AmArg& args) { AmArg* c_args = new AmArg(args); DBG("placing new call to %s\n", call_ruri.c_str()); - /* AmSession* s = */ AmUAC::dialout("callgen", // user + /* string tag = */ AmUAC::dialout("callgen", // user APP_NAME, call_ruri, "<" + from + ">", from, diff --git a/apps/examples/di_dialer/DIDial.cpp b/apps/examples/di_dialer/DIDial.cpp index 5af6ff31..2b2bd020 100644 --- a/apps/examples/di_dialer/DIDial.cpp +++ b/apps/examples/di_dialer/DIDial.cpp @@ -132,11 +132,11 @@ string DIDial::dialout(const string& application, DBG("dialout application '%s', user '%s', from '%s', to '%s'\n", application.c_str(), user.c_str(), from.c_str(), to.c_str()); - AmSession* s = AmUAC::dialout(user.c_str(), application, to, + string tag = AmUAC::dialout(user.c_str(), application, to, "<" + from + ">", from, "<" + to + ">", string(""), string(""), extra_params); - if (s) - return s->getLocalTag(); + if (!tag.empty()) + return tag; else return ""; } @@ -162,15 +162,15 @@ string DIDial::dialout_auth(const string& application, } else { a->setBorrowedPointer(new UACAuthCred(a_realm, a_user, a_pwd)); } - AmSession* s = AmUAC::dialout(user.c_str(), application, to, + string tag = AmUAC::dialout(user.c_str(), application, to, "<" + from + ">", from, "<" + to + ">", string(""), // callid string(""), // xtra hdrs a); delete a; - if (s) - return s->getLocalTag(); + if (!tag.empty()) + return tag; else return "\n"; } @@ -195,14 +195,14 @@ string DIDial::dialout_auth_b2b(const string& application, a.push(a_pwd.c_str()); a.push(callee_ruri.c_str()); - AmSession* s = AmUAC::dialout( + string tag = AmUAC::dialout( announcement.c_str(), application, caller_ruri, "<" + from + ">", from, "<" + to + ">", string(""), // callid string(""), //xtra hdrs &a); - if (s) - return s->getLocalTag(); + if (!tag.empty()) + return tag; else return "\n"; } @@ -223,7 +223,7 @@ string DIDial::dialout_pin(const string& application, it->second.user, it->second.pwd)); - AmSession* s = AmUAC::dialout(user.c_str(), application, + string tag = AmUAC::dialout(user.c_str(), application, "sip:"+to_user+"@"+it->second.realm, "second.user+"@"+it->second.realm + ">", "sip:"+it->second.user+"@"+it->second.realm, @@ -231,8 +231,8 @@ string DIDial::dialout_pin(const string& application, string(""), // callid string(""), // xtra hdrs a); - if (s) - return s->getLocalTag(); + if (!tag.empty()) + return tag; else return "\n"; } else diff --git a/apps/ivr/IvrUAC.cpp b/apps/ivr/IvrUAC.cpp index da2b4fa8..658e96c5 100644 --- a/apps/ivr/IvrUAC.cpp +++ b/apps/ivr/IvrUAC.cpp @@ -49,7 +49,6 @@ static PyObject* IvrUAC_dialout(IvrUAC* self, PyObject* args) &from, &from_uri, &to)) return NULL; - //AmSession* newSession = AmUAC::dialout(user, app_name, r_uri, from, from_uri, to); diff --git a/apps/webconference/WebConference.cpp b/apps/webconference/WebConference.cpp index ba6ccd9a..409f5d12 100644 --- a/apps/webconference/WebConference.cpp +++ b/apps/webconference/WebConference.cpp @@ -731,13 +731,12 @@ void WebConferenceFactory::dialout(const AmArg& args, AmArg& ret) { AmArg* a = new AmArg(); a->setBorrowedPointer(new UACAuthCred("", auth_user, auth_pwd)); - AmSession* s = AmUAC::dialout(room.c_str(), APP_NAME, to, - "<" + from + ">", from, "<" + to + ">", - string(""), // callid - headers, // headers - a); - if (s) { - string localtag = s->getLocalTag(); + string localtag = AmUAC::dialout(room.c_str(), APP_NAME, to, + "<" + from + ">", from, "<" + to + ">", + string(""), // callid + headers, // headers + a); + if (!localtag.empty()) { ret.push(0); ret.push("OK"); ret.push(localtag.c_str()); diff --git a/core/AmSessionContainer.cpp b/core/AmSessionContainer.cpp index 4fb2e198..bff01ed9 100644 --- a/core/AmSessionContainer.cpp +++ b/core/AmSessionContainer.cpp @@ -205,23 +205,38 @@ void AmSessionContainer::destroySession(AmSession* s) } } -AmSession* AmSessionContainer::startSessionUAC(AmSipRequest& req, AmArg* session_params) { +string AmSessionContainer::startSessionUAC(AmSipRequest& req, AmArg* session_params) { - AmSession* session = NULL; + auto_ptr session; try { - if((session = createSession(req, session_params)) != 0) { + session.reset(createSession(req, session_params)); + if(session.get() != 0) { + session->dlg.updateStatusFromLocalRequest(req); session->setCallgroup(req.from_tag); session->setNegotiateOnReply(true); - if (!addSession("","",req.from_tag,session)) { + switch(addSession(req.from_tag,session.get())) { + + case AmSessionContainer::Inserted: + // successful case + break; + + case AmSessionContainer::ShutDown: + throw AmSession::Exception(AmConfig::ShutdownModeErrCode, + AmConfig::ShutdownModeErrReason); + + case AmSessionContainer::AlreadyExist: + throw AmSession::Exception(482, + SIP_REPLY_LOOP_DETECTED); + + default: ERROR("adding session to session container\n"); - delete session; - return NULL; + throw string(SIP_REPLY_SERVER_INTERNAL_ERROR); } - - MONITORING_LOG5(session->getLocalTag().c_str(), + + MONITORING_LOG5(req.from_tag, "app", req.cmd.c_str(), "dir", "out", "from", req.from.c_str(), @@ -229,58 +244,49 @@ AmSession* AmSessionContainer::startSessionUAC(AmSipRequest& req, AmArg* session "ruri", req.r_uri.c_str()); if (int err = session->sendInvite(req.hdrs)) { - ERROR("INVITE could not be sent: error code = %d.\n", - err); - AmEventDispatcher::instance()-> - delEventQueue(session->getLocalTag(), - session->getCallID(), - session->getRemoteTag()); - MONITORING_MARK_FINISHED(session->getLocalTag().c_str()); - delete session; + ERROR("INVITE could not be sent: error code = %d.\n", err); + AmEventDispatcher::instance()->delEventQueue(req.from_tag); + MONITORING_MARK_FINISHED(req.from_tag.c_str()); return NULL; } - + if (AmConfig::LogSessions) { INFO("Starting UAC session %s app %s\n", - session->getLocalTag().c_str(), req.cmd.c_str()); + req.from_tag.c_str(), req.cmd.c_str()); } - try { session->start(); - } catch (const string& err) { - AmEventDispatcher::instance()-> - delEventQueue(session->getLocalTag(), - session->getCallID(), - session->getRemoteTag()); - - delete session; + } catch (...) { + AmEventDispatcher::instance()->delEventQueue(req.from_tag); throw; } - } } catch(const AmSession::Exception& e){ ERROR("%i %s\n",e.code,e.reason.c_str()); - AmSipDialog::reply_error(req,e.code,e.reason); + return ""; } catch(const string& err){ ERROR("startSession: %s\n",err.c_str()); - AmSipDialog::reply_error(req,500,err); + return ""; } catch(...){ ERROR("unexpected exception\n"); - AmSipDialog::reply_error(req,500,"unexpected exception"); + return ""; } - return session; + session.release(); + return req.from_tag; } void AmSessionContainer::startSessionUAS(AmSipRequest& req) { try { // Call-ID and From-Tag are unknown: it's a new session - AmSession* session; - if((session = createSession(req)) != 0){ + auto_ptr session; + + session.reset(createSession(req)); + if(session.get() != 0){ // update session's local tag (ID) if not already set session->setLocalTag(); @@ -293,9 +299,23 @@ void AmSessionContainer::startSessionUAS(AmSipRequest& req) local_tag.c_str(), req.cmd.c_str()); } - if (!addSession(req.callid,req.from_tag,local_tag,session)) { + switch(addSession(req.callid,req.from_tag,local_tag, + session.get())) { + + case AmSessionContainer::Inserted: + // successful case + break; + + case AmSessionContainer::ShutDown: + throw AmSession::Exception(AmConfig::ShutdownModeErrCode, + AmConfig::ShutdownModeErrReason); + + case AmSessionContainer::AlreadyExist: + throw AmSession::Exception(482, + SIP_REPLY_LOOP_DETECTED); + + default: ERROR("adding session to session container\n"); - delete session; throw string(SIP_REPLY_SERVER_INTERNAL_ERROR); } @@ -308,17 +328,14 @@ void AmSessionContainer::startSessionUAS(AmSipRequest& req) try { session->start(); - } catch (const string& err) { + } catch (...) { AmEventDispatcher::instance()-> - delEventQueue(session->getLocalTag(), - session->getCallID(), - session->getRemoteTag()); - - delete session; + delEventQueue(req.callid,req.from_tag,local_tag); throw; } session->postEvent(new AmSipRequestEvent(req)); + session.release(); } } catch(const AmSession::Exception& e){ @@ -425,27 +442,37 @@ AmSession* AmSessionContainer::createSession(AmSipRequest& req, return session; } -bool AmSessionContainer::addSession(const string& callid, - const string& remote_tag, - const string& local_tag, - AmSession* session) +AmSessionContainer::AddSessionStatus +AmSessionContainer::addSession(const string& callid, + const string& remote_tag, + const string& local_tag, + AmSession* session) { if(_container_closed.get()) - return false; + return ShutDown; - return AmEventDispatcher::instance()-> - addEventQueue(local_tag,(AmEventQueue*)session, - callid,remote_tag); + if(AmEventDispatcher::instance()-> + addEventQueue(local_tag,(AmEventQueue*)session, + callid,remote_tag)) { + return Inserted; + } + + return AlreadyExist; } -bool AmSessionContainer::addSession(const string& local_tag, - AmSession* session) +AmSessionContainer::AddSessionStatus +AmSessionContainer::addSession(const string& local_tag, + AmSession* session) { if(_container_closed.get()) - return false; + return ShutDown; + + if(AmEventDispatcher::instance()-> + addEventQueue(local_tag,(AmEventQueue*)session)){ + return Inserted; + } - return AmEventDispatcher::instance()-> - addEventQueue(local_tag,(AmEventQueue*)session); + return AlreadyExist; } void AmSessionContainer::enableUncleanShutdown() { diff --git a/core/AmSessionContainer.h b/core/AmSessionContainer.h index 82ee08c1..6f95f9c4 100644 --- a/core/AmSessionContainer.h +++ b/core/AmSessionContainer.h @@ -87,6 +87,12 @@ class AmSessionContainer : public AmThread static void dispose(); + enum AddSessionStatus { + ShutDown, + Inserted, + AlreadyExist + }; + /** * Creates a new session. * @param req local request @@ -99,17 +105,17 @@ class AmSessionContainer : public AmThread * Adds a session to the container (UAS only). * @return true if the session is new within the container. */ - bool addSession(const string& callid, - const string& remote_tag, - const string& local_tag, - AmSession* session); + AddSessionStatus addSession(const string& callid, + const string& remote_tag, + const string& local_tag, + AmSession* session); /** * Adds a session to the container. * @return true if the session is new within the container. */ - bool addSession(const string& local_tag, - AmSession* session); + AddSessionStatus addSession(const string& local_tag, + AmSession* session); /** * Constructs a new session and adds it to the active session container. @@ -121,8 +127,8 @@ class AmSessionContainer : public AmThread * Constructs a new session and adds it to the active session container. * @param req client's request */ - AmSession* startSessionUAC(AmSipRequest& req, - AmArg* session_params = NULL); + string startSessionUAC(AmSipRequest& req, + AmArg* session_params = NULL); /** * Detroys a session. diff --git a/core/AmSipDialog.cpp b/core/AmSipDialog.cpp index fa73dc1a..c868f19f 100644 --- a/core/AmSipDialog.cpp +++ b/core/AmSipDialog.cpp @@ -87,7 +87,20 @@ void AmSipDialog::updateStatus(const AmSipRequest& req) { DBG("AmSipDialog::updateStatus(req = %s)\n", req.method.c_str()); - if ((req.method == "ACK") || (req.method == "CANCEL")) { + if (req.method == "ACK") { + if(hdl) + hdl->onSipRequest(req); + return; + } + + if (req.method == "CANCEL") { + + TransMap::iterator t_it = uas_trans.find(req.cseq); + if(t_it == uas_trans.end()){ + reply_error(req,481,SIP_REPLY_NOT_EXIST); + return; + } + if(hdl) hdl->onSipRequest(req); return; @@ -236,8 +249,9 @@ int AmSipDialog::updateStatusReply(const AmSipRequest& req, unsigned int code) TransMap::iterator t_it = uas_trans.find(req.cseq); if(t_it == uas_trans.end()){ ERROR("could not find any transaction matching request\n"); - ERROR("method=%s; callid=%s; local_tag=%s; remote_tag=%s; cseq=%i\n", - req.method.c_str(),callid.c_str(),local_tag.c_str(), + ERROR("reply code=%i; method=%s; callid=%s; local_tag=%s; " + "remote_tag=%s; cseq=%i\n", + code,req.method.c_str(),callid.c_str(),local_tag.c_str(), remote_tag.c_str(),req.cseq); return -1; } diff --git a/core/AmSipDispatcher.cpp b/core/AmSipDispatcher.cpp index ac8dc857..f0283a06 100644 --- a/core/AmSipDispatcher.cpp +++ b/core/AmSipDispatcher.cpp @@ -85,22 +85,25 @@ void AmSipDispatcher::handleSipMsg(AmSipRequest &req) return; } - if(ev_disp->postSipRequest(callid, remote_tag, req)){ - - return; - } - DBG("method: `%s' [%zd].\n", req.method.c_str(), req.method.length()); if(req.method == "INVITE"){ AmSessionContainer::instance()->startSessionUAS(req); } - else if( (req.method == "CANCEL") || - (req.method == "BYE") ){ + else if(req.method == "CANCEL"){ - // CANCEL/BYE of a (here) non-existing dialog - AmSipDialog::reply_error(req,481, - "Call leg/Transaction does not exist"); + if(ev_disp->postSipRequest(callid, remote_tag, req)){ + return; + } + + // CANCEL of a (here) non-existing dialog + AmSipDialog::reply_error(req,481,SIP_REPLY_NOT_EXIST); + return; + } + else if(req.method == "BYE"){ + + // BYE of a (here) non-existing dialog + AmSipDialog::reply_error(req,481,SIP_REPLY_NOT_EXIST); return; } else { diff --git a/core/AmSipHeaders.h b/core/AmSipHeaders.h index cb8543cc..fea90af3 100644 --- a/core/AmSipHeaders.h +++ b/core/AmSipHeaders.h @@ -53,4 +53,7 @@ #define SIP_REPLY_SERVER_INTERNAL_ERROR "Server Internal Error" #define SIP_REPLY_BAD_EXTENSION "Bad Extension" #define SIP_REPLY_EXTENSION_REQUIRED "Extension Required" +#define SIP_REPLY_LOOP_DETECTED "Loop Detected" +#define SIP_REPLY_NOT_EXIST "Call Leg/Transaction Does Not Exist" + #endif /* __AMSIPHEADERS_H__ */ diff --git a/core/AmUAC.cpp b/core/AmUAC.cpp index 534418de..41362d40 100644 --- a/core/AmUAC.cpp +++ b/core/AmUAC.cpp @@ -31,15 +31,15 @@ #include "AmSessionContainer.h" #include "AmConfig.h" -AmSession* AmUAC::dialout(const string& user, - const string& app_name, - const string& r_uri, - const string& from, - const string& from_uri, - const string& to, - const string& local_tag, - const string& hdrs, - AmArg* session_params) { +string AmUAC::dialout(const string& user, + const string& app_name, + const string& r_uri, + const string& from, + const string& from_uri, + const string& to, + const string& local_tag, + const string& hdrs, + AmArg* session_params) { AmSipRequest req; diff --git a/core/AmUAC.h b/core/AmUAC.h index ce1f7ddc..a1d4ae72 100644 --- a/core/AmUAC.h +++ b/core/AmUAC.h @@ -38,15 +38,15 @@ using std::string; /** \brief API for UAC support */ class AmUAC { public: - static AmSession* dialout(const string& user, - const string& app_name, - const string& r_uri, - const string& from, - const string& from_uri, - const string& to, - const string& local_tag = "", - const string& hdrs = "", - AmArg* session_params = NULL); + static string dialout(const string& user, + const string& app_name, + const string& r_uri, + const string& from, + const string& from_uri, + const string& to, + const string& local_tag = "", + const string& hdrs = "", + AmArg* session_params = NULL); };