Fixed ICE candidates parsing

src/candidate.cpp

@@ -19,12 +19,14 @@
 #include "candidate.hpp"
 
 #include <algorithm>
+#include <array>
 #include <sstream>
 
 #include <netdb.h>
 #include <sys/socket.h>
 #include <sys/types.h>
 
+using std::array;
 using std::string;
 
 namespace {
@@ -39,9 +41,12 @@ inline bool hasprefix(const string &str, const string &prefix) {
 namespace rtc {
 
 Candidate::Candidate(string candidate, std::optional<string> mid) {
-	const string prefix{"candidate:"};
-	mCandidate =
-	    hasprefix(candidate, prefix) ? candidate.substr(prefix.size()) : std::move(candidate);
+	const std::array prefixes{"a=", "candidate:"};
+	for (string prefix : prefixes)
+		if (hasprefix(candidate, prefix))
+			candidate.erase(0, prefix.size());
+
+	mCandidate = std::move(candidate);
 	mMid = std::move(mid);
 
 	// See RFC 5245 for format
@@ -68,14 +73,15 @@ Candidate::Candidate(string candidate, std::optional<string> mid) {
 					char servbuffer[MAX_NUMERICSERV_LEN];
 					if (getnameinfo(p->ai_addr, p->ai_addrlen, nodebuffer, MAX_NUMERICNODE_LEN,
 					                servbuffer, MAX_NUMERICSERV_LEN,
-					                NI_NUMERICHOST | NI_NUMERICHOST) == 0) {
+					                NI_NUMERICHOST | NI_NUMERICSERV) == 0) {
 						string left;
 						std::getline(ss, left);
 						const char sp{' '};
 						ss.clear();
 						ss << foundation << sp << component << sp << transport << sp << priority;
 						ss << sp << nodebuffer << sp << servbuffer << sp << "typ" << sp << type;
-						ss << sp << left;
+						if (!left.empty())
+							ss << sp << left;
 						mCandidate = ss.str();
 						break;
 					}
@@ -90,7 +96,11 @@ string Candidate::candidate() const { return mCandidate; }
 
 std::optional<string> Candidate::mid() const { return mMid; }
 
-Candidate::operator string() const { return mCandidate; }
+Candidate::operator string() const {
+	std::ostringstream line;
+	line << "a=candidate:" << mCandidate;
+	return line.str();
+}
 
 } // namespace rtc
 

src/description.cpp

@@ -131,7 +131,7 @@ Description::operator string() const {
 		sdp << "a=sctp-port:" << *mSctpPort << "\n";
 
 	for (const auto &candidate : mCandidates) {
-		sdp << "a=candidate:" << string(candidate);
+		sdp << string(candidate) << "\n";
 	}
 
 	return sdp.str();

src/icetransport.cpp

@@ -33,9 +33,9 @@ namespace rtc {
 using std::shared_ptr;
 using std::weak_ptr;
 
-IceTransport::IceTransport(const IceConfiguration &config, candidate_callback candidateCallback,
-                           ready_callback ready)
-    : mRole(Description::Role::ActPass), mState(State::Disconnected), mNiceAgent(nullptr, nullptr),
+IceTransport::IceTransport(const IceConfiguration &config, Description::Role role,
+                           candidate_callback candidateCallback, ready_callback ready)
+    : mRole(role), mState(State::Disconnected), mNiceAgent(nullptr, nullptr),
       mMainLoop(nullptr, nullptr), mCandidateCallback(std::move(candidateCallback)),
       mReadyCallback(ready) {
 
@@ -141,17 +141,20 @@ void IceTransport::setRemoteDescription(const Description &description) {
 	                                                        : Description::Role::Active;
 
 	if (nice_agent_parse_remote_sdp(mNiceAgent.get(), string(description).c_str()))
-		throw std::runtime_error("Unable to parse remote SDP");
+		throw std::runtime_error("Failed to parse remote SDP");
 }
 
 void IceTransport::gatherLocalCandidates() {
 	if (!nice_agent_gather_candidates(mNiceAgent.get(), mStreamId))
-		throw std::runtime_error("Unable to gather local ICE candidates");
+		throw std::runtime_error("Failed to gather local ICE candidates");
 }
 
 bool IceTransport::addRemoteCandidate(const Candidate &candidate) {
-	NiceCandidate *cand = nice_agent_parse_remote_candidate_sdp(mNiceAgent.get(), mStreamId,
-	                                                            string(candidate).c_str());
+	// Warning: the candidate string must start with "a=candidate:" and it must not end with a
+	// newline, else libnice will reject it.
+	string sdp(candidate);
+	NiceCandidate *cand =
+	    nice_agent_parse_remote_candidate_sdp(mNiceAgent.get(), mStreamId, sdp.c_str());
 	if (!cand)
 		return false;
 

src/icetransport.hpp

@@ -50,8 +50,8 @@ public:
 	using candidate_callback = std::function<void(const std::optional<Candidate> &candidate)>;
 	using ready_callback = std::function<void(void)>;
 
-	IceTransport(const IceConfiguration &config, candidate_callback candidateCallback,
-	             ready_callback ready);
+	IceTransport(const IceConfiguration &config, Description::Role role,
+	             candidate_callback candidateCallback, ready_callback ready);
 	~IceTransport();
 
 	Description::Role role() const;

src/peerconnection.cpp

@@ -21,6 +21,8 @@
 #include "icetransport.hpp"
 #include "sctptransport.hpp"
 
+#include <iostream>
+
 namespace rtc {
 
 using namespace std::placeholders;
@@ -43,7 +45,7 @@ std::optional<Description> PeerConnection::remoteDescription() const { return mR
 
 void PeerConnection::setRemoteDescription(Description description) {
 	if (!mIceTransport) {
-		initIceTransport();
+		initIceTransport(Description::Role::ActPass);
 		mIceTransport->setRemoteDescription(description);
 		processLocalDescription(mIceTransport->getLocalDescription());
 		mIceTransport->gatherLocalCandidates();
@@ -58,8 +60,10 @@ void PeerConnection::setRemoteCandidate(Candidate candidate) {
 	if (!mRemoteDescription || !mIceTransport)
 		throw std::logic_error("Remote candidate set without remote description");
 
-	mIceTransport->addRemoteCandidate(candidate);
-	mRemoteDescription->addCandidate(std::move(candidate));
+	if (mIceTransport->addRemoteCandidate(candidate))
+		mRemoteDescription->addCandidate(std::move(candidate));
+	else
+		std::cerr << "Failed to add remote ICE candidate" << std::endl;
 }
 
 shared_ptr<DataChannel> PeerConnection::createDataChannel(const string &label,
@@ -80,7 +84,7 @@ shared_ptr<DataChannel> PeerConnection::createDataChannel(const string &label,
 	mDataChannels.insert(std::make_pair(stream, channel));
 
 	if (!mIceTransport) {
-		initIceTransport();
+		initIceTransport(Description::Role::Active);
 		processLocalDescription(mIceTransport->getLocalDescription());
 		mIceTransport->gatherLocalCandidates();
 	} else if (mSctpTransport && mSctpTransport->isReady()) {
@@ -104,9 +108,9 @@ void PeerConnection::onLocalCandidate(
 	mLocalCandidateCallback = callback;
 }
 
-void PeerConnection::initIceTransport() {
+void PeerConnection::initIceTransport(Description::Role role) {
 	mIceTransport = std::make_shared<IceTransport>(
-	    mConfig, std::bind(&PeerConnection::processLocalCandidate, this, _1),
+	    mConfig, role, std::bind(&PeerConnection::processLocalCandidate, this, _1),
 	    std::bind(&PeerConnection::initDtlsTransport, this));
 }
 

src/peerconnection.hpp

@@ -62,7 +62,7 @@ public:
 	void onLocalCandidate(std::function<void(const std::optional<Candidate> &candidate)> callback);
 
 private:
-	void initIceTransport();
+	void initIceTransport(Description::Role role);
 	void initDtlsTransport();
 	void initSctpTransport();