remove all but 2 redundant factory methods in TurnPort

and those will be fixed after I fixed downstream.

Bug: webrtc:10335
Change-Id: Ie824b422b4240fbcdb5d7ee40ae9be91655abae7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256111
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36274}

p2p/base/port_unittest.cc

@@ -556,11 +556,21 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> {
       ProtocolType int_proto,
       ProtocolType ext_proto,
       const rtc::SocketAddress& server_addr) {
-    return TurnPort::Create(&main_, socket_factory, MakeNetwork(addr), 0, 0,
+    RelayServerConfig config;
-                            username_, password_,
+    config.credentials = kRelayCredentials;
-                            ProtocolAddress(server_addr, int_proto),
+    ProtocolAddress server_address(server_addr, int_proto);
-                            kRelayCredentials, 0, {}, {}, nullptr, nullptr);
+    CreateRelayPortArgs args;
+    args.network_thread = &main_;
+    args.socket_factory = socket_factory;
+    args.network = MakeNetwork(addr);
+    args.username = username_;
+    args.password = password_;
+    args.server_address = &server_address;
+    args.config = &config;
+
+    return TurnPort::Create(args, 0, 0);
   }
+
   std::unique_ptr<rtc::NATServer> CreateNatServer(const SocketAddress& addr,
                                                   rtc::NATType type) {
     return std::make_unique<rtc::NATServer>(type, ss_.get(), addr, addr,

p2p/base/turn_port.cc

@@ -222,11 +222,16 @@ TurnPort::TurnPort(rtc::Thread* thread,
                    const ProtocolAddress& server_address,
                    const RelayCredentials& credentials,
                    int server_priority,
+                   const std::vector<std::string>& tls_alpn_protocols,
+                   const std::vector<std::string>& tls_elliptic_curves,
                    webrtc::TurnCustomizer* customizer,
+                   rtc::SSLCertificateVerifier* tls_cert_verifier,
                    const webrtc::WebRtcKeyValueConfig* field_trials)
     : Port(thread, RELAY_PORT_TYPE, factory, network, username, password),
       server_address_(server_address),
-      tls_cert_verifier_(nullptr),
+      tls_alpn_protocols_(tls_alpn_protocols),
+      tls_elliptic_curves_(tls_elliptic_curves),
+      tls_cert_verifier_(tls_cert_verifier),
       credentials_(credentials),
       socket_(socket),
       error_(0),

p2p/base/turn_port.h

@@ -52,7 +52,60 @@ class TurnPort : public Port {
                          // packets.
   };
 
+  static bool Validate(const CreateRelayPortArgs& args) {
+    // Do basic parameter validation.
+    if (args.config->credentials.username.size() > kMaxTurnUsernameLength) {
+      RTC_LOG(LS_ERROR) << "Attempt to use TURN with a too long username "
+                        << "of length "
+                        << args.config->credentials.username.size();
+      return false;
+    }
+    // Do not connect to low-numbered ports. The default STUN port is 3478.
+    if (!AllowedTurnPort(args.server_address->address.port(),
+                         args.field_trials)) {
+      RTC_LOG(LS_ERROR) << "Attempt to use TURN to connect to port "
+                        << args.server_address->address.port();
+      return false;
+    }
+    return true;
+  }
+
   // Create a TURN port using the shared UDP socket, `socket`.
+  static std::unique_ptr<TurnPort> Create(const CreateRelayPortArgs& args,
+                                          rtc::AsyncPacketSocket* socket) {
+    if (!Validate(args)) {
+      return nullptr;
+    }
+    // Using `new` to access a non-public constructor.
+    return absl::WrapUnique(
+        new TurnPort(args.network_thread, args.socket_factory, args.network,
+                     socket, args.username, args.password, *args.server_address,
+                     args.config->credentials, args.config->priority,
+                     args.config->tls_alpn_protocols,
+                     args.config->tls_elliptic_curves, args.turn_customizer,
+                     args.config->tls_cert_verifier, args.field_trials));
+  }
+
+  // Create a TURN port that will use a new socket, bound to `network` and
+  // using a port in the range between `min_port` and `max_port`.
+  static std::unique_ptr<TurnPort> Create(const CreateRelayPortArgs& args,
+                                          int min_port,
+                                          int max_port) {
+    if (!Validate(args)) {
+      return nullptr;
+    }
+    // Using `new` to access a non-public constructor.
+    return absl::WrapUnique(
+        new TurnPort(args.network_thread, args.socket_factory, args.network,
+                     min_port, max_port, args.username, args.password,
+                     *args.server_address, args.config->credentials,
+                     args.config->priority, args.config->tls_alpn_protocols,
+                     args.config->tls_elliptic_curves, args.turn_customizer,
+                     args.config->tls_cert_verifier, args.field_trials));
+  }
+
+  // Create a TURN port using the shared UDP socket, `socket`.
+  // TODO(jonaso) : remove in favor or CreateRelayPortArgs version.
   static std::unique_ptr<TurnPort> Create(
       rtc::Thread* thread,
       rtc::PacketSocketFactory* factory,
@@ -75,32 +128,15 @@ class TurnPort : public Port {
     if (!AllowedTurnPort(server_address.address.port(), field_trials)) {
       RTC_LOG(LS_ERROR) << "Attempt to use TURN to connect to port "
                         << server_address.address.port();
-      return nullptr;
     }
     // Using `new` to access a non-public constructor.
-    return absl::WrapUnique(new TurnPort(
+    return absl::WrapUnique(new TurnPort(thread, factory, network, socket,
-        thread, factory, network, socket, username, password, server_address,
+                                         username, password, server_address,
-        credentials, server_priority, customizer, field_trials));
+                                         credentials, server_priority, {}, {},
-  }
+                                         customizer, nullptr, field_trials));
-
-  // TODO(steveanton): Remove once downstream clients have moved to `Create`.
-  static std::unique_ptr<TurnPort> CreateUnique(
-      rtc::Thread* thread,
-      rtc::PacketSocketFactory* factory,
-      rtc::Network* network,
-      rtc::AsyncPacketSocket* socket,
-      const std::string& username,  // ice username.
-      const std::string& password,  // ice password.
-      const ProtocolAddress& server_address,
-      const RelayCredentials& credentials,
-      int server_priority,
-      webrtc::TurnCustomizer* customizer,
-      const webrtc::WebRtcKeyValueConfig* field_trials) {
-    return Create(thread, factory, network, socket, username, password,
-                  server_address, credentials, server_priority, customizer,
-                  field_trials);
   }
 
+  // TODO(jonaso) : remove in favor or CreateRelayPortArgs version.
   // Create a TURN port that will use a new socket, bound to `network` and
   // using a port in the range between `min_port` and `max_port`.
   static std::unique_ptr<TurnPort> Create(
@@ -138,29 +174,6 @@ class TurnPort : public Port {
         tls_elliptic_curves, customizer, tls_cert_verifier, field_trials));
   }
 
-  // TODO(steveanton): Remove once downstream clients have moved to `Create`.
-  static std::unique_ptr<TurnPort> CreateUnique(
-      rtc::Thread* thread,
-      rtc::PacketSocketFactory* factory,
-      rtc::Network* network,
-      uint16_t min_port,
-      uint16_t max_port,
-      const std::string& username,  // ice username.
-      const std::string& password,  // ice password.
-      const ProtocolAddress& server_address,
-      const RelayCredentials& credentials,
-      int server_priority,
-      const std::vector<std::string>& tls_alpn_protocols,
-      const std::vector<std::string>& tls_elliptic_curves,
-      webrtc::TurnCustomizer* customizer,
-      rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr,
-      const webrtc::WebRtcKeyValueConfig* field_trials = nullptr) {
-    return Create(thread, factory, network, min_port, max_port, username,
-                  password, server_address, credentials, server_priority,
-                  tls_alpn_protocols, tls_elliptic_curves, customizer,
-                  tls_cert_verifier, field_trials);
-  }
-
   ~TurnPort() override;
 
   const ProtocolAddress& server_address() const { return server_address_; }
@@ -269,7 +282,10 @@ class TurnPort : public Port {
            const ProtocolAddress& server_address,
            const RelayCredentials& credentials,
            int server_priority,
+           const std::vector<std::string>& tls_alpn_protocols,
+           const std::vector<std::string>& tls_elliptic_curves,
            webrtc::TurnCustomizer* customizer,
+           rtc::SSLCertificateVerifier* tls_cert_verifier = nullptr,
            const webrtc::WebRtcKeyValueConfig* field_trials = nullptr);
 
   TurnPort(rtc::Thread* thread,

p2p/base/turn_port_unittest.cc

@@ -292,11 +292,20 @@ class TurnPortTest : public ::testing::Test,
                                    const std::string& username,
                                    const std::string& password,
                                    const ProtocolAddress& server_address) {
-    RelayCredentials credentials(username, password);
+    RelayServerConfig config;
-    turn_port_ =
+    config.credentials = RelayCredentials(username, password);
-        TurnPort::Create(&main_, &socket_factory_, network, 0, 0, kIceUfrag1,
+    CreateRelayPortArgs args;
-                         kIcePwd1, server_address, credentials, 0, {}, {},
+    args.network_thread = &main_;
-                         turn_customizer_.get(), nullptr, &field_trials_);
+    args.socket_factory = &socket_factory_;
+    args.network = network;
+    args.username = kIceUfrag1;
+    args.password = kIcePwd1;
+    args.server_address = &server_address;
+    args.config = &config;
+    args.turn_customizer = turn_customizer_.get();
+    args.field_trials = &field_trials_;
+
+    turn_port_ = TurnPort::Create(args, 0, 0);
     if (!turn_port_) {
       return false;
     }
@@ -327,11 +336,19 @@ class TurnPortTest : public ::testing::Test,
                                         &TurnPortTest::OnSocketReadPacket);
     }
 
-    RelayCredentials credentials(username, password);
+    RelayServerConfig config;
-    turn_port_ =
+    config.credentials = RelayCredentials(username, password);
-        TurnPort::Create(&main_, &socket_factory_, MakeNetwork(kLocalAddr1),
+    CreateRelayPortArgs args;
-                         socket_.get(), kIceUfrag1, kIcePwd1, server_address,
+    args.network_thread = &main_;
-                         credentials, 0, nullptr, &field_trials_);
+    args.socket_factory = &socket_factory_;
+    args.network = MakeNetwork(kLocalAddr1);
+    args.username = kIceUfrag1;
+    args.password = kIcePwd1;
+    args.server_address = &server_address;
+    args.config = &config;
+    args.turn_customizer = turn_customizer_.get();
+    args.field_trials = &field_trials_;
+    turn_port_ = TurnPort::Create(args, socket_.get());
     // This TURN port will be the controlling.
     turn_port_->SetIceRole(ICEROLE_CONTROLLING);
     ConnectSignals();

p2p/client/relay_port_factory_interface.h

@@ -36,7 +36,6 @@ struct RelayServerConfig;
 
 // A struct containing arguments to RelayPortFactory::Create()
 struct CreateRelayPortArgs {
-  CreateRelayPortArgs();
   rtc::Thread* network_thread;
   rtc::PacketSocketFactory* socket_factory;
   rtc::Network* network;
@@ -44,12 +43,10 @@ struct CreateRelayPortArgs {
   const RelayServerConfig* config;
   std::string username;
   std::string password;
-  webrtc::TurnCustomizer* turn_customizer;
+  webrtc::TurnCustomizer* turn_customizer = nullptr;
   const webrtc::WebRtcKeyValueConfig* field_trials = nullptr;
 };
 
-inline CreateRelayPortArgs::CreateRelayPortArgs() {}
-
 // A factory for creating RelayPort's.
 class RelayPortFactoryInterface {
  public:

p2p/client/turn_port_factory.cc

@@ -23,11 +23,7 @@ TurnPortFactory::~TurnPortFactory() {}
 std::unique_ptr<Port> TurnPortFactory::Create(
     const CreateRelayPortArgs& args,
     rtc::AsyncPacketSocket* udp_socket) {
-  auto port = TurnPort::CreateUnique(
+  auto port = TurnPort::Create(args, udp_socket);
-      args.network_thread, args.socket_factory, args.network, udp_socket,
-      args.username, args.password, *args.server_address,
-      args.config->credentials, args.config->priority, args.turn_customizer,
-      args.field_trials);
   if (!port)
     return nullptr;
   port->SetTlsCertPolicy(args.config->tls_cert_policy);
@@ -38,12 +34,7 @@ std::unique_ptr<Port> TurnPortFactory::Create(
 std::unique_ptr<Port> TurnPortFactory::Create(const CreateRelayPortArgs& args,
                                               int min_port,
                                               int max_port) {
-  auto port = TurnPort::CreateUnique(
+  auto port = TurnPort::Create(args, min_port, max_port);
-      args.network_thread, args.socket_factory, args.network, min_port,
-      max_port, args.username, args.password, *args.server_address,
-      args.config->credentials, args.config->priority,
-      args.config->tls_alpn_protocols, args.config->tls_elliptic_curves,
-      args.turn_customizer, args.config->tls_cert_verifier, args.field_trials);
   if (!port)
     return nullptr;
   port->SetTlsCertPolicy(args.config->tls_cert_policy);