mirror of
https://github.com/mozilla/gecko-dev.git
synced 2025-02-16 13:56:29 +00:00
Bug 834100 - Null deref if you call addIceCandidate on an RTCPeerConnection before setting localDesc [@ fsmdef_ev_addcandidate]. r=abr
This commit is contained in:
parent
7da9afc158
commit
a3d39b9bfa
@ -552,7 +552,7 @@ PeerConnection.prototype = {
|
||||
return Cr.NS_ERROR_NOT_IMPLEMENTED;
|
||||
},
|
||||
|
||||
addIceCandidate: function(cand) {
|
||||
addIceCandidate: function(cand, onSuccess, onError) {
|
||||
if (!cand) {
|
||||
throw new Error ("NULL candidate passed to addIceCandidate!");
|
||||
}
|
||||
@ -561,10 +561,13 @@ PeerConnection.prototype = {
|
||||
throw new Error ("Invalid candidate passed to addIceCandidate!");
|
||||
}
|
||||
|
||||
this._onAddIceCandidateSuccess = onSuccess;
|
||||
this._onAddIceCandidateError = onError;
|
||||
|
||||
this._queueOrRun({
|
||||
func: this._pc.addIceCandidate,
|
||||
args: [cand.candidate, cand.sdpMid || "", cand.sdpMLineIndex],
|
||||
wait: false
|
||||
wait: true
|
||||
});
|
||||
},
|
||||
|
||||
@ -758,6 +761,22 @@ PeerConnectionObserver.prototype = {
|
||||
this._dompc._executeNext();
|
||||
},
|
||||
|
||||
onAddIceCandidateSuccess: function(code) {
|
||||
this._dompc._pendingType = null;
|
||||
if (this._dompc._onAddIceCandidateSuccess) {
|
||||
this._dompc._onAddIceCandidateSuccess.onCallback(code);
|
||||
}
|
||||
this._dompc._executeNext();
|
||||
},
|
||||
|
||||
onAddIceCandidateError: function(code) {
|
||||
this._dompc._pendingType = null;
|
||||
if (this._dompc._onAddIceCandidateError) {
|
||||
this._dompc._onAddIceCandidateError.onCallback(code);
|
||||
}
|
||||
this._dompc._executeNext();
|
||||
},
|
||||
|
||||
onStateChange: function(state) {
|
||||
if (state != Ci.IPeerConnectionObserver.kIceState) {
|
||||
return;
|
||||
|
@ -42,6 +42,8 @@ interface IPeerConnectionObserver : nsISupports
|
||||
void onSetRemoteDescriptionSuccess(in unsigned long code);
|
||||
void onSetLocalDescriptionError(in unsigned long code);
|
||||
void onSetRemoteDescriptionError(in unsigned long code);
|
||||
void onAddIceCandidateSuccess(in unsigned long code);
|
||||
void onAddIceCandidateError(in unsigned long code);
|
||||
|
||||
/* Data channel callbacks */
|
||||
void notifyDataChannel(in nsIDOMDataChannel channel);
|
||||
|
@ -59,7 +59,9 @@ interface nsIDOMRTCPeerConnection : nsISupports
|
||||
[optional] in jsval constraints,
|
||||
[optional] in bool restart);
|
||||
|
||||
void addIceCandidate(in nsIDOMRTCIceCandidate candidate);
|
||||
void addIceCandidate(in nsIDOMRTCIceCandidate candidate,
|
||||
[optional] in RTCPeerConnectionCallback successCallback,
|
||||
[optional] in RTCPeerConnectionCallback failureCallback);
|
||||
|
||||
void addStream(in nsIDOMMediaStream stream,
|
||||
[optional] in jsval constraints);
|
||||
|
24
dom/media/tests/crashtests/834100.html
Normal file
24
dom/media/tests/crashtests/834100.html
Normal file
@ -0,0 +1,24 @@
|
||||
<html class="reftest-wait">
|
||||
<head>
|
||||
<script language="javascript">
|
||||
|
||||
function start() {
|
||||
remotePC = new mozRTCPeerConnection();
|
||||
var cand = new mozRTCIceCandidate(
|
||||
{candidate: "1 1 UDP 1 127.0.0.1 34567 type host",
|
||||
sdpMid: "helloworld",
|
||||
sdbMid: "helloworld", // Mis-spelt attribute for bug 833948 compatibility.
|
||||
sdpMLineIndex: 1
|
||||
});
|
||||
remotePC.addIceCandidate(cand);
|
||||
remotePC.addIceCandidate(cand, function(sdp){}, finish);
|
||||
}
|
||||
|
||||
function finish(arg) {
|
||||
document.documentElement.removeAttribute("class");
|
||||
}
|
||||
</script>
|
||||
</head>
|
||||
<body onload="start()">
|
||||
</body>
|
||||
</html>
|
@ -8,4 +8,5 @@ load 799419.html
|
||||
load 801227.html
|
||||
load 802982.html
|
||||
load 812785.html
|
||||
load 834100.html
|
||||
load 822197.html
|
||||
|
@ -174,6 +174,14 @@ public:
|
||||
mObserver->OnSetRemoteDescriptionError(mCode);
|
||||
break;
|
||||
|
||||
case ADDICECANDIDATE:
|
||||
mObserver->OnAddIceCandidateSuccess(mCode);
|
||||
break;
|
||||
|
||||
case ADDICECANDIDATEERROR:
|
||||
mObserver->OnAddIceCandidateError(mCode);
|
||||
break;
|
||||
|
||||
case REMOTESTREAMADD:
|
||||
{
|
||||
nsDOMMediaStream* stream = nullptr;
|
||||
@ -200,7 +208,6 @@ public:
|
||||
}
|
||||
|
||||
case UPDATELOCALDESC:
|
||||
case UPDATEREMOTEDESC:
|
||||
/* No action necessary */
|
||||
break;
|
||||
|
||||
@ -1159,7 +1166,7 @@ PeerConnectionImpl::onCallEvent(ccapi_call_event_e aCallEvent,
|
||||
break;
|
||||
|
||||
case SETREMOTEDESC:
|
||||
case UPDATEREMOTEDESC:
|
||||
case ADDICECANDIDATE:
|
||||
mRemoteSDP = aInfo->getSDP();
|
||||
break;
|
||||
|
||||
|
@ -1319,6 +1319,7 @@ static void ccappUpdateSessionData (session_update_t *sessUpd)
|
||||
sessUpd->eventID == SET_REMOTE_DESC ||
|
||||
sessUpd->eventID == UPDATE_LOCAL_DESC ||
|
||||
sessUpd->eventID == UPDATE_REMOTE_DESC ||
|
||||
sessUpd->eventID == ICE_CANDIDATE_ADD ||
|
||||
sessUpd->eventID == REMOTE_STREAM_ADD ) {
|
||||
|
||||
CCAPP_DEBUG(DEB_F_PREFIX"CALL_SESSION_CREATED for session id 0x%x event is 0x%x \n",
|
||||
@ -1361,6 +1362,7 @@ static void ccappUpdateSessionData (session_update_t *sessUpd)
|
||||
sessUpd->eventID == SET_REMOTE_DESC ||
|
||||
sessUpd->eventID == UPDATE_LOCAL_DESC ||
|
||||
sessUpd->eventID == UPDATE_REMOTE_DESC ||
|
||||
sessUpd->eventID == ICE_CANDIDATE_ADD ||
|
||||
sessUpd->eventID == REMOTE_STREAM_ADD ) {
|
||||
data->attr = sessUpd->update.ccSessionUpd.data.state_data.attr;
|
||||
data->inst = sessUpd->update.ccSessionUpd.data.state_data.inst;
|
||||
@ -1391,6 +1393,7 @@ static void ccappUpdateSessionData (session_update_t *sessUpd)
|
||||
case SET_REMOTE_DESC:
|
||||
case UPDATE_LOCAL_DESC:
|
||||
case UPDATE_REMOTE_DESC:
|
||||
case ICE_CANDIDATE_ADD:
|
||||
data->sdp = sessUpd->update.ccSessionUpd.data.state_data.sdp;
|
||||
/* Fall through to the next case... */
|
||||
case REMOTE_STREAM_ADD:
|
||||
@ -1728,6 +1731,7 @@ static void ccappUpdateSessionData (session_update_t *sessUpd)
|
||||
case SET_REMOTE_DESC:
|
||||
case UPDATE_LOCAL_DESC:
|
||||
case UPDATE_REMOTE_DESC:
|
||||
case ICE_CANDIDATE_ADD:
|
||||
data->sdp = sessUpd->update.ccSessionUpd.data.state_data.sdp;
|
||||
/* Fall through to the next case... */
|
||||
case REMOTE_STREAM_ADD:
|
||||
|
@ -1670,25 +1670,20 @@ void ui_update_local_description(call_events event, line_t nLine, callid_t nCall
|
||||
}
|
||||
|
||||
/**
|
||||
* Let PeerConnection know about an updated remote session description
|
||||
* Send data from addIceCandidate to the UI
|
||||
*
|
||||
* @return none
|
||||
* @return none
|
||||
*/
|
||||
|
||||
void ui_update_remote_description(call_events event, line_t nLine, callid_t nCallID,
|
||||
uint16_t call_instance_id, string_t sdp)
|
||||
void ui_ice_candidate_add(call_events event, line_t nLine, callid_t nCallID,
|
||||
uint16_t call_instance_id, string_t sdp)
|
||||
{
|
||||
TNP_DEBUG(DEB_L_C_F_PREFIX"state=%d call_instance=%d\n",
|
||||
DEB_L_C_F_PREFIX_ARGS(UI_API, nLine, nCallID, __FUNCTION__),
|
||||
event, call_instance_id);
|
||||
DEB_L_C_F_PREFIX_ARGS(UI_API, nLine, nCallID, __FUNCTION__), event, call_instance_id);
|
||||
|
||||
post_message_helper(UPDATE_REMOTE_DESC, event, nLine, nCallID,
|
||||
call_instance_id, sdp, PC_OK);
|
||||
|
||||
return;
|
||||
post_message_helper(ICE_CANDIDATE_ADD, event, nLine, nCallID, call_instance_id, sdp, PC_OK);
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Send Remote Stream data to the UI
|
||||
*
|
||||
|
@ -3617,15 +3617,29 @@ fsmdef_ev_addcandidate(sm_event_t *event) {
|
||||
|
||||
config_get_value(CFGID_SDPMODE, &sdpmode, sizeof(sdpmode));
|
||||
if (sdpmode == FALSE) {
|
||||
|
||||
ui_ice_candidate_add(evAddIceCandidateError, line, call_id,
|
||||
dcb->caller_id.call_instance_id, strlib_empty());
|
||||
return (SM_RC_END);
|
||||
}
|
||||
|
||||
if (dcb == NULL) {
|
||||
FSM_DEBUG_SM(DEB_F_PREFIX"dcb is NULL.\n", DEB_F_PREFIX_ARGS(FSM, __FUNCTION__));
|
||||
ui_ice_candidate_add(evAddIceCandidateError, line, call_id,
|
||||
dcb->caller_id.call_instance_id, strlib_empty());
|
||||
return SM_RC_CLEANUP;
|
||||
}
|
||||
|
||||
if (!dcb->sdp) {
|
||||
FSM_DEBUG_SM(DEB_F_PREFIX"dcb->sdp is NULL. Has the "
|
||||
"remote description been set yet?\n",
|
||||
DEB_F_PREFIX_ARGS(FSM, __FUNCTION__));
|
||||
|
||||
ui_ice_candidate_add(evAddIceCandidateError, line, call_id,
|
||||
dcb->caller_id.call_instance_id, strlib_empty());
|
||||
|
||||
return SM_RC_END;
|
||||
}
|
||||
|
||||
/* Perform level lookup based on mid value */
|
||||
/* comment until mid is properly updated
|
||||
cause = gsmsdp_find_level_from_mid(dcb, (const char *)msg->data.candidate.mid, &level);
|
||||
@ -3665,10 +3679,12 @@ fsmdef_ev_addcandidate(sm_event_t *event) {
|
||||
remote_sdp = sipsdp_write_to_buf(dcb->sdp->dest_sdp, &remote_sdp_len);
|
||||
|
||||
if (!remote_sdp) {
|
||||
ui_ice_candidate_add(evAddIceCandidateError, line, call_id,
|
||||
dcb->caller_id.call_instance_id, strlib_empty());
|
||||
return (SM_RC_END);
|
||||
}
|
||||
|
||||
ui_update_remote_description(evUpdateRemoteDesc, line, call_id,
|
||||
ui_ice_candidate_add(evAddIceCandidate, line, call_id,
|
||||
dcb->caller_id.call_instance_id, strlib_malloc(remote_sdp,-1));
|
||||
|
||||
free(remote_sdp);
|
||||
|
@ -222,7 +222,8 @@ typedef enum {
|
||||
SET_REMOTE_DESC,
|
||||
UPDATE_LOCAL_DESC,
|
||||
UPDATE_REMOTE_DESC,
|
||||
REMOTE_STREAM_ADD
|
||||
REMOTE_STREAM_ADD,
|
||||
ICE_CANDIDATE_ADD
|
||||
} group_call_event_t;
|
||||
|
||||
/* File Player Session Events */
|
||||
|
@ -38,10 +38,11 @@ typedef enum {
|
||||
evSetLocalDesc = SETLOCALDESC,
|
||||
evSetRemoteDesc = SETREMOTEDESC,
|
||||
evUpdateLocalDesc = UPDATELOCALDESC,
|
||||
evUpdateRemoteDesc = UPDATEREMOTEDESC,
|
||||
evSetLocalDescError = SETLOCALDESCERROR,
|
||||
evSetRemoteDescError = SETREMOTEDESCERROR,
|
||||
evOnRemoteStreamAdd = REMOTESTREAMADD,
|
||||
evAddIceCandidate = ADDICECANDIDATE,
|
||||
evAddIceCandidateError = ADDICECANDIDATEERROR,
|
||||
evMaxEvent
|
||||
} call_events;
|
||||
|
||||
@ -162,8 +163,9 @@ void ui_set_remote_description(call_events event, line_t nLine, callid_t nCallID
|
||||
|
||||
void ui_update_local_description(call_events event, line_t nLine, callid_t nCallID,
|
||||
uint16_t call_instance_id, string_t sdp);
|
||||
void ui_update_remote_description(call_events event, line_t nLine, callid_t nCallID,
|
||||
uint16_t call_instance_id, string_t sdp);
|
||||
|
||||
void ui_ice_candidate_add(call_events event, line_t nLine, callid_t nCallID,
|
||||
uint16_t call_instance_id, string_t sdp);
|
||||
|
||||
void ui_on_remote_stream_added(call_events event, line_t nLine, callid_t nCallID,
|
||||
uint16_t call_instance_id, cc_media_remote_track_table_t media_tracks);
|
||||
|
@ -287,6 +287,8 @@ typedef enum {
|
||||
SETLOCALDESCERROR,
|
||||
SETREMOTEDESCERROR,
|
||||
REMOTESTREAMADD,
|
||||
ADDICECANDIDATE,
|
||||
ADDICECANDIDATEERROR,
|
||||
MAX_CALL_STATES
|
||||
} cc_call_state_t;
|
||||
|
||||
|
@ -138,9 +138,6 @@ std::string CC_SIPCCCallInfo::callStateToString (cc_call_state_t state)
|
||||
case UPDATELOCALDESC:
|
||||
statestr = "UPDATELOCALDESC";
|
||||
break;
|
||||
case UPDATEREMOTEDESC:
|
||||
statestr = "UPDATEREMOTEDESC";
|
||||
break;
|
||||
case SETLOCALDESCERROR:
|
||||
statestr = "SETLOCALDESCERROR";
|
||||
break;
|
||||
@ -150,6 +147,12 @@ std::string CC_SIPCCCallInfo::callStateToString (cc_call_state_t state)
|
||||
case REMOTESTREAMADD:
|
||||
statestr = "REMOTESTREAMADD";
|
||||
break;
|
||||
case ADDICECANDIDATE:
|
||||
statestr = "ADDICECANDIDATE";
|
||||
break;
|
||||
case ADDICECANDIDATEERROR:
|
||||
statestr = "ADDICECANDIDATEERROR";
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
@ -144,7 +144,7 @@ public:
|
||||
};
|
||||
|
||||
TestObserver(sipcc::PeerConnectionImpl *peerConnection) :
|
||||
state(stateNoResponse),
|
||||
state(stateNoResponse), addIceSuccessCount(0),
|
||||
onAddStreamCalled(false),
|
||||
pc(peerConnection) {
|
||||
}
|
||||
@ -160,6 +160,7 @@ public:
|
||||
char *lastString;
|
||||
uint32_t lastStatusCode;
|
||||
uint32_t lastStateType;
|
||||
int addIceSuccessCount;
|
||||
bool onAddStreamCalled;
|
||||
|
||||
private:
|
||||
@ -354,6 +355,24 @@ TestObserver::FoundIceCandidate(const char* strCandidate)
|
||||
return NS_OK;
|
||||
}
|
||||
|
||||
NS_IMETHODIMP
|
||||
TestObserver::OnAddIceCandidateSuccess(uint32_t code)
|
||||
{
|
||||
lastStatusCode = code;
|
||||
state = stateSuccess;
|
||||
addIceSuccessCount++;
|
||||
return NS_OK;
|
||||
}
|
||||
|
||||
NS_IMETHODIMP
|
||||
TestObserver::OnAddIceCandidateError(uint32_t code)
|
||||
{
|
||||
lastStatusCode = code;
|
||||
state = stateError;
|
||||
cout << "onAddIceCandidateError = " << code << endl;
|
||||
return NS_OK;
|
||||
}
|
||||
|
||||
class ParsedSDP {
|
||||
public:
|
||||
//Line number with the corresponding SDP line.
|
||||
@ -699,18 +718,24 @@ void CreateAnswer(sipcc::MediaConstraints& constraints, std::string offer,
|
||||
}
|
||||
|
||||
void DoTrickleIce(ParsedSDP &sdp) {
|
||||
int expectAddIce = 0;
|
||||
pObserver->addIceSuccessCount = 0;
|
||||
for (std::multimap<int, std::string>::iterator it = sdp.ice_candidates_.begin();
|
||||
it != sdp.ice_candidates_.end(); ++it) {
|
||||
if ((*it).first != 0) {
|
||||
std::cerr << "Adding trickle ICE candidate " << (*it).second << std::endl;
|
||||
|
||||
ASSERT_TRUE(NS_SUCCEEDED(pc->AddIceCandidate((*it).second.c_str(), "", (*it).first)));
|
||||
expectAddIce++;
|
||||
}
|
||||
}
|
||||
ASSERT_TRUE_WAIT(pObserver->addIceSuccessCount == expectAddIce,
|
||||
kDefaultTimeout);
|
||||
}
|
||||
|
||||
|
||||
void DoTrickleIceChrome(ParsedSDP &sdp) {
|
||||
int expectAddIce = 0;
|
||||
pObserver->addIceSuccessCount = 0;
|
||||
for (std::multimap<int, std::string>::iterator it = sdp.ice_candidates_.begin();
|
||||
it != sdp.ice_candidates_.end(); ++it) {
|
||||
if ((*it).first != 0) {
|
||||
@ -718,8 +743,11 @@ void CreateAnswer(sipcc::MediaConstraints& constraints, std::string offer,
|
||||
std::cerr << "Adding trickle ICE candidate " << candidate << std::endl;
|
||||
|
||||
ASSERT_TRUE(NS_SUCCEEDED(pc->AddIceCandidate(candidate.c_str(), "", (*it).first)));
|
||||
expectAddIce++;
|
||||
}
|
||||
}
|
||||
ASSERT_TRUE_WAIT(pObserver->addIceSuccessCount == expectAddIce,
|
||||
kDefaultTimeout);
|
||||
}
|
||||
|
||||
|
||||
@ -729,8 +757,16 @@ void CreateAnswer(sipcc::MediaConstraints& constraints, std::string offer,
|
||||
return state == sipcc::PeerConnectionImpl::kIceConnected;
|
||||
}
|
||||
|
||||
void AddIceCandidate(const char* candidate, const char* mid, unsigned short level) {
|
||||
void AddIceCandidate(const char* candidate, const char* mid, unsigned short level,
|
||||
bool expectSuccess) {
|
||||
pObserver->state = TestObserver::stateNoResponse;
|
||||
pc->AddIceCandidate(candidate, mid, level);
|
||||
ASSERT_TRUE_WAIT(pObserver->state != TestObserver::stateNoResponse,
|
||||
kDefaultTimeout);
|
||||
ASSERT_TRUE(pObserver->state ==
|
||||
expectSuccess ? TestObserver::stateSuccess :
|
||||
TestObserver::stateError
|
||||
);
|
||||
}
|
||||
|
||||
int GetPacketsReceived(int stream) {
|
||||
@ -1016,7 +1052,12 @@ public:
|
||||
const char * candidate, const char * mid,
|
||||
unsigned short level, uint32_t sdpCheck) {
|
||||
a1_.CreateOffer(constraints, OFFER_AV, sdpCheck);
|
||||
a1_.AddIceCandidate(candidate, mid, level);
|
||||
a1_.AddIceCandidate(candidate, mid, level, true);
|
||||
}
|
||||
|
||||
void AddIceCandidateEarly(const char * candidate, const char * mid,
|
||||
unsigned short level) {
|
||||
a1_.AddIceCandidate(candidate, mid, level, false);
|
||||
}
|
||||
|
||||
protected:
|
||||
@ -1342,6 +1383,13 @@ TEST_F(SignalingTest, CreateOfferAddCandidate)
|
||||
SHOULD_SENDRECV_AV);
|
||||
}
|
||||
|
||||
TEST_F(SignalingTest, AddIceCandidateEarly)
|
||||
{
|
||||
sipcc::MediaConstraints constraints;
|
||||
AddIceCandidateEarly(strSampleCandidate.c_str(),
|
||||
strSampleMid.c_str(), nSamplelevel);
|
||||
}
|
||||
|
||||
// XXX adam@nostrum.com -- This test seems questionable; we need to think
|
||||
// through what actually needs to be tested here.
|
||||
TEST_F(SignalingTest, DISABLED_OfferAnswerReNegotiateOfferAnswerDontReceiveVideoNoVideoStream)
|
||||
|
Loading…
x
Reference in New Issue
Block a user