mirror of
https://gitlab.winehq.org/wine/wine-gecko.git
synced 2024-09-13 09:24:08 -07:00
Bug 917619 - Fix setup direction when a=setup is missing r=ehugg
This commit is contained in:
parent
fcdbb897d4
commit
63298ca29b
@ -1891,39 +1891,6 @@ gsmsdp_set_setup_attribute(uint16_t level,
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* gsmsdp_set_connection_attribute
|
||||
*
|
||||
* Description:
|
||||
*
|
||||
* Adds a connection attribute to the specified SDP.
|
||||
*
|
||||
* Parameters:
|
||||
*
|
||||
* level - The media level of the SDP where the media attribute exists.
|
||||
* sdp_p - Pointer to the SDP to set the ice candidate attribute against.
|
||||
* connection_type - Value for the a=connection line
|
||||
*/
|
||||
static void
|
||||
gsmsdp_set_connection_attribute(uint16_t level,
|
||||
void *sdp_p, sdp_connection_type_e connection_type) {
|
||||
uint16_t a_instance = 0;
|
||||
sdp_result_e result;
|
||||
|
||||
result = sdp_add_new_attr(sdp_p, level, 0, SDP_ATTR_CONNECTION,
|
||||
&a_instance);
|
||||
if (result != SDP_SUCCESS) {
|
||||
GSM_ERR_MSG("Failed to add attribute");
|
||||
return;
|
||||
}
|
||||
|
||||
result = sdp_attr_set_connection_attribute(sdp_p, level, 0,
|
||||
a_instance, connection_type);
|
||||
if (result != SDP_SUCCESS) {
|
||||
GSM_ERR_MSG("Failed to set attribute");
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* gsmsdp_set_dtls_fingerprint_attribute
|
||||
*
|
||||
@ -4700,7 +4667,23 @@ gsmsdp_negotiate_rtcp_fb (cc_sdp_t *cc_sdp_p,
|
||||
* initial_offer - Boolean indicating if the remote SDP came in the first OFFER of this session
|
||||
* offer - Boolean indicating if the remote SDP came in an OFFER.
|
||||
* notify_stream_added - Boolean indicating the UI should be notified of streams added
|
||||
* create_answer - indicates whether the provided offer is supposed to generate
|
||||
* an answer (versus simply setting the remote description)
|
||||
*
|
||||
* In practice, the initial_offer, offer, and create_answer flags only make
|
||||
* sense in a limited number of configurations:
|
||||
*
|
||||
* Phase | i_o | offer | c_a
|
||||
* ------------------------------+-------+-------+-------
|
||||
* SetRemote (initial offer) | true | true | false
|
||||
* SetRemote (reneg. offer) | false | true | false
|
||||
* SetRemote (answer) | false | false | false
|
||||
* CreateAnswer (initial) | true | true | true
|
||||
* CreateAnswer (renegotitaion) | false | true | true
|
||||
*
|
||||
* TODO(adam@nostrum.com): These flags make the code very hard to read at
|
||||
* the calling site. They should be replaced by an enumeration that
|
||||
* contains only those five valid combinations described above.
|
||||
*/
|
||||
cc_causes_t
|
||||
gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial_offer,
|
||||
@ -4735,7 +4718,6 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial
|
||||
sdp_result_e sdp_res;
|
||||
boolean created_media_stream = FALSE;
|
||||
int lsm_rc;
|
||||
sdp_setup_type_e remote_setup_type;
|
||||
|
||||
config_get_value(CFGID_SDPMODE, &sdpmode, sizeof(sdpmode));
|
||||
|
||||
@ -5032,41 +5014,63 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial
|
||||
if (!unsupported_line) {
|
||||
|
||||
if (sdpmode) {
|
||||
sdp_setup_type_e remote_setup_type;
|
||||
int j;
|
||||
|
||||
/* Find the remote a=setup value */
|
||||
sdp_res = sdp_attr_get_setup_attribute(
|
||||
sdp_p->dest_sdp, i, 0, 1, &remote_setup_type);
|
||||
|
||||
|
||||
/* setup attribute
|
||||
We are setting our local SDP to be ACTIVE if the value
|
||||
in the remote SDP is missing, PASSIVE or ACTPASS.
|
||||
If the remote value is ACTIVE, then we will respond
|
||||
with PASSIVE.
|
||||
If the remote value is HOLDCONN we will respond with
|
||||
HOLDCONN and set the direction to INACTIVE
|
||||
The DTLS role will then be set when the TransportFlow
|
||||
is created */
|
||||
media->setup = SDP_SETUP_ACTIVE;
|
||||
|
||||
if (sdp_res == SDP_SUCCESS) {
|
||||
if (remote_setup_type == SDP_SETUP_ACTIVE) {
|
||||
media->setup = SDP_SETUP_PASSIVE;
|
||||
} else if (remote_setup_type == SDP_SETUP_HOLDCONN) {
|
||||
media->setup = SDP_SETUP_HOLDCONN;
|
||||
media->direction = SDP_DIRECTION_INACTIVE;
|
||||
}
|
||||
/*
|
||||
* Although a=setup is required for DTLS per RFC 5763,
|
||||
* there are several implementations (including older
|
||||
* versions of Firefox) that don't signal direction.
|
||||
* To work with these cases, we assume that an omitted
|
||||
* direction in SDP means "PASSIVE" in an offer, and
|
||||
* "ACTIVE" in an answer.
|
||||
*/
|
||||
if (sdp_res != SDP_SUCCESS) {
|
||||
remote_setup_type =
|
||||
offer ? SDP_SETUP_PASSIVE : SDP_SETUP_ACTIVE;
|
||||
}
|
||||
|
||||
gsmsdp_set_setup_attribute(media->level, dcb_p->sdp->src_sdp,
|
||||
media->setup);
|
||||
/* The DTLS role will be set based on the media->setup
|
||||
value when the TransportFlow is created */
|
||||
switch (remote_setup_type) {
|
||||
case SDP_SETUP_ACTIVE:
|
||||
media->setup = SDP_SETUP_PASSIVE;
|
||||
break;
|
||||
case SDP_SETUP_PASSIVE:
|
||||
media->setup = SDP_SETUP_ACTIVE;
|
||||
break;
|
||||
case SDP_SETUP_ACTPASS:
|
||||
/*
|
||||
* This should only happen in an offer. If the
|
||||
* remote side is ACTPASS, we choose to be ACTIVE;
|
||||
* this allows us to start setting up the DTLS
|
||||
* association immediately, saving 1/2 RTT in
|
||||
* association establishment.
|
||||
*/
|
||||
media->setup = SDP_SETUP_ACTIVE;
|
||||
break;
|
||||
case SDP_SETUP_HOLDCONN:
|
||||
media->setup = SDP_SETUP_HOLDCONN;
|
||||
media->direction = SDP_DIRECTION_INACTIVE;
|
||||
break;
|
||||
default:
|
||||
/*
|
||||
* If we don't recognize the remote endpoint's setup
|
||||
* attribute, we fall back to being active if they
|
||||
* sent an offer, and passive if they sent an answer.
|
||||
*/
|
||||
media->setup =
|
||||
offer ? SDP_SETUP_ACTIVE : SDP_SETUP_PASSIVE;
|
||||
}
|
||||
|
||||
/* TODO(ehugg) we are not yet supporting existing connections
|
||||
See bug 857115. We currently always respond with
|
||||
connection:new */
|
||||
gsmsdp_set_connection_attribute(media->level,
|
||||
dcb_p->sdp->src_sdp, SDP_CONNECTION_NEW);
|
||||
if (create_answer) {
|
||||
gsmsdp_set_setup_attribute(media->level,
|
||||
dcb_p->sdp->src_sdp,
|
||||
media->setup);
|
||||
}
|
||||
|
||||
/* Set ICE */
|
||||
for (j=0; j<media->candidate_ct; j++) {
|
||||
@ -5585,13 +5589,9 @@ gsmsdp_add_media_line (fsmdef_dcb_t *dcb_p, const cc_media_cap_t *media_cap,
|
||||
sdp_rtcp_fb_ccm_to_bitmap(SDP_RTCP_FB_CCM_FIR));
|
||||
}
|
||||
|
||||
/* setup and connection attributes */
|
||||
/* Add a=setup attribute */
|
||||
gsmsdp_set_setup_attribute(level, dcb_p->sdp->src_sdp, media->setup);
|
||||
|
||||
/* This is a new media line so we should send connection:new */
|
||||
gsmsdp_set_connection_attribute(level, dcb_p->sdp->src_sdp,
|
||||
SDP_CONNECTION_NEW);
|
||||
|
||||
/*
|
||||
* wait until here to set ICE candidates as SDP is now initialized
|
||||
*/
|
||||
|
@ -2872,57 +2872,8 @@ TEST_F(SignalingTest, AudioCallGarbageSetup)
|
||||
ASSERT_GE(a2_.GetPacketsReceived(0), 40);
|
||||
}
|
||||
|
||||
// In this test we will change the offer SDP's a=connection value
|
||||
// from new to garbage. It should ignore the garbage value
|
||||
// and respond with connection:new
|
||||
TEST_F(SignalingTest, AudioCallGarbageConnection)
|
||||
{
|
||||
sipcc::MediaConstraints constraints;
|
||||
size_t match;
|
||||
|
||||
a1_.CreateOffer(constraints, OFFER_AUDIO, SHOULD_SENDRECV_AUDIO);
|
||||
|
||||
// By default the offer should give connection:new
|
||||
std::string offer(a1_.offer());
|
||||
match = offer.find("\r\na=connection:new");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
// Now replace the 'new' with a garbage value
|
||||
offer.replace(match, strlen("\r\na=connection:new"),
|
||||
"\r\na=connection:G4rb4g3V4lu3");
|
||||
std::cout << "Modified SDP " << std::endl
|
||||
<< indent(offer) << std::endl;
|
||||
|
||||
a1_.SetLocal(TestObserver::OFFER, offer.c_str(), false);
|
||||
a2_.SetRemote(TestObserver::OFFER, offer.c_str(), false);
|
||||
a2_.CreateAnswer(constraints, offer.c_str(), OFFER_AUDIO | ANSWER_AUDIO);
|
||||
|
||||
// Now the answer should contain a=connection:new
|
||||
std::string answer(a2_.answer());
|
||||
match = answer.find("\r\na=connection:new");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
|
||||
// This should setup the DTLS with the same roles
|
||||
// as the regular tests above.
|
||||
a2_.SetLocal(TestObserver::ANSWER, a2_.answer(), false);
|
||||
a1_.SetRemote(TestObserver::ANSWER, a2_.answer(), false);
|
||||
|
||||
ASSERT_TRUE_WAIT(a1_.IceCompleted() == true, kDefaultTimeout);
|
||||
ASSERT_TRUE_WAIT(a2_.IceCompleted() == true, kDefaultTimeout);
|
||||
|
||||
// Wait for some data to get written
|
||||
ASSERT_TRUE_WAIT(a1_.GetPacketsSent(0) >= 40 &&
|
||||
a2_.GetPacketsReceived(0) >= 40, kDefaultTimeout * 2);
|
||||
|
||||
a1_.CloseSendStreams();
|
||||
a2_.CloseReceiveStreams();
|
||||
|
||||
ASSERT_GE(a1_.GetPacketsSent(0), 40);
|
||||
ASSERT_GE(a2_.GetPacketsReceived(0), 40);
|
||||
}
|
||||
|
||||
// In this test we will change the offer SDP to remove the
|
||||
// a=setup and a=connection lines. Answer should respond with
|
||||
// a=setup:active and a=connection:new
|
||||
// a=setup line. Answer should respond with a=setup:active.
|
||||
TEST_F(SignalingTest, AudioCallOfferNoSetupOrConnection)
|
||||
{
|
||||
sipcc::MediaConstraints constraints;
|
||||
@ -2930,16 +2881,12 @@ TEST_F(SignalingTest, AudioCallOfferNoSetupOrConnection)
|
||||
|
||||
a1_.CreateOffer(constraints, OFFER_AUDIO, SHOULD_SENDRECV_AUDIO);
|
||||
|
||||
// By default the offer should give setup:actpass and connection:new
|
||||
// By default the offer should give setup:actpass
|
||||
std::string offer(a1_.offer());
|
||||
match = offer.find("\r\na=setup:actpass");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
// Remove the a=setup line
|
||||
offer.replace(match, strlen("\r\na=setup:actpass"), "");
|
||||
match = offer.find("\r\na=connection:new");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
// Remove the a=connection line
|
||||
offer.replace(match, strlen("\r\na=connection:new"), "");
|
||||
std::cout << "Modified SDP " << std::endl
|
||||
<< indent(offer) << std::endl;
|
||||
|
||||
@ -2947,12 +2894,10 @@ TEST_F(SignalingTest, AudioCallOfferNoSetupOrConnection)
|
||||
a2_.SetRemote(TestObserver::OFFER, offer.c_str(), false);
|
||||
a2_.CreateAnswer(constraints, offer.c_str(), OFFER_AUDIO | ANSWER_AUDIO);
|
||||
|
||||
// Now the answer should contain a=setup:active and a=connection:new
|
||||
// Now the answer should contain a=setup:active
|
||||
std::string answer(a2_.answer());
|
||||
match = answer.find("\r\na=setup:active");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
match = answer.find("\r\na=connection:new");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
|
||||
// This should setup the DTLS with the same roles
|
||||
// as the regular tests above.
|
||||
@ -2974,8 +2919,8 @@ TEST_F(SignalingTest, AudioCallOfferNoSetupOrConnection)
|
||||
}
|
||||
|
||||
// In this test we will change the answer SDP to remove the
|
||||
// a=setup and a=connection lines. ICE should still connect
|
||||
// since active will be assumed.
|
||||
// a=setup line. ICE should still connect since active will
|
||||
// be assumed.
|
||||
TEST_F(SignalingTest, AudioCallAnswerNoSetupOrConnection)
|
||||
{
|
||||
sipcc::MediaConstraints constraints;
|
||||
@ -2983,34 +2928,28 @@ TEST_F(SignalingTest, AudioCallAnswerNoSetupOrConnection)
|
||||
|
||||
a1_.CreateOffer(constraints, OFFER_AUDIO, SHOULD_SENDRECV_AUDIO);
|
||||
|
||||
// By default the offer should give setup:actpass and connection:new
|
||||
// By default the offer should give setup:actpass
|
||||
std::string offer(a1_.offer());
|
||||
match = offer.find("\r\na=setup:actpass");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
match = offer.find("\r\na=connection:new");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
|
||||
a1_.SetLocal(TestObserver::OFFER, offer.c_str(), false);
|
||||
a2_.SetRemote(TestObserver::OFFER, offer.c_str(), false);
|
||||
a2_.CreateAnswer(constraints, offer.c_str(), OFFER_AUDIO | ANSWER_AUDIO);
|
||||
|
||||
// Now the answer should contain a=setup:active and a=connection:new
|
||||
// Now the answer should contain a=setup:active
|
||||
std::string answer(a2_.answer());
|
||||
match = answer.find("\r\na=setup:active");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
// Remove the a=setup line
|
||||
answer.replace(match, strlen("\r\na=setup:active"), "");
|
||||
match = answer.find("\r\na=connection:new");
|
||||
ASSERT_NE(match, std::string::npos);
|
||||
// Remove the a=connection line
|
||||
answer.replace(match, strlen("\r\na=connection:new"), "");
|
||||
std::cout << "Modified SDP " << std::endl
|
||||
<< indent(answer) << std::endl;
|
||||
|
||||
// This should setup the DTLS with the same roles
|
||||
// as the regular tests above.
|
||||
a2_.SetLocal(TestObserver::ANSWER, a2_.answer(), false);
|
||||
a1_.SetRemote(TestObserver::ANSWER, a2_.answer(), false);
|
||||
a2_.SetLocal(TestObserver::ANSWER, answer, false);
|
||||
a1_.SetRemote(TestObserver::ANSWER, answer, false);
|
||||
|
||||
ASSERT_TRUE_WAIT(a1_.IceCompleted() == true, kDefaultTimeout);
|
||||
ASSERT_TRUE_WAIT(a2_.IceCompleted() == true, kDefaultTimeout);
|
||||
|
Loading…
Reference in New Issue
Block a user