Fix Socket remoteAddress obtaining on Win32 #51778

runtime/bin/eventhandler_win.cc

@@ -33,6 +33,14 @@ namespace bin {
 static constexpr int kBufferSize = 64 * 1024;
 static constexpr int kStdOverlappedBufferSize = 16 * 1024;
 static constexpr int kMaxUDPPackageLength = 64 * 1024;
+// For AcceptEx there needs to be buffer storage for address
+// information for two addresses (local and remote address). The
+// AcceptEx documentation says: "This value must be at least 16
+// bytes more than the maximum address length for the transport
+// protocol in use."
+static constexpr int kAcceptExAddressAdditionalBytes = 16;
+static constexpr int kAcceptExAddressStorageSize =
+    sizeof(SOCKADDR_STORAGE) + kAcceptExAddressAdditionalBytes;
 
 OverlappedBuffer* OverlappedBuffer::AllocateBuffer(int buffer_size,
                                                    Operation operation) {
@@ -455,17 +463,21 @@ bool ListenSocket::LoadAcceptEx() {
   return (status != SOCKET_ERROR);
 }
 
+bool ListenSocket::LoadGetAcceptExSockaddrs() {
+  // Load the GetAcceptExSockaddrs function into memory using WSAIoctl.
+  GUID guid_get_accept_ex_sockaddrs = WSAID_GETACCEPTEXSOCKADDRS;
+  DWORD bytes;
+  int status =
+      WSAIoctl(socket(), SIO_GET_EXTENSION_FUNCTION_POINTER,
+               &guid_get_accept_ex_sockaddrs,
+               sizeof(guid_get_accept_ex_sockaddrs), &GetAcceptExSockaddrs_,
+               sizeof(GetAcceptExSockaddrs_), &bytes, nullptr, nullptr);
+  return (status != SOCKET_ERROR);
+}
+
 bool ListenSocket::IssueAccept() {
   MonitorLocker ml(&monitor_);
 
-  // For AcceptEx there needs to be buffer storage for address
-  // information for two addresses (local and remote address). The
-  // AcceptEx documentation says: "This value must be at least 16
-  // bytes more than the maximum address length for the transport
-  // protocol in use."
-  const int kAcceptExAddressAdditionalBytes = 16;
-  const int kAcceptExAddressStorageSize =
-      sizeof(SOCKADDR_STORAGE) + kAcceptExAddressAdditionalBytes;
   OverlappedBuffer* buffer =
       OverlappedBuffer::AllocateAcceptBuffer(2 * kAcceptExAddressStorageSize);
   DWORD received;
@@ -498,8 +510,23 @@ void ListenSocket::AcceptComplete(OverlappedBuffer* buffer,
     int rc = setsockopt(buffer->client(), SOL_SOCKET, SO_UPDATE_ACCEPT_CONTEXT,
                         reinterpret_cast<char*>(&s), sizeof(s));
     if (rc == NO_ERROR) {
+      // getpeername() returns incorrect results when used with a socket that was
+      // accepted using overlapped I/O. AcceptEx includes the remote address in its
+      // result so retrieve it using GetAcceptExSockaddrs and save it.
+      LPSOCKADDR local_addr;
+      int local_addr_length;
+      LPSOCKADDR remote_addr;
+      int remote_addr_length;
+      GetAcceptExSockaddrs_(
+          buffer->GetBufferStart(), 0, kAcceptExAddressStorageSize,
+          kAcceptExAddressStorageSize, &local_addr, &local_addr_length,
+          &remote_addr, &remote_addr_length);
+      RawAddr* remote_addr_ = new RawAddr;
+      memcpy(remote_addr_, remote_addr, remote_addr_length);
+
       // Insert the accepted socket into the list.
-      ClientSocket* client_socket = new ClientSocket(buffer->client());
+      ClientSocket* client_socket =
+          new ClientSocket(buffer->client(), remote_addr_);
       client_socket->mark_connected();
       client_socket->CreateCompletionPort(completion_port);
       if (accepted_head_ == nullptr) {
@@ -557,8 +584,9 @@ void ListenSocket::DoClose() {
   }
   // To finish resetting the state of the ListenSocket back to what it was
   // before EnsureInitialized was called, we have to reset the AcceptEx_
-  // function pointer.
+  // and GetAcceptExSockaddrs_ function pointers.
   AcceptEx_ = nullptr;
+  GetAcceptExSockaddrs_ = nullptr;
 }
 
 bool ListenSocket::CanAccept() {
@@ -601,7 +629,12 @@ void ListenSocket::EnsureInitialized(
     ASSERT(event_handler_ == nullptr);
     event_handler_ = event_handler;
     CreateCompletionPort(event_handler_->completion_port());
-    LoadAcceptEx();
+    bool isLoaded = LoadAcceptEx();
+    ASSERT(isLoaded);
+  }
+  if (GetAcceptExSockaddrs_ == nullptr) {
+    bool isLoaded = LoadGetAcceptExSockaddrs();
+    ASSERT(isLoaded);
   }
 }
 

runtime/bin/eventhandler_win.h

@@ -15,6 +15,7 @@
 
 #include "bin/builtin.h"
 #include "bin/reference_counting.h"
+#include "bin/socket_base.h"
 #include "bin/thread.h"
 
 namespace dart {
@@ -387,6 +388,7 @@ class ListenSocket : public DescriptorInfoMultipleMixin<SocketHandle> {
   explicit ListenSocket(intptr_t s)
       : DescriptorInfoMultipleMixin(s, true),
         AcceptEx_(nullptr),
+        GetAcceptExSockaddrs_(nullptr),
         pending_accept_count_(0),
         accepted_head_(nullptr),
         accepted_tail_(nullptr),
@@ -418,8 +420,10 @@ class ListenSocket : public DescriptorInfoMultipleMixin<SocketHandle> {
 
  private:
   bool LoadAcceptEx();
+  bool LoadGetAcceptExSockaddrs();
 
   LPFN_ACCEPTEX AcceptEx_;
+  LPFN_GETACCEPTEXSOCKADDRS GetAcceptExSockaddrs_;
 
   // The number of asynchronous `IssueAccept` operations which haven't completed
   // yet.
@@ -440,12 +444,13 @@ class ListenSocket : public DescriptorInfoMultipleMixin<SocketHandle> {
 // Information on connected sockets.
 class ClientSocket : public DescriptorInfoSingleMixin<SocketHandle> {
  public:
-  explicit ClientSocket(intptr_t s)
+  explicit ClientSocket(intptr_t s, RawAddr* remote_addr = nullptr)
       : DescriptorInfoSingleMixin(s, true),
         DisconnectEx_(nullptr),
         next_(nullptr),
         connected_(false),
-        closed_(false) {
+        closed_(false),
+        remote_addr_(remote_addr) {
     LoadDisconnectEx();
     type_ = kClientSocket;
   }
@@ -480,6 +485,8 @@ class ClientSocket : public DescriptorInfoSingleMixin<SocketHandle> {
 
   void mark_closed() { closed_ = true; }
 
+  RawAddr* const remote_addr() const { return remote_addr_.get(); }
+
 #if defined(DEBUG)
   static intptr_t disconnecting() { return disconnecting_; }
 #endif
@@ -491,6 +498,7 @@ class ClientSocket : public DescriptorInfoSingleMixin<SocketHandle> {
   ClientSocket* next_;
   bool connected_;
   bool closed_;
+  std::unique_ptr<RawAddr> remote_addr_;
 
 #if defined(DEBUG)
   static intptr_t disconnecting_;

runtime/bin/socket_base_win.cc

@@ -183,9 +183,20 @@ SocketAddress* SocketBase::GetRemotePeer(intptr_t fd, intptr_t* port) {
   ASSERT(reinterpret_cast<Handle*>(fd)->is_socket());
   SocketHandle* socket_handle = reinterpret_cast<SocketHandle*>(fd);
   RawAddr raw;
-  socklen_t size = sizeof(raw);
-  if (getpeername(socket_handle->socket(), &raw.addr, &size)) {
-    return nullptr;
+  RawAddr* cached_remote_addr{};
+  if (socket_handle->is_client_socket()) {
+    cached_remote_addr = reinterpret_cast<ClientSocket*>(fd)->remote_addr();
+  }
+  if(cached_remote_addr) {
+    raw = *cached_remote_addr;
+  } else {
+    socklen_t size = sizeof(raw);
+    if (getpeername(socket_handle->socket(), &raw.addr, &size)) {
+      return nullptr;
+    }
+    if (!size) {
+      return nullptr;
+    }
   }
   *port = SocketAddress::GetAddrPort(raw);
   // Clear the port before calling WSAAddressToString as WSAAddressToString

tests/standalone/standalone_vm.status

@@ -42,7 +42,6 @@ io/socket_upgrade_to_secure_test: Skip # Issue 27638
 [ $system == windows ]
 io/process_sync_test: Pass, Timeout # Issue 24596
 io/sleep_test: Pass, Fail # Issue 25757
-io/socket_info_ipv6_test: Skip
 verbose_gc_to_bmu_test: Skip
 
 [ $arch == arm && $mode == release && $runtime == dart_precompiled && $system == android ]