Refactored to make MDnsSocketFactory return array of sockets.

net/dns/mdns_client_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/dns/mdns_client_impl.h"
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/stl_util.h"
 #include "base/time/default_clock.h"
 #include "net/base/dns_util.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/rand_callback.h"
 #include "net/dns/dns_protocol.h"
 #include "net/dns/record_rdata.h"
 #include "net/udp/datagram_socket.h"
 
 // TODO(gene): Remove this temporary method of disabling NSEC support once it
 // becomes clear whether this feature should be
 // supported. http://crbug.com/255232
 #define ENABLE_NSEC
 
 namespace net {
 
 namespace {
 const unsigned MDnsTransactionTimeoutSeconds = 3;
 }
 
-MDnsConnection::SocketHandler::SocketHandler(
-    MDnsConnection* connection, const IPEndPoint& multicast_addr,
-    MDnsConnection::SocketFactory* socket_factory)
-    : socket_(socket_factory->CreateSocket()), connection_(connection),
-      response_(new DnsResponse(dns_protocol::kMaxMulticastSize)),
-      multicast_addr_(multicast_addr) {
+MDnsConnection::SocketHandler::SocketHandler(scoped_ptr<Socket> socket,
+                                             MDnsConnection* connection)
+    : socket_(socket.Pass()),
+      connection_(connection),
+      response_(dns_protocol::kMaxMulticastSize) {
 }
 
 MDnsConnection::SocketHandler::~SocketHandler() {
 }
 
 int MDnsConnection::SocketHandler::Start() {
   return DoLoop(0);
 }
 
 int MDnsConnection::SocketHandler::DoLoop(int rv) {
   do {
     if (rv > 0)
-      connection_->OnDatagramReceived(response_.get(), recv_addr_, rv);
+      connection_->OnDatagramReceived(&response_, recv_addr_, rv);
 
     rv = socket_->RecvFrom(
-        response_->io_buffer(),
-        response_->io_buffer()->size(),
+        response_.io_buffer(),
+        response_.io_buffer()->size(),
         &recv_addr_,
         base::Bind(&MDnsConnection::SocketHandler::OnDatagramReceived,
                    base::Unretained(this)));
   } while (rv > 0);
 
   if (rv != ERR_IO_PENDING)
     return rv;
 
   return OK;
 }
 
 void MDnsConnection::SocketHandler::OnDatagramReceived(int rv) {
   if (rv >= OK)
     rv = DoLoop(rv);
 
   if (rv != OK)
     connection_->OnError(this, rv);
 }
 
 int MDnsConnection::SocketHandler::Send(IOBuffer* buffer, unsigned size) {
-  return socket_->SendTo(
-      buffer, size, multicast_addr_,
-      base::Bind(&MDnsConnection::SocketHandler::SendDone,
-                 base::Unretained(this) ));
+  return socket_->SendTo(buffer, size,
+                         base::Bind(&MDnsConnection::SocketHandler::SendDone,
+                                    base::Unretained(this) ));
 }
 
 void MDnsConnection::SocketHandler::SendDone(int rv) {
   // TODO(noamsml): Retry logic.
 }
 
-int MDnsConnection::SocketHandler::Bind() {
-  IPAddressNumber address_any(multicast_addr_.address().size());
-
-  IPEndPoint bind_endpoint(address_any, multicast_addr_.port());
-
-  socket_->AllowAddressReuse();
-  int rv = socket_->Listen(bind_endpoint);
-
-  if (rv < OK) return rv;
-
-  socket_->SetMulticastLoopbackMode(false);
-
-  return socket_->JoinGroup(multicast_addr_.address());
-}
-
 MDnsConnection::MDnsConnection(MDnsConnection::Delegate* delegate) :
       delegate_(delegate) {
 }
 
 MDnsConnection::~MDnsConnection() {
 }
 
 bool MDnsConnection::Init(MDnsConnection::SocketFactory* socket_factory) {
-  // TODO(vitalybuka): crbug.com/297690 Make socket_factory return list
-  // of initialized sockets.
-  socket_handlers_.push_back(
-      new SocketHandler(this, GetMDnsIPEndPoint(ADDRESS_FAMILY_IPV4),
-                        socket_factory));
-  socket_handlers_.push_back(
-      new SocketHandler(this, GetMDnsIPEndPoint(ADDRESS_FAMILY_IPV6),
-                        socket_factory));
+  ScopedVector<Socket> sockets;
+  socket_factory->CreateSockets(&sockets);
 
-  for (size_t i = 0; i < socket_handlers_.size();) {
-    int rv = socket_handlers_[i]->Bind();
-    if (rv != OK) {
-      socket_handlers_.erase(socket_handlers_.begin() + i);
-      VLOG(1) << "Bind failed, socket=" << i << ", error=" << rv;
-    } else {
-      ++i;
-    }
+  for (size_t i = 0; i < sockets.size(); ++i) {
+    socket_handlers_.push_back(

[szym, 2013/11/27 00:28:09]

Empty sockets (which failed to bind in CreateSockets) should be filtered out
here.

[Vitaly Buka (NO REVIEWS), 2013/11/27 00:45:45]

CreateSockets should filter. That was initial idea, but somehow I missed that
part. i'll put that at line 191

On 2013/11/27 00:28:09, szym wrote:

+        new MDnsConnection::SocketHandler(make_scoped_ptr(sockets[i]), this));
   }
+  sockets.weak_clear();
 
   // All unbound sockets need to be bound before processing untrusted input.
   // This is done for security reasons, so that an attacker can't get an unbound
   // socket.
   for (size_t i = 0; i < socket_handlers_.size();) {
     int rv = socket_handlers_[i]->Start();
     if (rv != OK) {
       socket_handlers_.erase(socket_handlers_.begin() + i);
       VLOG(1) << "Start failed, socket=" << i << ", error=" << rv;
     } else {
@@ skipping 26 matching lines @@
 
 void MDnsConnection::OnDatagramReceived(
     DnsResponse* response,
     const IPEndPoint& recv_addr,
     int bytes_read) {
   // TODO(noamsml): More sophisticated error handling.
   DCHECK_GT(bytes_read, 0);
   delegate_->HandlePacket(response, bytes_read);
 }
 
+class SocketImpl : public MDnsConnection::Socket {
+ public:
+  explicit SocketImpl(const IPEndPoint& multicast_addr)
+      : multicast_addr_(multicast_addr),
+        socket_(NULL, NetLog::Source()) {};
+  virtual ~SocketImpl() {};
+
+  int Bind() {
+    IPAddressNumber address_any(multicast_addr_.address().size());
+    IPEndPoint bind_endpoint(address_any, multicast_addr_.port());
+
+    socket_.AllowAddressReuse();
+    int rv = socket_.Listen(bind_endpoint);
+    if (rv < OK)
+      return rv;
+
+    socket_.SetMulticastLoopbackMode(false);
+    return socket_.JoinGroup(multicast_addr_.address());
+  }
+
+  virtual int RecvFrom(IOBuffer* buf,
+                      int buf_len,
+                      IPEndPoint* address,
+                      const CompletionCallback& callback) OVERRIDE {
+    return socket_.RecvFrom(buf, buf_len, address, callback);
+  }
+
+  virtual int SendTo(IOBuffer* buf,
+                     int buf_len,
+                     const CompletionCallback& callback) OVERRIDE {
+   return socket_.SendTo(buf, buf_len, multicast_addr_, callback);
+  }
+
+ private:
+  IPEndPoint multicast_addr_;
+  UDPServerSocket socket_;
+
+  DISALLOW_COPY_AND_ASSIGN(SocketImpl);
+};
+
 class MDnsConnectionSocketFactoryImpl
     : public MDnsConnection::SocketFactory {
  public:
-  MDnsConnectionSocketFactoryImpl();
-  virtual ~MDnsConnectionSocketFactoryImpl();
+  MDnsConnectionSocketFactoryImpl() {};
+  virtual ~MDnsConnectionSocketFactoryImpl() {};
 
-  virtual scoped_ptr<DatagramServerSocket> CreateSocket() OVERRIDE;
+  virtual void CreateSockets(
+      ScopedVector<MDnsConnection::Socket>* sockets) OVERRIDE {
+    sockets->push_back(CreateSocket(ADDRESS_FAMILY_IPV4).release());
+    sockets->push_back(CreateSocket(ADDRESS_FAMILY_IPV6).release());
+  }
+
+ private:
+  scoped_ptr<MDnsConnection::Socket> CreateSocket(
+      AddressFamily address_family) {
+    scoped_ptr<SocketImpl> socket(
+        new SocketImpl(GetMDnsIPEndPoint(address_family)));
+    int rv = socket->Bind();
+    if (rv != OK) {
+      VLOG(1) << "Bind failed, family=" << address_family << ", error=" << rv;
+      socket.reset();
+    }
+    return socket.PassAs<MDnsConnection::Socket>();
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(MDnsConnectionSocketFactoryImpl);
 };
 
-MDnsConnectionSocketFactoryImpl::MDnsConnectionSocketFactoryImpl() {
-}
-
-MDnsConnectionSocketFactoryImpl::~MDnsConnectionSocketFactoryImpl() {
-}
-
-scoped_ptr<DatagramServerSocket>
-MDnsConnectionSocketFactoryImpl::CreateSocket() {
-  return scoped_ptr<DatagramServerSocket>(new UDPServerSocket(
-      NULL, NetLog::Source()));
-}
-
 // static
 scoped_ptr<MDnsConnection::SocketFactory>
 MDnsConnection::SocketFactory::CreateDefault() {
   return scoped_ptr<MDnsConnection::SocketFactory>(
       new MDnsConnectionSocketFactoryImpl);
 }
 
 MDnsClientImpl::Core::Core(MDnsClientImpl* client)
     : client_(client), connection_(new MDnsConnection(this)) {
 }
@@ skipping 463 matching lines @@
 
 void MDnsTransactionImpl::OnNsecRecord(const std::string& name, unsigned type) {
   TriggerCallback(RESULT_NSEC, NULL);
 }
 
 void MDnsTransactionImpl::OnCachePurged() {
   // TODO(noamsml): Cache purge situations not yet implemented
 }
 
 }  // namespace net