Code review cleanup

CMakeLists.txt

@@ -125,7 +125,7 @@ set(LIBDATACHANNEL_IMPL_SOURCES
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/sha.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/pollinterrupter.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/pollservice.cpp
-	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/tcpproxytransport.cpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/httpproxytransport.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/tcpserver.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/tcptransport.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/tlstransport.cpp
@@ -158,7 +158,7 @@ set(LIBDATACHANNEL_IMPL_HEADERS
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/sha.hpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/pollinterrupter.hpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/pollservice.hpp
-	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/tcpproxytransport.hpp
+	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/httpproxytransport.hpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/tcpserver.hpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/tcptransport.hpp
 	${CMAKE_CURRENT_SOURCE_DIR}/src/impl/tlstransport.hpp

src/impl/tcpproxytransport.cpp → src/impl/httpproxytransport.cpp

@@ -7,7 +7,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-#include "tcpproxytransport.hpp"
+#include "httpproxytransport.hpp"
 #include "tcptransport.hpp"
 #include "utils.hpp"
 
@@ -18,41 +18,43 @@ namespace rtc::impl {
 using std::to_string;
 using std::chrono::system_clock;
 
-TcpProxyTransport::TcpProxyTransport(shared_ptr<TcpTransport> lower, std::string hostname, std::string service, state_callback stateCallback)
+HttpProxyTransport::HttpProxyTransport(shared_ptr<TcpTransport> lower, std::string hostname, std::string service, state_callback stateCallback)
     : Transport(lower, std::move(stateCallback))
-	, mIsActive( lower->isActive() )
 	, mHostname( std::move(hostname) )
 	, mService( std::move(service) )
 {
-	PLOG_DEBUG << "Initializing TCP Proxy transport";
+	if (!lower->isActive())
+		throw std::logic_error("Http proxy creation failed, expects lower transport to be active");
+
+	PLOG_DEBUG << "Initializing http Proxy transport";
 }
 
-TcpProxyTransport::~TcpProxyTransport() { unregisterIncoming(); }
+HttpProxyTransport::~HttpProxyTransport() { unregisterIncoming(); }
 
-void TcpProxyTransport::start() {
+void HttpProxyTransport::start() {
 	registerIncoming();
 
 	changeState(State::Connecting);
 	sendHttpRequest();
 }
 
-void TcpProxyTransport::stop() {
+void HttpProxyTransport::stop() {
 	unregisterIncoming();
 }
 
-bool TcpProxyTransport::send(message_ptr message) {
+bool HttpProxyTransport::send(message_ptr message) {
 	std::lock_guard lock(mSendMutex);
 
 	if (state() != State::Connected)
-		throw std::runtime_error("Tcp proxy connection is not open");
+		throw std::runtime_error("Http proxy connection is not open");
 
 	PLOG_VERBOSE << "Send size=" << message->size();
 	return outgoing(message);
 }
 
-bool TcpProxyTransport::isActive() const { return mIsActive; }
+bool HttpProxyTransport::isActive() const { return true; }
 
-void TcpProxyTransport::incoming(message_ptr message) {
+void HttpProxyTransport::incoming(message_ptr message) {
 	auto s = state();
 	if (s != State::Connecting && s != State::Connected)
 		return; // Drop
@@ -61,13 +63,18 @@ void TcpProxyTransport::incoming(message_ptr message) {
 		PLOG_VERBOSE << "Incoming size=" << message->size();
 
 		try {
-			mBuffer.insert(mBuffer.end(), message->begin(), message->end());
-
 			if (state() == State::Connecting) {
+				mBuffer.insert(mBuffer.end(), message->begin(), message->end());
 				if (size_t len = parseHttpResponse(mBuffer.data(), mBuffer.size())) {
-					PLOG_INFO << "Tcp proxy connection open";
+					PLOG_INFO << "Http proxy connection open";
 					changeState(State::Connected);
 					mBuffer.erase(mBuffer.begin(), mBuffer.begin() + len);
+
+					if( !mBuffer.empty() )
+					{
+						recv(make_message(mBuffer));
+						mBuffer.clear();
+					}
 				}
 			}
 			else if (state() == State::Connected)
@@ -82,24 +89,24 @@ void TcpProxyTransport::incoming(message_ptr message) {
 	}
 
 	if (state() == State::Connected) {
-		PLOG_INFO << "TCP Proxy disconnected";
+		PLOG_INFO << "Http Proxy disconnected";
 		changeState(State::Disconnected);
 		recv(nullptr);
 	} else {
-		PLOG_ERROR << "TCP Proxy failed";
+		PLOG_ERROR << "Http Proxy failed";
 		changeState(State::Failed);
 	}
 }
 
-bool TcpProxyTransport::sendHttpRequest() {
-	PLOG_DEBUG << "Sending TcpProxy HTTP request";
+bool HttpProxyTransport::sendHttpRequest() {
+	PLOG_DEBUG << "Sending proxy http request";
 
 	const string request = generateHttpRequest();
 	auto data = reinterpret_cast<const byte *>(request.data());
 	return outgoing(make_message(data, data + request.size()));
 }
 
-std::string TcpProxyTransport::generateHttpRequest()
+std::string HttpProxyTransport::generateHttpRequest()
 {
 	std::string out =
 		"CONNECT " +
@@ -109,7 +116,7 @@ std::string TcpProxyTransport::generateHttpRequest()
 	return out;
 }
 
-size_t TcpProxyTransport::parseHttpResponse( std::byte* buffer, size_t size )
+size_t HttpProxyTransport::parseHttpResponse( std::byte* buffer, size_t size )
 {
 	std::list<string> lines;
 	size_t length = utils::parseHttpLines(buffer, size, lines);
@@ -117,7 +124,7 @@ size_t TcpProxyTransport::parseHttpResponse( std::byte* buffer, size_t size )
 		return 0;
 
 	if (lines.empty())
-		throw std::runtime_error("Invalid HTTP request for Tcp Proxy");
+		throw std::runtime_error("Invalid http request for proxy");
 
 	std::istringstream status(std::move(lines.front()));
 	lines.pop_front();
@@ -127,7 +134,7 @@ size_t TcpProxyTransport::parseHttpResponse( std::byte* buffer, size_t size )
 	status >> protocol >> code;
 
 	if (code != 200)
-		throw std::runtime_error("Unexpected response code " + to_string(code) + " for Tcp Proxy");
+		throw std::runtime_error("Unexpected response code " + to_string(code) + " for proxy");
 
 	return length;
 }

src/impl/tcpproxytransport.hpp → src/impl/httpproxytransport.hpp

@@ -19,11 +19,11 @@ namespace rtc::impl {
 
 class TcpTransport;
 
-class TcpProxyTransport final : public Transport, public std::enable_shared_from_this<TcpProxyTransport> {
+class HttpProxyTransport final : public Transport, public std::enable_shared_from_this<HttpProxyTransport> {
 public:
-	TcpProxyTransport(shared_ptr<TcpTransport> lower, std::string hostname, std::string service,
+	HttpProxyTransport(shared_ptr<TcpTransport> lower, std::string hostname, std::string service,
 				state_callback stateCallback);
-	~TcpProxyTransport();
+	~HttpProxyTransport();
 
 	void start() override;
 	void stop() override;
@@ -37,7 +37,6 @@ private:
 	std::string generateHttpRequest();
 	size_t parseHttpResponse( std::byte* buffer, size_t size );
 
-	const bool mIsActive;
 	std::string mHostname;
 	std::string mService;
 	binary mBuffer;

src/impl/tlstransport.cpp

@@ -8,7 +8,7 @@
 
 #include "tlstransport.hpp"
 #include "tcptransport.hpp"
-#include "tcpproxytransport.hpp"
+#include "httpproxytransport.hpp"
 #include "threadpool.hpp"
 
 #if RTC_ENABLE_WEBSOCKET
@@ -59,14 +59,11 @@ void TlsTransport::Cleanup() {
 	// Nothing to do
 }
 
-TlsTransport::TlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<TcpProxyTransport>> lower, optional<string> host,
+TlsTransport::TlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<HttpProxyTransport>> lower, optional<string> host,
                            certificate_ptr certificate, state_callback callback)
     : Transport(std::visit([](auto l) { return std::static_pointer_cast<Transport>(l); }, lower),
 			    std::move(callback)), mHost(std::move(host))
-	, mIsClient(
-		std::visit(rtc::overloaded{[](shared_ptr<TcpTransport> l) { return l->isActive(); },
-                                   [](shared_ptr<TcpProxyTransport> l) { return l->isActive(); }},
-                   lower))
+	, mIsClient(std::visit([](auto l) { return l->isActive(); }, lower))
 	, mIncomingQueue(RECV_QUEUE_LIMIT, message_size_func) {
 
 	PLOG_DEBUG << "Initializing TLS transport (GnuTLS)";
@@ -314,14 +311,11 @@ void TlsTransport::Cleanup() {
 	// Nothing to do
 }
 
-TlsTransport::TlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<TcpProxyTransport>> lower, optional<string> host,
+TlsTransport::TlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<HttpProxyTransport>> lower, optional<string> host,
                            certificate_ptr certificate, state_callback callback)
     : Transport(std::visit([](auto l) { return std::static_pointer_cast<Transport>(l); }, lower),
 				std::move(callback)), mHost(std::move(host))
-	, mIsClient(
-          std::visit(rtc::overloaded{[](shared_ptr<TcpTransport> l) { return l->isActive(); },
-                                     [](shared_ptr<TcpProxyTransport> l) { return l->isActive(); }},
-                     lower))
+	, mIsClient(std::visit([](auto l) { return l->isActive(); }, lower))
 	, mIncomingQueue(RECV_QUEUE_LIMIT, message_size_func) {
 
 	PLOG_DEBUG << "Initializing TLS transport (OpenSSL)";

src/impl/tlstransport.hpp

@@ -23,14 +23,14 @@
 namespace rtc::impl {
 
 class TcpTransport;
-class TcpProxyTransport;
+class HttpProxyTransport;
 
 class TlsTransport : public Transport, public std::enable_shared_from_this<TlsTransport> {
 public:
 	static void Init();
 	static void Cleanup();
 
-	TlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<TcpProxyTransport>> lower, optional<string> host, certificate_ptr certificate,
+	TlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<HttpProxyTransport>> lower, optional<string> host, certificate_ptr certificate,
 	             state_callback callback);
 	virtual ~TlsTransport();
 

src/impl/utils.hpp

@@ -34,7 +34,7 @@ string base64_encode(const binary &data);
 // Return a random seed sequence
 std::seed_seq random_seed();
 
-// Parse an http packet into lines
+// Parse an http message into lines
 size_t parseHttpLines(const byte *buffer, size_t size, std::list<string> &lines);
 
 // Parse headers of a http message

src/impl/verifiedtlstransport.cpp

@@ -13,7 +13,7 @@
 
 namespace rtc::impl {
 
-VerifiedTlsTransport::VerifiedTlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<TcpProxyTransport>> lower, string host,
+VerifiedTlsTransport::VerifiedTlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<HttpProxyTransport>> lower, string host,
                                            certificate_ptr certificate, state_callback callback)
     : TlsTransport(std::move(lower), std::move(host), std::move(certificate), std::move(callback)) {
 

src/impl/verifiedtlstransport.hpp

@@ -17,7 +17,7 @@ namespace rtc::impl {
 
 class VerifiedTlsTransport final : public TlsTransport {
 public:
-	VerifiedTlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<TcpProxyTransport>> lower, string host, certificate_ptr certificate,
+	VerifiedTlsTransport(variant<shared_ptr<TcpTransport>, shared_ptr<HttpProxyTransport>> lower, string host, certificate_ptr certificate,
 	                     state_callback callback);
 	~VerifiedTlsTransport();
 };

src/impl/websocket.cpp

@@ -15,7 +15,7 @@
 #include "utils.hpp"
 
 #include "tcptransport.hpp"
-#include "tcpproxytransport.hpp"
+#include "httpproxytransport.hpp"
 #include "tlstransport.hpp"
 #include "verifiedtlstransport.hpp"
 #include "wstransport.hpp"
@@ -48,11 +48,6 @@ void WebSocket::open(const string &url) {
 	if (state != State::Closed)
 		throw std::logic_error("WebSocket must be closed before opening");
 
-	if (config.proxyServer) {
-		mIsProxied = true;
-		mProxy = config.proxyServer;
-	}
-
 	// Modified regex from RFC 3986, see https://www.rfc-editor.org/rfc/rfc3986.html#appendix-B
 	static const char *rs =
 	    R"(^(([^:.@/?#]+):)?(/{0,2}((([^:@]*)(:([^@]*))?)@)?(([^:/?#]*)(:([^/?#]*))?))?([^?#]*)(\?([^#]*))?(#(.*))?)";
@@ -109,9 +104,9 @@ void WebSocket::open(const string &url) {
 
 	changeState(State::Connecting);
 
-	if (mIsProxied)
+	if (config.proxyServer)
 	{
-		setTcpTransport(std::make_shared<TcpTransport>(mProxy.value().hostname, std::to_string(mProxy.value().port), nullptr));
+		setTcpTransport(std::make_shared<TcpTransport>(config.proxyServer->hostname, std::to_string(config.proxyServer->port), nullptr));
 	}
 	else
 	{
@@ -229,7 +224,7 @@ shared_ptr<TcpTransport> WebSocket::setTcpTransport(shared_ptr<TcpTransport> tra
 				return;
 			switch (transportState) {
 			case State::Connected:
-				if (mIsProxied)
+				if (config.proxyServer)
 					initProxyTransport();
 				else if (mIsSecure)
 					initTlsTransport();
@@ -263,9 +258,9 @@ shared_ptr<TcpTransport> WebSocket::setTcpTransport(shared_ptr<TcpTransport> tra
 	}
 }
 
-shared_ptr<TcpProxyTransport> WebSocket::initProxyTransport() {
+shared_ptr<HttpProxyTransport> WebSocket::initProxyTransport() {
 	PLOG_VERBOSE << "Starting Tcp Proxy transport";
-	using State = TcpProxyTransport::State;
+	using State = HttpProxyTransport::State;
 	try {
 		if (auto transport = std::atomic_load(&mProxyTransport))
 			return transport;
@@ -298,7 +293,7 @@ shared_ptr<TcpProxyTransport> WebSocket::initProxyTransport() {
 			}
 		};
 
-		auto transport = std::make_shared<TcpProxyTransport>( lower, mHostname.value(), mService.value(), stateChangeCallback );
+		auto transport = std::make_shared<HttpProxyTransport>( lower, mHostname.value(), mService.value(), stateChangeCallback );
 
 		return emplaceTransport(this, &mProxyTransport, std::move(transport));
 
@@ -316,17 +311,17 @@ shared_ptr<TlsTransport> WebSocket::initTlsTransport() {
 		if (auto transport = std::atomic_load(&mTlsTransport))
 			return transport;
 
-		variant<shared_ptr<TcpTransport>, shared_ptr<TcpProxyTransport>> lower;
-		if (mIsProxied) {
+		variant<shared_ptr<TcpTransport>, shared_ptr<HttpProxyTransport>> lower;
+		if (config.proxyServer) {
 			auto transport = std::atomic_load(&mProxyTransport);
 			if (!transport)
-				throw std::logic_error("No underlying TLS transport for WebSocket transport");
+				throw std::logic_error("No underlying proxy transport for TLS transport");
 
 			lower = transport;
 		} else {
 			auto transport = std::atomic_load(&mTcpTransport);
 			if (!transport)
-				throw std::logic_error("No underlying TCP transport for WebSocket transport");
+				throw std::logic_error("No underlying TCP transport for TLS transport");
 
 			lower = transport;
 		}

src/impl/websocket.hpp

@@ -17,7 +17,7 @@
 #include "message.hpp"
 #include "queue.hpp"
 #include "tcptransport.hpp"
-#include "tcpproxytransport.hpp"
+#include "httpproxytransport.hpp"
 #include "tlstransport.hpp"
 #include "wstransport.hpp"
 
@@ -52,7 +52,7 @@ struct WebSocket final : public Channel, public std::enable_shared_from_this<Web
 	bool changeState(State state);
 
 	shared_ptr<TcpTransport> setTcpTransport(shared_ptr<TcpTransport> transport);
-	shared_ptr<TcpProxyTransport> initProxyTransport();
+	shared_ptr<HttpProxyTransport> initProxyTransport();
 	shared_ptr<TlsTransport> initTlsTransport();
 	shared_ptr<WsTransport> initWsTransport();
 	shared_ptr<TcpTransport> getTcpTransport() const;
@@ -71,14 +71,12 @@ private:
 
 	const certificate_ptr mCertificate;
 	bool mIsSecure;
-	bool mIsProxied{false};
 
-	optional<ProxyServer> mProxy;
 	optional<string> mHostname; // for TLS SNI and Proxy
 	optional<string> mService; // for Proxy
 
 	shared_ptr<TcpTransport> mTcpTransport;
-	shared_ptr<TcpProxyTransport> mProxyTransport;
+	shared_ptr<HttpProxyTransport> mProxyTransport;
 	shared_ptr<TlsTransport> mTlsTransport;
 	shared_ptr<WsTransport> mWsTransport;
 	shared_ptr<WsHandshake> mWsHandshake;

src/impl/wshandshake.hpp

@@ -13,8 +13,6 @@
 
 #if RTC_ENABLE_WEBSOCKET
 
-#include <list>
-#include <map>
 #include <vector>
 
 namespace rtc::impl {