From 4fb85d505ca7258a6778b9197a77ee5ddd093f60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Tue, 16 Aug 2022 17:08:25 -0400
Subject: [PATCH] misc: remove hacky underlyingICE()
upper layer should not need to interact with the IceTransport
directly and only uses the API of the socket endpoint.
Change-Id: Iea65b03b37e87e9a7ab542ba88be5a2c6790ae98
GitLab: #703
---
src/account.cpp | 1 -
src/jamidht/channeled_transport.cpp | 17 +++++-----
src/jamidht/channeled_transport.h | 3 --
src/jamidht/connectionmanager.cpp | 1 -
src/jamidht/jamiaccount.cpp | 21 ++++--------
src/jamidht/multiplexed_socket.cpp | 51 ++++++++++++++++++-----------
src/jamidht/multiplexed_socket.h | 10 +++---
src/jamidht/p2p.cpp | 1 -
src/peer_connection.cpp | 34 ++++++++++++++++---
src/peer_connection.h | 5 ++-
src/sip/sipcall.cpp | 1 -
src/sip/siptransport.cpp | 11 +------
12 files changed, 87 insertions(+), 69 deletions(-)
diff --git a/src/account.cpp b/src/account.cpp
index ed13a003f6..088cb452b2 100644
--- a/src/account.cpp
+++ b/src/account.cpp
@@ -46,7 +46,6 @@ using random_device = dht::crypto::random_device;
#include "fileutils.h"
#include "config/yamlparser.h"
#include "system_codec_container.h"
-#include "ice_transport.h"
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
diff --git a/src/jamidht/channeled_transport.cpp b/src/jamidht/channeled_transport.cpp
index 56086335fc..fdfb124d4b 100644
--- a/src/jamidht/channeled_transport.cpp
+++ b/src/jamidht/channeled_transport.cpp
@@ -33,19 +33,18 @@ namespace jami {
namespace tls {
ChanneledSIPTransport::ChanneledSIPTransport(pjsip_endpoint* endpt,
- int tp_type,
const std::shared_ptr<ChannelSocket>& socket,
- const IpAddr& local,
- const IpAddr& remote,
onShutdownCb&& cb)
: socket_(socket)
, shutdownCb_(std::move(cb))
- , local_ {local}
- , remote_ {remote}
, trData_()
, pool_ {nullptr, pj_pool_release}
, rxPool_(nullptr, pj_pool_release)
{
+ local_ = socket->getLocalAddress();
+ remote_ = socket->getRemoteAddress();
+ int tp_type = local_.isIpv6() ? PJSIP_TRANSPORT_TLS6 : PJSIP_TRANSPORT_TLS;
+
JAMI_DBG("ChanneledSIPTransport@%p {tr=%p}", this, &trData_.base);
// Init memory
@@ -70,12 +69,12 @@ ChanneledSIPTransport::ChanneledSIPTransport(pjsip_endpoint* endpt,
if (pj_lock_create_recursive_mutex(pool_.get(), "chan", &base.lock) != PJ_SUCCESS)
throw std::runtime_error("Can't create PJSIP mutex.");
- if (not local) {
- JAMI_ERR("Invalid local address [%s]", local.toString(true).c_str());
+ if (not local_) {
+ JAMI_ERR("Invalid local address");
throw std::runtime_error("Invalid local address");
}
- if (not remote) {
- JAMI_ERR("Invalid remote address [%s]", remote.toString(true).c_str());
+ if (not remote_) {
+ JAMI_ERR("Invalid remote address");
throw std::runtime_error("Invalid remote address");
}
diff --git a/src/jamidht/channeled_transport.h b/src/jamidht/channeled_transport.h
index 868ce40318..88985e75bf 100644
--- a/src/jamidht/channeled_transport.h
+++ b/src/jamidht/channeled_transport.h
@@ -49,10 +49,7 @@ class ChanneledSIPTransport : public AbstractSIPTransport
{
public:
ChanneledSIPTransport(pjsip_endpoint* endpt,
- int tp_type,
const std::shared_ptr<ChannelSocket>& socket,
- const IpAddr& local,
- const IpAddr& remote,
onShutdownCb&& cb);
~ChanneledSIPTransport();
diff --git a/src/jamidht/connectionmanager.cpp b/src/jamidht/connectionmanager.cpp
index aa13ca0987..684b0ccd8f 100644
--- a/src/jamidht/connectionmanager.cpp
+++ b/src/jamidht/connectionmanager.cpp
@@ -20,7 +20,6 @@
#include "account_const.h"
#include "account_manager.h"
#include "manager.h"
-#include "ice_transport.h"
#include "peer_connection.h"
#include "logger.h"
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 47febb32c8..37af0244d2 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -48,8 +48,6 @@
#include "sip/siptransport.h"
#include "sip/sip_utils.h"
-#include "ice_transport.h"
-
#include "p2p.h"
#include "uri.h"
@@ -226,8 +224,6 @@ struct JamiAccount::DiscoveredPeer
std::shared_ptr<Task> cleanupTask;
};
-static constexpr int ICE_COMP_ID_SIP_TRANSPORT {1};
-
static constexpr const char* const RING_URI_PREFIX = "ring:";
static constexpr const char* const JAMI_URI_PREFIX = "jami:";
static constexpr const char* DEFAULT_TURN_SERVER = "turn.jami.net";
@@ -597,8 +593,8 @@ JamiAccount::startOutgoingCall(const std::shared_ptr<SIPCall>& call, const std::
}
auto transport = sipConn.transport;
- auto ice = sipConn.channel->underlyingICE();
- if (!transport or !ice)
+ auto remote_address = sipConn.channel->getRemoteAddress();
+ if (!transport or !remote_address)
continue;
channels.emplace_back(sipConn.channel);
@@ -633,7 +629,6 @@ JamiAccount::startOutgoingCall(const std::shared_ptr<SIPCall>& call, const std::
return true;
});
- auto remote_address = ice->getRemoteAddress(ICE_COMP_ID_SIP_TRANSPORT);
try {
onConnectedOutgoingCall(dev_call, toUri, remote_address);
} catch (const VoipLinkException&) {
@@ -3976,14 +3971,13 @@ JamiAccount::sendSIPMessage(SipConnection& conn,
if (!channel)
throw std::runtime_error(
"A SIP transport exists without Channel, this is a bug. Please report");
- auto ice = channel->underlyingICE();
- if (!ice)
+ auto remote_address = channel->getRemoteAddress();
+ if (!remote_address)
return false;
// Build SIP Message
// "deviceID@IP"
- auto toURI = getToUri(to + "@"
- + ice->getRemoteAddress(ICE_COMP_ID_SIP_TRANSPORT).toString(true));
+ auto toURI = getToUri(fmt::format("{}@{}", to, remote_address.toString(true)));
std::string from = getFromUri();
pjsip_tx_data* tdata;
@@ -4146,10 +4140,9 @@ JamiAccount::cacheSIPConnection(std::shared_ptr<ChannelSocket>&& socket,
return;
pc->setSipTransport(sip_tr, getContactHeader(sip_tr));
pc->setState(Call::ConnectionState::PROGRESSING);
- if (auto ice = socket->underlyingICE()) {
- auto remoted_address = ice->getRemoteAddress(ICE_COMP_ID_SIP_TRANSPORT);
+ if (auto remote_address = socket->getRemoteAddress()) {
try {
- onConnectedOutgoingCall(pc, peerId, remoted_address);
+ onConnectedOutgoingCall(pc, peerId, remote_address);
} catch (const VoipLinkException&) {
// In this case, the main scenario is that SIPStartCall failed because
// the ICE is dead and the TLS session didn't send any packet on that dead
diff --git a/src/jamidht/multiplexed_socket.cpp b/src/jamidht/multiplexed_socket.cpp
index 900400c96d..727a0d2f62 100644
--- a/src/jamidht/multiplexed_socket.cpp
+++ b/src/jamidht/multiplexed_socket.cpp
@@ -640,15 +640,8 @@ void
MultiplexedSocket::onShutdown(OnShutdownCb&& cb)
{
pimpl_->onShutdown_ = std::move(cb);
- if (pimpl_->isShutdown_) {
+ if (pimpl_->isShutdown_)
pimpl_->onShutdown_();
- }
-}
-
-std::shared_ptr<IceTransport>
-MultiplexedSocket::underlyingICE() const
-{
- return pimpl_->endpoint->underlyingICE();
}
void
@@ -662,9 +655,7 @@ MultiplexedSocket::monitor() const
auto now = clock::now();
JAMI_DBG("- Duration: %lu",
std::chrono::duration_cast<std::chrono::milliseconds>(now - pimpl_->start_).count());
- const auto& ice = underlyingICE();
- if (ice)
- JAMI_DBG("\t- Ice connection: %s", ice->link().c_str());
+ pimpl_->endpoint->monitor();
std::lock_guard<std::mutex> lk(pimpl_->socketsMutex);
for (const auto& [_, channel] : pimpl_->sockets) {
if (channel)
@@ -720,6 +711,20 @@ MultiplexedSocket::sendVersion()
{
pimpl_->sendVersion();
}
+
+
+IpAddr
+MultiplexedSocket::getLocalAddress() const
+{
+ return pimpl_->endpoint->getLocalAddress();
+}
+
+IpAddr
+MultiplexedSocket::getRemoteAddress() const
+{
+ return pimpl_->endpoint->getRemoteAddress();
+}
+
#endif
////////////////////////////////////////////////////////////////
@@ -839,14 +844,6 @@ ChannelSocket::onRecv(std::vector<uint8_t>&& pkt)
pimpl_->cv.notify_all();
}
-std::shared_ptr<IceTransport>
-ChannelSocket::underlyingICE() const
-{
- if (auto mtx = pimpl_->endpoint.lock())
- return mtx->underlyingICE();
- return {};
-}
-
#ifdef DRING_TESTABLE
std::shared_ptr<MultiplexedSocket>
ChannelSocket::underlyingSocket() const
@@ -990,4 +987,20 @@ ChannelSocket::peerCertificate() const
return {};
}
+IpAddr
+ChannelSocket::getLocalAddress() const
+{
+ if (auto ep = pimpl_->endpoint.lock())
+ return ep->getLocalAddress();
+ return {};
+}
+
+IpAddr
+ChannelSocket::getRemoteAddress() const
+{
+ if (auto ep = pimpl_->endpoint.lock())
+ return ep->getRemoteAddress();
+ return {};
+}
+
} // namespace jami
diff --git a/src/jamidht/multiplexed_socket.h b/src/jamidht/multiplexed_socket.h
index aa75e32361..99bc200bd4 100644
--- a/src/jamidht/multiplexed_socket.h
+++ b/src/jamidht/multiplexed_socket.h
@@ -118,8 +118,6 @@ public:
*/
void onShutdown(OnShutdownCb&& cb);
- std::shared_ptr<IceTransport> underlyingICE() const;
-
/**
* Get informations from socket (channels opened)
*/
@@ -136,6 +134,9 @@ public:
*/
std::shared_ptr<dht::crypto::Certificate> peerCertificate() const;
+ IpAddr getLocalAddress() const;
+ IpAddr getRemoteAddress() const;
+
#ifdef DRING_TESTABLE
/**
* Check if we can send beacon on the socket
@@ -232,8 +233,6 @@ public:
void onRecv(std::vector<uint8_t>&& pkt);
- std::shared_ptr<IceTransport> underlyingICE() const;
-
/**
* Send a beacon on the socket and close if no response come
* @param timeout
@@ -257,6 +256,9 @@ public:
void removable();
bool isRemovable() const;
+ IpAddr getLocalAddress() const;
+ IpAddr getRemoteAddress() const;
+
private:
class Impl;
std::unique_ptr<Impl> pimpl_;
diff --git a/src/jamidht/p2p.cpp b/src/jamidht/p2p.cpp
index a616f1ba96..776cbf0423 100644
--- a/src/jamidht/p2p.cpp
+++ b/src/jamidht/p2p.cpp
@@ -23,7 +23,6 @@
#include "account_schema.h"
#include "jamiaccount.h"
-#include "ice_transport.h"
#include "ftp_server.h"
#include "manager.h"
#include "peer_connection.h"
diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp
index d0f72a9f7e..9c1f90837b 100644
--- a/src/peer_connection.cpp
+++ b/src/peer_connection.cpp
@@ -51,6 +51,8 @@
#include <sys/time.h>
#endif
+static constexpr int ICE_COMP_ID_SIP_TRANSPORT {1};
+
namespace jami {
int
@@ -238,6 +240,14 @@ public:
tls.reset();
}
+ std::shared_ptr<IceTransport> underlyingICE() const
+ {
+ if (ep_)
+ if (const auto* iceSocket = reinterpret_cast<const IceSocketEndpoint*>(ep_))
+ return iceSocket->underlyingICE();
+ return {};
+ }
+
// TLS callbacks
int verifyCertificate(gnutls_session_t);
void onTlsStateChange(tls::TlsSessionState);
@@ -411,12 +421,26 @@ TlsSocketEndpoint::shutdown()
}
}
-std::shared_ptr<IceTransport>
-TlsSocketEndpoint::underlyingICE() const
+void
+TlsSocketEndpoint::monitor() const
+{
+ if (auto ice = pimpl_->underlyingICE())
+ JAMI_DBG("\t- Ice connection: %s", ice->link().c_str());
+}
+
+IpAddr
+TlsSocketEndpoint::getLocalAddress() const
+{
+ if (auto ice = pimpl_->underlyingICE())
+ return ice->getLocalAddress(ICE_COMP_ID_SIP_TRANSPORT);
+ return {};
+}
+
+IpAddr
+TlsSocketEndpoint::getRemoteAddress() const
{
- if (pimpl_->ep_)
- if (const auto* iceSocket = reinterpret_cast<const IceSocketEndpoint*>(pimpl_->ep_))
- return iceSocket->underlyingICE();
+ if (auto ice = pimpl_->underlyingICE())
+ return ice->getRemoteAddress(ICE_COMP_ID_SIP_TRANSPORT);
return {};
}
diff --git a/src/peer_connection.h b/src/peer_connection.h
index c5f101ce35..72da21fb30 100644
--- a/src/peer_connection.h
+++ b/src/peer_connection.h
@@ -147,7 +147,10 @@ public:
void setOnStateChange(OnStateChangeCb&& cb);
void setOnReady(OnReadyCb&& cb);
- std::shared_ptr<IceTransport> underlyingICE() const;
+ IpAddr getLocalAddress() const;
+ IpAddr getRemoteAddress() const;
+
+ void monitor() const;
private:
class Impl;
diff --git a/src/sip/sipcall.cpp b/src/sip/sipcall.cpp
index 47a74dc8f3..d2025c6bd8 100644
--- a/src/sip/sipcall.cpp
+++ b/src/sip/sipcall.cpp
@@ -40,7 +40,6 @@
#include "jami/call_const.h"
#include "jami/media_const.h"
#include "client/ring_signal.h"
-#include "ice_transport.h"
#include "pjsip-ua/sip_inv.h"
#ifdef ENABLE_PLUGIN
diff --git a/src/sip/siptransport.cpp b/src/sip/siptransport.cpp
index e04c7d79fa..adcf658702 100644
--- a/src/sip/siptransport.cpp
+++ b/src/sip/siptransport.cpp
@@ -22,7 +22,6 @@
#include "siptransport.h"
#include "sip_utils.h"
#include "ip_utils.h"
-#include "ice_transport.h"
#include "security/tls_session.h"
#include "jamidht/abstract_sip_transport.h"
@@ -61,7 +60,6 @@ constexpr const char* TRANSPORT_STATE_STR[] = {"CONNECTED",
"DESTROY",
"UNKNOWN STATE"};
constexpr const size_t TRANSPORT_STATE_SZ = std::size(TRANSPORT_STATE_STR);
-static constexpr int ICE_COMP_ID_SIP_TRANSP {1};
void
SipTransport::deleteTransport(pjsip_transport* t)
@@ -405,17 +403,10 @@ SipTransportBroker::getChanneledTransport(const std::shared_ptr<SIPAccountBase>&
const std::shared_ptr<ChannelSocket>& socket,
onShutdownCb&& cb)
{
- auto ice = socket->underlyingICE();
- if (!ice)
+ if (!socket)
return {};
- auto local = ice->getLocalAddress(ICE_COMP_ID_SIP_TRANSP);
- auto remote = ice->getRemoteAddress(ICE_COMP_ID_SIP_TRANSP);
- auto type = local.isIpv6() ? PJSIP_TRANSPORT_TLS6 : PJSIP_TRANSPORT_TLS;
auto sips_tr = std::make_unique<tls::ChanneledSIPTransport>(endpt_,
- type,
socket,
- local,
- remote,
std::move(cb));
auto tr = sips_tr->getTransportBase();
auto sip_tr = std::make_shared<SipTransport>(tr, socket->peerCertificate());
--
GitLab