Skip to content
Snippets Groups Projects
Commit 3e73e264 authored by Sébastien Blin's avatar Sébastien Blin Committed by Adrien Béraud
Browse files

pjproject: fix regression caused by bump to 2.11 

rfc6544 rebase introduced an error where no_pending_check was
wrong causing the on_ice_complete to never be called if all
candidates failed.

GitLab: #597
Change-Id: Idca5f9bb2b03d9ae4687cef64df96cef6509b15d
parent dd7c1e1c
No related branches found
No related tags found
No related merge requests found
Show whitespace changes
Inline Side-by-side
contrib/src/pjproject/0001-rfc6544.patch
+ 38
− 32
View file @ 3e73e264
......@@ -20,29 +20,29 @@ Rebased for pjsip 2.10 by Peymane Marandi
on behalf of Savoir-faire Linux.
---
pjnath/include/pjnath/config.h | 9 +
pjnath/include/pjnath/ice_session.h | 216 ++++-
pjnath/include/pjnath/ice_strans.h | 21 +
pjnath/include/pjnath/stun_session.h | 75 +-
pjnath/include/pjnath/stun_sock.h | 67 +-
pjnath/include/pjnath/turn_sock.h | 11 +
pjnath/include/pjnath/config.h | 9 ++
pjnath/include/pjnath/ice_session.h | 216 ++++++++++++++++++++++++++++++++++++++-------
pjnath/include/pjnath/ice_strans.h | 21 +++++
pjnath/include/pjnath/stun_session.h | 75 +++++++++++++++-
pjnath/include/pjnath/stun_sock.h | 67 +++++++++++++-
pjnath/include/pjnath/turn_sock.h | 11 +++
pjnath/src/pjnath-test/concur_test.c | 5 +-
pjnath/src/pjnath-test/sess_auth.c | 14 +-
pjnath/src/pjnath-test/sess_auth.c | 14 ++-
pjnath/src/pjnath-test/stun_sock_test.c | 7 +-
pjnath/src/pjnath/ice_session.c | 609 +++++++++++--
pjnath/src/pjnath/ice_strans.c | 743 +++++++++++++---
pjnath/src/pjnath/ice_session.c | 613 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------
pjnath/src/pjnath/ice_strans.c | 743 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
pjnath/src/pjnath/nat_detect.c | 7 +-
pjnath/src/pjnath/stun_session.c | 19 +-
pjnath/src/pjnath/stun_sock.c | 1081 +++++++++++++++++++----
pjnath/src/pjnath/stun_session.c | 19 +++-
pjnath/src/pjnath/stun_sock.c | 1081 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------
pjnath/src/pjnath/stun_transaction.c | 3 +
pjnath/src/pjnath/turn_session.c | 3 +-
pjnath/src/pjnath/turn_sock.c | 24 +-
pjnath/src/pjnath/turn_sock.c | 24 +++--
pjnath/src/pjturn-client/client_main.c | 2 +-
pjnath/src/pjturn-srv/allocation.c | 3 +-
pjnath/src/pjturn-srv/server.c | 2 +-
pjsip-apps/src/samples/icedemo.c | 116 ++-
pjsip-apps/src/samples/icedemo.c | 116 ++++++++++++++++++++++--
pjsip/src/pjsua-lib/pjsua_core.c | 2 +-
22 files changed, 2577 insertions(+), 462 deletions(-)
22 files changed, 2580 insertions(+), 463 deletions(-)
diff --git a/pjnath/include/pjnath/config.h b/pjnath/include/pjnath/config.h
index 8a656c225..3e7c6ae3a 100644
......@@ -762,7 +762,7 @@ index 18a1bbd13..577659c87 100644
if (status != PJ_SUCCESS && status != PJ_EPENDING) {
app_perror(" error: server sending data", status);
diff --git a/pjnath/src/pjnath/ice_session.c b/pjnath/src/pjnath/ice_session.c
index 3fecc3def..a3869d9ca 100644
index 3fecc3def..1fd3cfd2a 100644
--- a/pjnath/src/pjnath/ice_session.c
+++ b/pjnath/src/pjnath/ice_session.c
@@ -18,6 +18,7 @@
......@@ -917,7 +917,7 @@ index 3fecc3def..a3869d9ca 100644
case TIMER_NONE:
/* Nothing to do, just to get rid of gcc warning */
break;
@@ -1481,12 +1516,22 @@ static pj_bool_t check_ice_complete(pj_ice_sess *ice)
@@ -1481,15 +1516,27 @@ static pj_bool_t check_ice_complete(pj_ice_sess *ice)
* See if all checks in the checklist have completed. If we do,
* then mark ICE processing as failed.
*/
......@@ -944,9 +944,15 @@ index 3fecc3def..a3869d9ca 100644
+ if (!ice->is_trickling &&c->state < PJ_ICE_SESS_CHECK_STATE_SUCCEEDED) {
+ break;
}
no_pending_check = (i == ice->clist.count);
- no_pending_check = (i == ice->clist.count);
}
@@ -1506,8 +1551,8 @@ static pj_bool_t check_ice_complete(pj_ice_sess *ice)
+ if (!ice->is_trickling) {
+ no_pending_check = (i == ice->clist.count);
+ }
if (no_pending_check) {
/* All checks have completed, but we don't have nominated pair.
@@ -1506,8 +1553,8 @@ static pj_bool_t check_ice_complete(pj_ice_sess *ice)
if (i < ice->comp_cnt) {
/* This component ID doesn't have valid pair.
......@@ -957,7 +963,7 @@ index 3fecc3def..a3869d9ca 100644
on_ice_complete(ice, PJNATH_EICEFAILED);
return PJ_TRUE;
} else {
@@ -1541,11 +1586,48 @@ static pj_bool_t check_ice_complete(pj_ice_sess *ice)
@@ -1541,11 +1588,48 @@ static pj_bool_t check_ice_complete(pj_ice_sess *ice)
/* Unreached */
} else if (ice->is_nominating) {
......@@ -1011,7 +1017,7 @@ index 3fecc3def..a3869d9ca 100644
return PJ_TRUE;
} else {
@@ -1737,6 +1819,44 @@ static pj_bool_t on_check_complete(pj_ice_sess *ice,
@@ -1737,6 +1821,44 @@ static pj_bool_t on_check_complete(pj_ice_sess *ice,
return check_ice_complete(ice);
}
......@@ -1056,7 +1062,7 @@ index 3fecc3def..a3869d9ca 100644
/* Get foundation index of a check pair. This function can also be used for
* adding a new foundation (combination of local & remote cands foundations)
@@ -1905,6 +2025,29 @@ static pj_status_t add_rcand_and_update_checklist(
@@ -1905,6 +2027,29 @@ static pj_status_t add_rcand_and_update_checklist(
continue;
}
......@@ -1086,7 +1092,7 @@ index 3fecc3def..a3869d9ca 100644
#if 0
/* Trickle ICE:
* Make sure that pair has not been added to checklist
@@ -1925,7 +2068,6 @@ static pj_status_t add_rcand_and_update_checklist(
@@ -1925,7 +2070,6 @@ static pj_status_t add_rcand_and_update_checklist(
}
#endif
......@@ -1094,7 +1100,7 @@ index 3fecc3def..a3869d9ca 100644
/* Add the pair */
chk = &clist->checks[clist->count];
chk->lcand = lcand;
@@ -2121,6 +2263,7 @@ PJ_DEF(pj_status_t) pj_ice_sess_create_check_list(
@@ -2121,6 +2265,7 @@ PJ_DEF(pj_status_t) pj_ice_sess_create_check_list(
td = PJ_POOL_ZALLOC_T(ice->pool, timer_data);
td->ice = ice;
td->clist = clist;
......@@ -1102,7 +1108,7 @@ index 3fecc3def..a3869d9ca 100644
clist->timer.user_data = (void*)td;
clist->timer.cb = &periodic_timer;
@@ -2209,6 +2352,36 @@ PJ_DEF(pj_status_t) pj_ice_sess_update_check_list(
@@ -2209,6 +2354,36 @@ PJ_DEF(pj_status_t) pj_ice_sess_update_check_list(
return status;
}
......@@ -1139,7 +1145,7 @@ index 3fecc3def..a3869d9ca 100644
/* Perform check on the specified candidate pair. */
static pj_status_t perform_check(pj_ice_sess *ice,
pj_ice_sess_checklist *clist,
@@ -2219,19 +2392,17 @@ static pj_status_t perform_check(pj_ice_sess *ice,
@@ -2219,19 +2394,17 @@ static pj_status_t perform_check(pj_ice_sess *ice,
pj_ice_msg_data *msg_data;
pj_ice_sess_check *check;
const pj_ice_sess_cand *lcand;
......@@ -1160,7 +1166,7 @@ index 3fecc3def..a3869d9ca 100644
/* Create request */
status = pj_stun_session_create_req(comp->stun_sess,
@@ -2282,32 +2453,71 @@ static pj_status_t perform_check(pj_ice_sess *ice,
@@ -2282,32 +2455,71 @@ static pj_status_t perform_check(pj_ice_sess *ice,
&ice->tie_breaker);
} else {
......@@ -1249,7 +1255,7 @@ index 3fecc3def..a3869d9ca 100644
}
@@ -2344,55 +2554,108 @@ static pj_status_t start_periodic_check(pj_timer_heap_t *th,
@@ -2344,55 +2556,108 @@ static pj_status_t start_periodic_check(pj_timer_heap_t *th,
pj_log_push_indent();
/* Send STUN Binding request for check with highest priority on
......@@ -1379,7 +1385,7 @@ index 3fecc3def..a3869d9ca 100644
}
pj_grp_lock_release(ice->grp_lock);
@@ -2636,6 +2899,204 @@ static pj_status_t on_stun_send_msg(pj_stun_session *sess,
@@ -2636,6 +2901,204 @@ static pj_status_t on_stun_send_msg(pj_stun_session *sess,
return status;
}
......@@ -1584,7 +1590,7 @@ index 3fecc3def..a3869d9ca 100644
/* This callback is called when outgoing STUN request completed */
static void on_stun_request_complete(pj_stun_session *stun_sess,
@@ -2941,7 +3402,9 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
@@ -2941,7 +3404,9 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
&check->lcand->base_addr,
&check->lcand->base_addr,
pj_sockaddr_get_len(&xaddr->sockaddr),
......@@ -1595,7 +1601,7 @@ index 3fecc3def..a3869d9ca 100644
if (status != PJ_SUCCESS) {
check_set_state(ice, check, PJ_ICE_SESS_CHECK_STATE_FAILED,
status);
@@ -3003,11 +3466,7 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
@@ -3003,11 +3468,7 @@ static void on_stun_request_complete(pj_stun_session *stun_sess,
/* Perform 7.1.2.2.2. Updating Pair States.
* This may terminate ICE processing.
*/
......@@ -1608,7 +1614,7 @@ index 3fecc3def..a3869d9ca 100644
pj_grp_lock_release(ice->grp_lock);
}
@@ -3202,9 +3661,13 @@ static pj_status_t on_stun_rx_request(pj_stun_session *sess,
@@ -3202,9 +3663,13 @@ static pj_status_t on_stun_rx_request(pj_stun_session *sess,
msg_data->has_req_data = PJ_FALSE;
/* Send the response */
......@@ -1624,7 +1630,7 @@ index 3fecc3def..a3869d9ca 100644
/*
* Handling early check.
@@ -3323,12 +3786,12 @@ static void handle_incoming_check(pj_ice_sess *ice,
@@ -3323,12 +3788,12 @@ static void handle_incoming_check(pj_ice_sess *ice,
/* Just get candidate with the highest priority and same transport ID
* for the specified component ID in the checklist.
*/
......
test/unitTest/ice/ice.cpp
+ 118
− 1
View file @ 3e73e264
......@@ -59,12 +59,14 @@ private:
void testTurnMasterIceConnection();
void testTurnSlaveIceConnection();
void testReceiveTooManyCandidates();
void testCompleteOnFailure();
CPPUNIT_TEST_SUITE(IceTest);
CPPUNIT_TEST(testRawIceConnection);
CPPUNIT_TEST(testTurnMasterIceConnection);
CPPUNIT_TEST(testTurnSlaveIceConnection);
CPPUNIT_TEST(testReceiveTooManyCandidates);
CPPUNIT_TEST(testCompleteOnFailure);
CPPUNIT_TEST_SUITE_END();
};
......@@ -512,6 +514,121 @@ IceTest::testReceiveTooManyCandidates()
cv.wait_for(lk, std::chrono::seconds(10), [&] { return iceMasterReady && iceSlaveReady; }));
}
void
IceTest::testCompleteOnFailure()
{
const auto& addr4 = dht_->getPublicAddress(AF_INET);
CPPUNIT_ASSERT(addr4.size() != 0);
CPPUNIT_ASSERT(turnV4_);
IceTransportOptions ice_config;
ice_config.upnpEnable = true;
ice_config.tcpEnable = true;
std::shared_ptr<IceTransport> ice_master, ice_slave;
std::mutex mtx, mtx_create, mtx_resp, mtx_init;
std::unique_lock<std::mutex> lk {mtx}, lk_create {mtx_create}, lk_resp {mtx_resp},
lk_init {mtx_init};
std::condition_variable cv, cv_create, cv_resp, cv_init;
std::string init = {};
std::string response = {};
bool iceMasterNotReady = false, iceSlaveNotReady = false;
ice_config.onInitDone = [&](bool ok) {
CPPUNIT_ASSERT(ok);
dht::ThreadPool::io().run([&] {
CPPUNIT_ASSERT(cv_create.wait_for(lk_create, std::chrono::seconds(10), [&] {
return ice_master != nullptr;
}));
auto iceAttributes = ice_master->getLocalAttributes();
std::stringstream icemsg;
icemsg << iceAttributes.ufrag << "\n";
icemsg << iceAttributes.pwd << "\n";
for (const auto& addr : ice_master->getLocalCandidates(1)) {
if (addr.find("relay") != std::string::npos) {
// We only want to relayed and modify the rest (to have CONNREFUSED)
icemsg << addr << "\n";
JAMI_DBG() << "Added local ICE candidate " << addr;
} else {
// Replace host by non existing IP (we still need host to not fail the start)
std::regex e("((?:[0-9]{1,3}\\.){3}[0-9]{1,3})");
auto newaddr = std::regex_replace(addr, e, "100.100.100.100");
if (newaddr != addr)
icemsg << newaddr << "\n";
}
}
init = icemsg.str();
cv_init.notify_one();
CPPUNIT_ASSERT(cv_resp.wait_for(lk_resp, std::chrono::seconds(10), [&] {
return !response.empty();
}));
auto sdp = ice_master->parseIceCandidates(response);
CPPUNIT_ASSERT(
ice_master->startIce({sdp.rem_ufrag, sdp.rem_pwd}, std::move(sdp.rem_candidates)));
});
};
ice_config.onNegoDone = [&](bool ok) {
iceMasterNotReady = !ok;
cv.notify_one();
};
ice_config.accountPublicAddr = IpAddr(*addr4[0].get());
ice_config.accountLocalAddr = ip_utils::getLocalAddr(AF_INET);
ice_config.master = true;
ice_config.streamsCount = 1;
ice_config.compCountPerStream = 1;
ice_master = Manager::instance().getIceTransportFactory().createTransport("master ICE",
ice_config);
cv_create.notify_all();
ice_config.onInitDone = [&](bool ok) {
CPPUNIT_ASSERT(ok);
dht::ThreadPool::io().run([&] {
CPPUNIT_ASSERT(cv_create.wait_for(lk_create, std::chrono::seconds(10), [&] {
return ice_slave != nullptr;
}));
auto iceAttributes = ice_slave->getLocalAttributes();
std::stringstream icemsg;
icemsg << iceAttributes.ufrag << "\n";
icemsg << iceAttributes.pwd << "\n";
for (const auto& addr : ice_slave->getLocalCandidates(1)) {
if (addr.find("relay") != std::string::npos) {
// We only want to relayed and modify the rest (to have CONNREFUSED)
icemsg << addr << "\n";
JAMI_DBG() << "Added local ICE candidate " << addr;
} else {
// Replace host by non existing IP (we still need host to not fail the start)
std::regex e("((?:[0-9]{1,3}\\.){3}[0-9]{1,3})");
auto newaddr = std::regex_replace(addr, e, "100.100.100.100");
if (newaddr != addr)
icemsg << newaddr << "\n";
}
}
response = icemsg.str();
cv_resp.notify_one();
CPPUNIT_ASSERT(
cv_init.wait_for(lk_resp, std::chrono::seconds(10), [&] { return !init.empty(); }));
auto sdp = ice_slave->parseIceCandidates(init);
CPPUNIT_ASSERT(
ice_slave->startIce({sdp.rem_ufrag, sdp.rem_pwd}, std::move(sdp.rem_candidates)));
});
};
ice_config.onNegoDone = [&](bool ok) {
iceSlaveNotReady = !ok;
cv.notify_one();
};
ice_config.turnServers.emplace_back(TurnServerInfo()
.setUri(turnV4_->toString(true))
.setUsername("ring")
.setPassword("ring")
.setRealm("ring"));
ice_config.master = false;
ice_config.streamsCount = 1;
ice_config.compCountPerStream = 1;
ice_slave = Manager::instance().getIceTransportFactory().createTransport("slave ICE",
ice_config);
cv_create.notify_all();
// Check that nego failed and callback called
CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(120), [&] {
return iceMasterNotReady && iceSlaveNotReady;
}));
}
} // namespace test
} // namespace jami
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment