[net/dns] Resolve AF_UNSPEC on dual-stacked systems. Sort addresses according to RFC3484.

net/dns/address_sorter_win.cc

+// Copyright (c) 2012 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/address_sorter.h"
+
+#include <winsock2.h>
+
+#include <algorithm>
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/threading/worker_pool.h"
+#include "base/win/windows_version.h"
+#include "net/base/address_list.h"
+#include "net/base/ip_endpoint.h"
+#include "net/base/winsock_init.h"
+
+namespace net {
+
+namespace {
+
+class AddressSorterWin : public AddressSorter {
+ public:
+  AddressSorterWin() {
+    EnsureWinsockInit();
+  }
+
+  virtual ~AddressSorterWin() {}
+
+  virtual void Sort(const AddressList& list,
+                    const CallbackType& callback) const OVERRIDE {
+    scoped_refptr<Job> job = new Job(list, callback);
+  }
+
+ private:
+  // Executes the SIO_ADDRESS_LIST_SORT ioctl on the WorkerPool, and
+  // performs the necessary conversions to/from AddressList.
+  class Job : public base::RefCountedThreadSafe<Job> {
+   public:
+    Job(const AddressList& list, const CallbackType& callback)
+        : callback_(callback),
+          success_(false),
+          buffer_size_(sizeof(SOCKET_ADDRESS_LIST) +
+                       list.size() * (sizeof(SOCKET_ADDRESS) +
+                                      sizeof(SOCKADDR_STORAGE))),
+          buffer_(reinterpret_cast<SOCKET_ADDRESS_LIST*>(
+              malloc(buffer_size_))) {
+

[mmenke, 2012/08/09 19:51:11]

nit:  Remove blank line.  The prevailing style is not to have a blank line at
the start of function bodies, even with initializer lists.

+      buffer_->iAddressCount = list.size();
+      SOCKADDR_STORAGE* storage = reinterpret_cast<SOCKADDR_STORAGE*>(
+          buffer_->Address + buffer_->iAddressCount);
+
+      for (size_t i = 0; i < list.size(); ++i) {
+        IPEndPoint ipe = list[i];
+        // Addresses must be sockaddr_in6.
+        if (ipe.GetFamily() == AF_INET) {
+          ipe = IPEndPoint(ConvertIPv4NumberToIPv6Number(ipe.address()),
+                           ipe.port());
+        }
+
+        struct sockaddr* addr = reinterpret_cast<struct sockaddr*>(storage + i);
+        socklen_t addr_len = sizeof(SOCKADDR_STORAGE);
+        bool result = ipe.ToSockAddr(addr, &addr_len);
+        DCHECK(result);
+        buffer_->Address[i].lpSockaddr = addr;
+        buffer_->Address[i].iSockaddrLength = addr_len;
+      }
+
+      if (!base::WorkerPool::PostTaskAndReply(
+          FROM_HERE,
+          base::Bind(&Job::Run, this),
+          base::Bind(&Job::OnComplete, this),
+          false /* task is slow */)) {
+        LOG(ERROR) << "WorkerPool::PostTaskAndReply failed";
+        OnComplete();
+      }
+    }
+
+   private:
+    friend class base::RefCountedThreadSafe<Job>;
+    ~Job() {}
+
+    // Executed on the WorkerPool.
+    void Run() {
+      SOCKET sock = socket(AF_INET6, SOCK_DGRAM, IPPROTO_UDP);
+      DCHECK_NE(INVALID_SOCKET, sock);
+      DWORD result_size = 0;
+      int result = WSAIoctl(sock, SIO_ADDRESS_LIST_SORT, buffer_.get(),
+                            buffer_size_, buffer_.get(), buffer_size_,

[mmenke, 2012/08/09 19:51:11]

Are you sure it's safe to use the same input and output buffers?

Also, it's not clear if input buffer size should include the SOCKADDR_STORAGEs
on the end of the list.

[szym, 2012/08/10 05:56:01]

I am fairly certain that the answer is yes to both questions. I have no
documentation confirming the first one so I'll change it to two buffers for
safety. Regarding the second, AFAIK: In ioctl calls, all of the memory must be
contained within the user process and all of it has to be allocated before the
call, so there's no way for the ioctl call to complete if it cannot fit
SOCKADDR_STORAGE inside the buffer.

What I wonder about though is whether using overlapped calls would allow us to
avoid calling the WorkerPool. I've previously experimented with that but
observed that the call would block (measured elapsed time could still reach into
10ms).

[mmenke, 2012/08/10 14:20:11]

I actually meant the input buffer size, not the output one - the output one, I
agree with you, but with the input one, it's not so clear.

[szym, 2012/08/10 15:04:39]

I don't think there's any risk in including it for the input. |iAddressCount|
stores the actual count. I'd be concerned that the OS needs to copy data from
the buffer into kernel, so it's safer to include it.

[mmenke, 2012/08/10 15:12:44]

SGTM

+                            &result_size, NULL, NULL);
+      if (result == SOCKET_ERROR) {
+        LOG(ERROR) << "SIO_ADDRESS_LIST_SORT failed " << WSAGetLastError();
+      } else {
+        success_ = true;
+      }
+      closesocket(sock);
+    }
+
+    // Executed on the calling thread.
+    void OnComplete() {
+      AddressList list;
+      if (success_) {
+        list.reserve(buffer_->iAddressCount);
+        for (int i = 0; i < buffer_->iAddressCount; ++i) {
+          IPEndPoint ipe;
+          ipe.FromSockAddr(buffer_->Address[i].lpSockaddr,
+                           buffer_->Address[i].iSockaddrLength);
+          // Unmap V4MAPPED IPv6 addresses so that Happy Eyeballs works.
+          if (IsIPv4Mapped(ipe.address())) {
+            ipe = IPEndPoint(ConvertIPv4MappedToIPv4(ipe.address()),
+                                                     ipe.port());
+          }
+          list.push_back(ipe);
+        }
+      }
+      callback_.Run(success_, list);
+    }
+
+    CallbackType callback_;
+    bool success_;
+    size_t buffer_size_;

[mmenke, 2012/08/09 19:51:11]

buffer_size_ and callback_ can both be const.

+    scoped_ptr_malloc<SOCKET_ADDRESS_LIST> buffer_;
+
+    DISALLOW_COPY_AND_ASSIGN(Job);
+  };
+
+  DISALLOW_COPY_AND_ASSIGN(AddressSorterWin);
+};
+
+// Merges |list_ipv4| and |list_ipv6| before passing it to |callback|, but
+// only if |success| is true.
+void MergeResults(const AddressSorter::CallbackType& callback,
+                  const AddressList& list_ipv4,
+                  bool success,
+                  const AddressList& list_ipv6) {
+  if (!success) {
+    callback.Run(false, AddressList());
+    return;
+  }
+  AddressList list;
+  list.insert(list.end(), list_ipv6.begin(), list_ipv6.end());
+  list.insert(list.end(), list_ipv4.begin(), list_ipv4.end());
+  callback.Run(true, list);
+}
+
+// Wrapper for AddressSorterWin which does not sort IPv4 or IPv4-mapped
+// addresses but always puts them at the end of the list. Needed because the
+// SIO_ADDRESS_LIST_SORT does not support IPv4 addresses on Windows XP.
+class AddressSorterWinXP : public AddressSorter {
+ public:
+  AddressSorterWinXP() {}
+  virtual ~AddressSorterWinXP() {}
+
+  virtual void Sort(const AddressList& list,
+                    const CallbackType& callback) const OVERRIDE {
+    AddressList list_ipv4;
+    AddressList list_ipv6;
+    for (size_t i = 0; i < list.size(); ++i) {
+      const IPEndPoint& ipe = list[i];
+      if (ipe.GetFamily() == AF_INET) {
+        list_ipv4.push_back(ipe);
+      } else {
+        list_ipv6.push_back(ipe);
+      }
+    }
+    if (!list_ipv6.empty()) {

[mmenke, 2012/08/09 19:51:11]

NOTREACHED()?  Currently shouldn't reach this when we don't have any IPv6
addresses.

[szym, 2012/08/10 05:56:01]

Ok, I'll add this requirement to the spec.

+      sorter_.Sort(list_ipv6, base::Bind(&MergeResults, callback, list_ipv4));
+    } else {
+      callback.Run(true, list);
+    }
+  }
+
+ private:
+  AddressSorterWin sorter_;
+
+  DISALLOW_COPY_AND_ASSIGN(AddressSorterWinXP);
+};
+
+}  // namespace
+
+// static
+scoped_ptr<AddressSorter> AddressSorter::CreateAddressSorter() {
+  if (base::win::GetVersion() < base::win::VERSION_VISTA)
+    return scoped_ptr<AddressSorter>(new AddressSorterWinXP());
+  return scoped_ptr<AddressSorter>(new AddressSorterWin());
+}
+
+}  // namespace net
+