From 041e20edfb01f56bbbd03aef184822590455c9f4 Mon Sep 17 00:00:00 2001 From: Simon Morlat Date: Mon, 6 Jan 2014 23:24:00 +0100 Subject: [PATCH] * fix incorrect behavior when the body of an INVITE cannot be understood or parsed. Previously it was answering 200Ok. * do not send 415 but 488 in case of incompatible codecs, which is more correct according to the RFC. 415 should be replied if the body cannot be understood (for example: is not application/sdp) --- coreapi/bellesip_sal/sal_op_call.c | 86 ++++++++++++++++++++++-------- coreapi/bellesip_sal/sal_op_impl.c | 6 +-- coreapi/callbacks.c | 3 +- coreapi/linphonecore.c | 2 +- coreapi/linphonecore.h | 5 +- coreapi/misc.c | 16 +++--- coreapi/sal.c | 2 +- include/sal/sal.h | 6 +-- tester/call_tester.c | 44 +++++++++++++-- tester/liblinphone_tester.c | 2 + 10 files changed, 125 insertions(+), 47 deletions(-) diff --git a/coreapi/bellesip_sal/sal_op_call.c b/coreapi/bellesip_sal/sal_op_call.c index a286001b2..024cf5313 100644 --- a/coreapi/bellesip_sal/sal_op_call.c +++ b/coreapi/bellesip_sal/sal_op_call.c @@ -19,6 +19,8 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. #include "sal_impl.h" #include "offeranswer.h" +static int extract_sdp(belle_sip_message_t* message,belle_sdp_session_description_t** session_desc, SalReason *error); + /*used for calls terminated before creation of a dialog*/ static void call_set_released(SalOp* op){ if (!op->call_released){ @@ -84,8 +86,8 @@ static void sdp_process(SalOp *h){ } } } - } + static int set_sdp(belle_sip_message_t *msg,belle_sdp_session_description_t* session_desc) { belle_sip_header_content_type_t* content_type ; belle_sip_header_content_length_t* content_length; @@ -159,10 +161,13 @@ static void process_dialog_terminated(void *ctx, const belle_sip_dialog_terminat static void handle_sdp_from_response(SalOp* op,belle_sip_response_t* response) { belle_sdp_session_description_t* sdp; - if ((sdp=belle_sdp_session_description_create(BELLE_SIP_MESSAGE(response)))) { - op->base.remote_media=sal_media_description_new(); - sdp_to_media_description(sdp,op->base.remote_media); - if (op->base.local_media) sdp_process(op); + SalReason reason; + if (extract_sdp(BELLE_SIP_MESSAGE(response),&sdp,&reason)==0) { + if (sdp){ + op->base.remote_media=sal_media_description_new(); + sdp_to_media_description(sdp,op->base.remote_media); + if (op->base.local_media) sdp_process(op); + }/*if no sdp in response, what can we do ?*/ } } @@ -322,15 +327,46 @@ static void unsupported_method(belle_sip_server_transaction_t* server_transactio return; } -static void process_sdp_for_invite(SalOp* op,belle_sip_request_t* invite) { +/* + * Extract the sdp from a sip message. + * If there is no body in the message, the session_desc is set to null, 0 is returned. + * If body was present is not a SDP or parsing of SDP failed, -1 is returned and SalReason is set appropriately. + * +**/ +static int extract_sdp(belle_sip_message_t* message,belle_sdp_session_description_t** session_desc, SalReason *error) { + belle_sip_header_content_type_t* content_type=belle_sip_message_get_header_by_type(message,belle_sip_header_content_type_t); + if (content_type){ + if (strcmp("application",belle_sip_header_content_type_get_type(content_type))==0 + && strcmp("sdp",belle_sip_header_content_type_get_subtype(content_type))==0) { + *session_desc=belle_sdp_session_description_parse(belle_sip_message_get_body(message)); + if (*session_desc==NULL) { + ms_error("Failed to parse SDP message."); + *error=SalReasonNotAcceptable; + return -1; + } + }else{ + *error=SalReasonUnsupportedContent; + return -1; + } + }else *session_desc=NULL; + return 0; +} + +static int process_sdp_for_invite(SalOp* op,belle_sip_request_t* invite) { belle_sdp_session_description_t* sdp; - if ((sdp=belle_sdp_session_description_create(BELLE_SIP_MESSAGE(invite)))) { - op->sdp_offering=FALSE; - op->base.remote_media=sal_media_description_new(); - sdp_to_media_description(sdp,op->base.remote_media); - belle_sip_object_unref(sdp); - }else - op->sdp_offering=TRUE; + SalReason reason; + if (extract_sdp(BELLE_SIP_MESSAGE(invite),&sdp,&reason)==0) { + if (sdp){ + op->sdp_offering=FALSE; + op->base.remote_media=sal_media_description_new(); + sdp_to_media_description(sdp,op->base.remote_media); + belle_sip_object_unref(sdp); + }else op->sdp_offering=TRUE; /*INVITE without SDP*/ + return 0; + }else{ + sal_call_decline(op,reason,NULL); + return -1; + } } static void process_request_event(void *op_base, const belle_sip_request_event_t *event) { @@ -415,13 +451,18 @@ static void process_request_event(void *op_base, const belle_sip_request_event_t /*great ACK received*/ if (strcmp("ACK",belle_sip_request_get_method(req))==0) { if (op->sdp_offering){ - if ((sdp=belle_sdp_session_description_create(BELLE_SIP_MESSAGE(req)))){ - if (op->base.remote_media) - sal_media_description_unref(op->base.remote_media); - op->base.remote_media=sal_media_description_new(); - sdp_to_media_description(sdp,op->base.remote_media); - sdp_process(op); - belle_sip_object_unref(sdp); + SalReason reason; + if (extract_sdp(BELLE_SIP_MESSAGE(req),&sdp,&reason)==0){ + if (sdp){ + if (op->base.remote_media) + sal_media_description_unref(op->base.remote_media); + op->base.remote_media=sal_media_description_new(); + sdp_to_media_description(sdp,op->base.remote_media); + sdp_process(op); + belle_sip_object_unref(sdp); + }else{ + ms_warning("SDP expected in ACK but not found."); + } } } /*FIXME @@ -445,9 +486,8 @@ static void process_request_event(void *op_base, const belle_sip_request_event_t sal_media_description_unref(op->result); op->result=NULL; } - process_sdp_for_invite(op,req); - - op->base.root->callbacks.call_updating(op); + if (process_sdp_for_invite(op,req)==0) + op->base.root->callbacks.call_updating(op); } else if (strcmp("INFO",belle_sip_request_get_method(req))==0){ if (belle_sip_message_get_body(BELLE_SIP_MESSAGE(req)) && strstr(belle_sip_message_get_body(BELLE_SIP_MESSAGE(req)),"picture_fast_update")) { diff --git a/coreapi/bellesip_sal/sal_op_impl.c b/coreapi/bellesip_sal/sal_op_impl.c index 47958ac6e..51dec9b01 100644 --- a/coreapi/bellesip_sal/sal_op_impl.c +++ b/coreapi/bellesip_sal/sal_op_impl.c @@ -334,7 +334,7 @@ SalReason sal_reason_to_sip_code(SalReason r){ case SalReasonForbidden: ret=403; break; - case SalReasonMedia: + case SalReasonUnsupportedContent: ret=415; break; case SalReasonNotFound: @@ -356,7 +356,7 @@ SalReason sal_reason_to_sip_code(SalReason r){ ret=401; break; case SalReasonNotAcceptable: - ret=488; + ret=488; /*or maybe 606 Not Acceptable ?*/ break; case SalReasonNoMatch: ret=481; @@ -385,7 +385,7 @@ void sal_compute_sal_errors_from_code(int code ,SalError* sal_err,SalReason* sal break; case 415: *sal_err=SalErrorFailure; - *sal_reason=SalReasonMedia; + *sal_reason=SalReasonUnsupportedContent; break; case 422: ms_error ("422 not implemented yet");; diff --git a/coreapi/callbacks.c b/coreapi/callbacks.c index 3ebce7488..c2c382114 100644 --- a/coreapi/callbacks.c +++ b/coreapi/callbacks.c @@ -612,7 +612,8 @@ static void call_failure(SalOp *op, SalError error, SalReason sr, const char *de if (lc->vtable.display_status) lc->vtable.display_status(lc,msg600); break; - case SalReasonMedia: + case SalReasonUnsupportedContent: /*params.media_encryption == LinphoneMediaEncryptionSRTP && !linphone_core_is_media_encryption_mandatory(lc)) { diff --git a/coreapi/linphonecore.c b/coreapi/linphonecore.c index 4bebd832a..a378f8366 100644 --- a/coreapi/linphonecore.c +++ b/coreapi/linphonecore.c @@ -2903,7 +2903,7 @@ void linphone_core_notify_incoming_call(LinphoneCore *lc, LinphoneCall *call){ md=sal_call_get_final_media_description(call->op); if (md){ if (sal_media_description_empty(md) || linphone_core_incompatible_security(lc,md)){ - sal_call_decline(call->op,SalReasonMedia,NULL); + sal_call_decline(call->op,SalReasonNotAcceptable,NULL); linphone_call_stop_media_streams(call); linphone_core_del_call(lc,call); linphone_call_unref(call); diff --git a/coreapi/linphonecore.h b/coreapi/linphonecore.h index 83fa35994..7358159a9 100644 --- a/coreapi/linphonecore.h +++ b/coreapi/linphonecore.h @@ -167,7 +167,7 @@ enum _LinphoneReason{ LinphoneReasonNotFound, /**lc,marie->lc,&pauline->stat.number_of_LinphoneCallOutgoingInit,1)); - /*flexisip will retain the 415 until the "urgent reply" timeout arrives.*/ + /*flexisip will retain the 488 until the "urgent reply" timeout arrives.*/ CU_ASSERT_TRUE(wait_for_until(pauline->lc,marie->lc,&pauline->stat.number_of_LinphoneCallError,1,6000)); - CU_ASSERT_EQUAL(linphone_call_get_reason(out_call),LinphoneReasonMedia); + CU_ASSERT_EQUAL(linphone_call_get_reason(out_call),LinphoneReasonNotAcceptable); CU_ASSERT_EQUAL(marie->stat.number_of_LinphoneCallIncomingReceived,0); linphone_call_unref(out_call); @@ -1387,6 +1387,41 @@ static void call_established_with_rejected_reinvite(void) { linphone_core_manager_destroy(pauline); } +static void call_established_with_rejected_incoming_reinvite(void) { + LinphoneCoreManager* marie = linphone_core_manager_new( "marie_rc"); + LinphoneCoreManager* pauline = linphone_core_manager_new( "pauline_rc"); + + CU_ASSERT_TRUE(call(pauline,marie)); + + /*wait for ACK to be transmitted before going to reINVITE*/ + wait_for_until(marie->lc,pauline->lc,NULL,0,1000); + + linphone_core_enable_payload_type(pauline->lc,linphone_core_find_payload_type(pauline->lc,"PCMU",8000,1),FALSE); /*disable PCMU*/ + linphone_core_enable_payload_type(pauline->lc,linphone_core_find_payload_type(pauline->lc,"PCMA",8000,1),TRUE); /*enable PCMA*/ + + linphone_core_update_call(marie->lc + ,linphone_core_get_current_call(marie->lc) + ,linphone_call_get_current_params(linphone_core_get_current_call(marie->lc))); + + + CU_ASSERT_TRUE(wait_for(marie->lc,pauline->lc,&marie->stat.number_of_LinphoneCallUpdating,1)); + CU_ASSERT_TRUE(wait_for(marie->lc,pauline->lc,&marie->stat.number_of_LinphoneCallStreamsRunning,2)); + + CU_ASSERT_EQUAL(linphone_call_get_reason(linphone_core_get_current_call(marie->lc)),LinphoneReasonNotAcceptable); + + CU_ASSERT_EQUAL(pauline->stat.number_of_LinphoneCallStreamsRunning,1); + check_call_state(pauline,LinphoneCallStreamsRunning); + check_call_state(marie,LinphoneCallStreamsRunning); + + /*just to sleep*/ + linphone_core_terminate_all_calls(pauline->lc); + CU_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&pauline->stat.number_of_LinphoneCallEnd,1)); + CU_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&marie->stat.number_of_LinphoneCallEnd,1)); + + linphone_core_manager_destroy(marie); + linphone_core_manager_destroy(pauline); +} + static void call_established_with_rejected_reinvite_with_error(void) { LinphoneCoreManager* marie = linphone_core_manager_new( "marie_rc"); LinphoneCoreManager* pauline = linphone_core_manager_new( "pauline_rc"); @@ -1399,8 +1434,8 @@ static void call_established_with_rejected_reinvite_with_error(void) { sal_enable_unconditional_answer(marie->lc->sal,TRUE); linphone_core_update_call( pauline->lc - ,linphone_core_get_current_call(pauline->lc) - ,linphone_call_get_current_params(linphone_core_get_current_call(pauline->lc))); + ,linphone_core_get_current_call(pauline->lc) + ,linphone_call_get_current_params(linphone_core_get_current_call(pauline->lc))); CU_ASSERT_TRUE(wait_for(marie->lc,pauline->lc,&pauline->stat.number_of_LinphoneCallStreamsRunning,2)); @@ -1460,6 +1495,7 @@ test_t call_tests[] = { { "Call with custom headers",call_with_custom_headers}, { "Call established with rejected INFO",call_established_with_rejected_info}, { "Call established with rejected RE-INVITE",call_established_with_rejected_reinvite}, + { "Call established with rejected incoming RE-INVITE", call_established_with_rejected_incoming_reinvite }, { "Call established with rejected RE-INVITE in error", call_established_with_rejected_reinvite_with_error} }; diff --git a/tester/liblinphone_tester.c b/tester/liblinphone_tester.c index 910cb43b3..a5be38d65 100644 --- a/tester/liblinphone_tester.c +++ b/tester/liblinphone_tester.c @@ -136,9 +136,11 @@ bool_t wait_for_until(LinphoneCore* lc_1, LinphoneCore* lc_2,int* counter,int va ms_list_free(lcs); return result; } + bool_t wait_for(LinphoneCore* lc_1, LinphoneCore* lc_2,int* counter,int value) { return wait_for_until(lc_1, lc_2,counter,value,3000); } + bool_t wait_for_list(MSList* lcs,int* counter,int value,int timeout_ms) { int retry=0; MSList* iterator;