Author: rmudgett Date: Fri Mar 13 11:26:38 2015 New Revision: 432892 URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=432892 Log: chan_pjsip/res_pjsip_callerid: Make Party ID handling simpler and consistent.
The res_pjsip modules were manually checking both name and number presentation values when there is a function that determines the combined presentation for a party ID struct. The function takes into account if the name or number components are valid while the manual code rarely checked if the data was even valid. * Made use ast_party_id_presentation() rather than manually checking party ID presentation values. * Ensure that set_id_from_pai() and set_id_from_rpid() will not return presentation values other than what is pulled out of the SIP headers. It is best if the code doesn't assume that AST_PRES_ALLOWED and AST_PRES_USER_NUMBER_UNSCREENED are zero. * Fixed copy paste error in add_privacy_params() dealing with RPID privacy. * Pulled the id->number.valid test from add_privacy_header() and add_privacy_params() up into the parent function add_id_headers() to skip adding PAI/RPID headers earlier. * Made update_connected_line_information() not send out connected line updates if the connected line number is invalid. Lower level code would not add the party ID information and thus the sent message would be unnecessary. * Eliminated RAII_VAR usage in send_direct_media_request(). Review: https://reviewboard.asterisk.org/r/4472/ Modified: branches/13/channels/chan_pjsip.c branches/13/res/res_pjsip_caller_id.c Modified: branches/13/channels/chan_pjsip.c URL: http://svnview.digium.com/svn/asterisk/branches/13/channels/chan_pjsip.c?view=diff&rev=432892&r1=432891&r2=432892 ============================================================================== --- branches/13/channels/chan_pjsip.c (original) +++ branches/13/channels/chan_pjsip.c Fri Mar 13 11:26:38 2015 @@ -219,10 +219,13 @@ static int send_direct_media_request(void *data) { - RAII_VAR(struct ast_sip_session *, session, data, ao2_cleanup); - - return ast_sip_session_refresh(session, NULL, NULL, NULL, - session->endpoint->media.direct_media.method, 1); + struct ast_sip_session *session = data; + int res; + + res = ast_sip_session_refresh(session, NULL, NULL, NULL, + session->endpoint->media.direct_media.method, 1); + ao2_ref(session, -1); + return res; } /*! \brief Destructor function for \ref transport_info_data */ @@ -1057,17 +1060,66 @@ return 0; } +/*! + * \internal + * \brief TRUE if a COLP update can be sent to the peer. + * \since 13.3.0 + * + * \param session The session to see if the COLP update is allowed. + * + * \retval 0 Update is not allowed. + * \retval 1 Update is allowed. + */ +static int is_colp_update_allowed(struct ast_sip_session *session) +{ + struct ast_party_id connected_id; + int update_allowed = 0; + + if (!session->endpoint->id.send_pai && !session->endpoint->id.send_rpid) { + return 0; + } + + /* + * Check if privacy allows the update. Check while the channel + * is locked so we can work with the shallow connected_id copy. + */ + ast_channel_lock(session->channel); + connected_id = ast_channel_connected_effective_id(session->channel); + if (connected_id.number.valid + && (session->endpoint->id.trust_outbound + || (ast_party_id_presentation(&connected_id) & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED)) { + update_allowed = 1; + } + ast_channel_unlock(session->channel); + + return update_allowed; +} + /*! \brief Update connected line information */ static int update_connected_line_information(void *data) { - RAII_VAR(struct ast_sip_session *, session, data, ao2_cleanup); - - if ((ast_channel_state(session->channel) != AST_STATE_UP) && (session->inv_session->role == PJSIP_UAS_ROLE)) { + struct ast_sip_session *session = data; + + if (ast_channel_state(session->channel) == AST_STATE_UP + || session->inv_session->role == PJSIP_ROLE_UAC) { + if (is_colp_update_allowed(session)) { + enum ast_sip_session_refresh_method method; + int generate_new_sdp; + + method = session->endpoint->id.refresh_method; + if (session->inv_session->invite_tsx + && (session->inv_session->options & PJSIP_INV_SUPPORT_UPDATE)) { + method = AST_SIP_SESSION_REFRESH_METHOD_UPDATE; + } + + /* Only the INVITE method actually needs SDP, UPDATE can do without */ + generate_new_sdp = (method == AST_SIP_SESSION_REFRESH_METHOD_INVITE); + + ast_sip_session_refresh(session, NULL, NULL, NULL, method, generate_new_sdp); + } + } else if (session->inv_session->state != PJSIP_INV_STATE_DISCONNECTED + && is_colp_update_allowed(session)) { int response_code = 0; - - if (session->inv_session->state == PJSIP_INV_STATE_DISCONNECTED) { - return 0; - } if (ast_channel_state(session->channel) == AST_STATE_RING) { response_code = !session->endpoint->inband_progress ? 180 : 183; @@ -1082,34 +1134,9 @@ ast_sip_session_send_response(session, packet); } } - } else { - enum ast_sip_session_refresh_method method = session->endpoint->id.refresh_method; - int generate_new_sdp; - struct ast_party_id connected_id; - - if (session->inv_session->invite_tsx && (session->inv_session->options & PJSIP_INV_SUPPORT_UPDATE)) { - method = AST_SIP_SESSION_REFRESH_METHOD_UPDATE; - } - - /* Only the INVITE method actually needs SDP, UPDATE can do without */ - generate_new_sdp = (method == AST_SIP_SESSION_REFRESH_METHOD_INVITE); - - /* - * We can get away with a shallow copy here because we are - * not looking at strings. - */ - ast_channel_lock(session->channel); - connected_id = ast_channel_connected_effective_id(session->channel); - ast_channel_unlock(session->channel); - - if ((session->endpoint->id.send_pai || session->endpoint->id.send_rpid) && - (session->endpoint->id.trust_outbound || - ((connected_id.name.presentation & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED && - (connected_id.number.presentation & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED))) { - ast_sip_session_refresh(session, NULL, NULL, NULL, method, generate_new_sdp); - } - } - + } + + ao2_ref(session, -1); return 0; } Modified: branches/13/res/res_pjsip_caller_id.c URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_caller_id.c?view=diff&rev=432892&r1=432891&r2=432892 ============================================================================== --- branches/13/res/res_pjsip_caller_id.c (original) +++ branches/13/res/res_pjsip_caller_id.c Fri Mar 13 11:26:38 2015 @@ -131,12 +131,12 @@ } privacy = pjsip_msg_find_hdr_by_name(rdata->msg_info.msg, &privacy_str, NULL); - if (!privacy) { - return 0; - } - if (!pj_stricmp2(&privacy->hvalue, "id")) { + if (privacy && !pj_stricmp2(&privacy->hvalue, "id")) { id->number.presentation = AST_PRES_PROHIB_USER_NUMBER_NOT_SCREENED; id->name.presentation = AST_PRES_PROHIB_USER_NUMBER_NOT_SCREENED; + } else { + id->number.presentation = AST_PRES_ALLOWED_USER_NUMBER_NOT_SCREENED; + id->name.presentation = AST_PRES_ALLOWED_USER_NUMBER_NOT_SCREENED; } return 0; @@ -176,12 +176,18 @@ privacy = pjsip_param_find(&rpid_hdr->other_param, &privacy_str); screen = pjsip_param_find(&rpid_hdr->other_param, &screen_str); if (privacy && !pj_stricmp2(&privacy->value, "full")) { - id->number.presentation |= AST_PRES_RESTRICTED; - id->name.presentation |= AST_PRES_RESTRICTED; + id->number.presentation = AST_PRES_RESTRICTED; + id->name.presentation = AST_PRES_RESTRICTED; + } else { + id->number.presentation = AST_PRES_ALLOWED; + id->name.presentation = AST_PRES_ALLOWED; } if (screen && !pj_stricmp2(&screen->value, "yes")) { id->number.presentation |= AST_PRES_USER_NUMBER_PASSED_SCREEN; id->name.presentation |= AST_PRES_USER_NUMBER_PASSED_SCREEN; + } else { + id->number.presentation |= AST_PRES_USER_NUMBER_UNSCREENED; + id->name.presentation |= AST_PRES_USER_NUMBER_UNSCREENED; } return 0; @@ -475,8 +481,7 @@ old_privacy = pjsip_msg_find_hdr_by_name(tdata->msg, &pj_privacy_name, NULL); - if ((id->name.presentation & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED && - (id->number.presentation & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED) { + if ((ast_party_id_presentation(id) & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED) { if (old_privacy) { pj_list_erase(old_privacy); } @@ -498,10 +503,6 @@ static const pj_str_t pj_pai_name = { "P-Asserted-Identity", 19 }; pjsip_fromto_hdr *pai_hdr; pjsip_fromto_hdr *old_pai; - - if (!id->number.valid) { - return; - } /* Since inv_session reuses responses, we have to make sure there's not already * a P-Asserted-Identity present. If there is, we just modify the old one. @@ -546,6 +547,7 @@ pjsip_param *old_screen; pjsip_param *privacy; pjsip_param *screen; + int presentation; old_privacy = pjsip_param_find(&hdr->other_param, &privacy_str); old_screen = pjsip_param_find(&hdr->other_param, &screen_str); @@ -566,15 +568,13 @@ screen = old_screen; } - if ((id->name.presentation & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED && - (id->name.presentation & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED) { + presentation = ast_party_id_presentation(id); + if ((presentation & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED) { privacy->value = privacy_off_str; } else { privacy->value = privacy_full_str; } - - if ((id->name.presentation & AST_PRES_NUMBER_TYPE) == AST_PRES_USER_NUMBER_PASSED_SCREEN && - (id->number.presentation & AST_PRES_NUMBER_TYPE) == AST_PRES_USER_NUMBER_PASSED_SCREEN) { + if ((presentation & AST_PRES_NUMBER_TYPE) == AST_PRES_USER_NUMBER_PASSED_SCREEN) { screen->value = screen_yes_str; } else { screen->value = screen_no_str; @@ -592,10 +592,6 @@ static const pj_str_t pj_rpid_name = { "Remote-Party-ID", 15 }; pjsip_fromto_hdr *rpid_hdr; pjsip_fromto_hdr *old_rpid; - - if (!id->number.valid) { - return; - } /* Since inv_session reuses responses, we have to make sure there's not already * a P-Asserted-Identity present. If there is, we just modify the old one. @@ -628,9 +624,9 @@ */ static void add_id_headers(const struct ast_sip_session *session, pjsip_tx_data *tdata, const struct ast_party_id *id) { - if (((id->name.presentation & AST_PRES_RESTRICTION) == AST_PRES_RESTRICTED || - (id->number.presentation & AST_PRES_RESTRICTION) == AST_PRES_RESTRICTED) && - !session->endpoint->id.trust_outbound) { + if (!id->number.valid + || (!session->endpoint->id.trust_outbound + && (ast_party_id_presentation(id) & AST_PRES_RESTRICTION) != AST_PRES_ALLOWED)) { return; } if (session->endpoint->id.send_pai) { @@ -679,10 +675,9 @@ from = pjsip_msg_find_hdr(tdata->msg, PJSIP_H_FROM, tdata->msg->hdr.next); dlg = session->inv_session->dlg; - if (ast_strlen_zero(session->endpoint->fromuser) && - (session->endpoint->id.trust_outbound || - ((connected_id.name.presentation & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED && - (connected_id.number.presentation & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED))) { + if (ast_strlen_zero(session->endpoint->fromuser) + && (session->endpoint->id.trust_outbound + || (ast_party_id_presentation(&connected_id) & AST_PRES_RESTRICTION) == AST_PRES_ALLOWED)) { modify_id_header(tdata->pool, from, &connected_id); modify_id_header(dlg->pool, dlg->local.info, &connected_id); } -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits
