The resource returned by socket_create() should be a 'Socket', not 's…

…tream'

Summary:
The resource returned by socket_create() should be a 'Socket', not 'stream'

Closes #4036

Reviewed By: paulbiss

Differential Revision: D4245547

fbshipit-source-id: bfd57d69c8f22e16dde0a37fe2ede8825b16faa0

hphp/runtime/base/classname-is.h

@@ -42,6 +42,14 @@ T InstantStatic<T, TInit, init>::value(init());
     return InstantStatic<const StaticString, const char*, GetClassName> \
       ::value;                                                          \
   }
+
+#define RESOURCENAME_IS(str)                                            \
+  static const char *GetResourceName() { return str; }                  \
+  static const StaticString& resourcenameof() {                         \
+    return InstantStatic<const StaticString, const char*, GetResourceName> \
+      ::value;                                                          \
+  }
+
 }
 
 #endif

hphp/runtime/base/socket.h

@@ -61,9 +61,6 @@ struct SocketData : FileData {
 struct Socket : File {
   DECLARE_RESOURCE_ALLOCATION(Socket);
 
-  Socket();
-  Socket(int sockfd, int type, const char *address = nullptr, int port = 0,
-         double timeout = 0, const StaticString& streamType = empty_string_ref);
   virtual ~Socket();
 
   // overriding ResourceData
@@ -94,21 +91,24 @@ struct Socket : File {
   std::string getAddress() const { return m_data->m_address; }
   int         getPort() const    { return m_data->m_port; }
 
-  explicit Socket(std::shared_ptr<SocketData> data);
   std::shared_ptr<SocketData> getData() const {
     return std::static_pointer_cast<SocketData>(File::getData());
   }
 protected:
   bool waitForData();
   bool timedOut() const { return m_data->m_timedOut; }
 
+  Socket();
+  Socket(int sockfd, int type, const char *address = nullptr, int port = 0,
+         double timeout = 0, const StaticString& streamType = empty_string_ref);
   Socket(std::shared_ptr<SocketData> data,
          int sockfd,
          int type,
          const char *address = nullptr,
          int port = 0,
          double timeout = 0,
          const StaticString& streamType = empty_string_ref);
+  explicit Socket(std::shared_ptr<SocketData> data);
 
   // make private?
   SocketData* getSocketData() { return m_data; }
@@ -120,6 +120,35 @@ struct Socket : File {
   static __thread int s_lastErrno;
 };
 
+// This class provides exactly the same functionality as Socket but reports as a
+// class/resource of 'Socket' instead of 'stream'.
+struct ConcreteSocket final : Socket {
+  CLASSNAME_IS("Socket");
+  RESOURCENAME_IS("Socket");
+
+  ConcreteSocket() = default;
+  ConcreteSocket(int sockfd, int type, const char *address = nullptr,
+                 int port = 0, double timeout = 0,
+                 const StaticString& streamType = empty_string_ref) :
+      Socket(sockfd, type, address, port, timeout, streamType) { }
+  explicit ConcreteSocket(std::shared_ptr<SocketData> data) : Socket(data) { }
+
+  // overriding ResourceData
+  const String& o_getClassNameHook() const override { return classnameof(); }
+  const String& o_getResourceName() const override { return resourcenameof(); }
+};
+
+// This class provides exactly the same functionality as ConcreteSocket but
+// reports the default behavior for File.
+struct StreamSocket final : Socket {
+  StreamSocket() = default;
+  StreamSocket(int sockfd, int type, const char *address = nullptr,
+                 int port = 0, double timeout = 0,
+                 const StaticString& streamType = empty_string_ref) :
+      Socket(sockfd, type, address, port, timeout, streamType) { }
+  explicit StreamSocket(std::shared_ptr<SocketData> data) : Socket(data) { }
+};
+
 ///////////////////////////////////////////////////////////////////////////////
 }
 

hphp/runtime/base/ssl-socket.h

@@ -33,7 +33,7 @@ struct SSLSocketData;
 /**
  * TCP sockets running SSL protocol.
  */
-struct SSLSocket : Socket {
+struct SSLSocket final : Socket {
   enum class CryptoMethod {
     ClientSSLv2,
     ClientSSLv3,

hphp/runtime/debugger/debugger_client.cpp

@@ -568,8 +568,8 @@ req::ptr<Socket> DebuggerClient::connectLocal() {
   if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) != 0) {
     throw Exception("unable to create socket pair for local debugging");
   }
-  auto socket1 = req::make<Socket>(fds[0], AF_UNIX);
-  auto socket2 = req::make<Socket>(fds[1], AF_UNIX);
+  auto socket1 = req::make<StreamSocket>(fds[0], AF_UNIX);
+  auto socket2 = req::make<StreamSocket>(fds[1], AF_UNIX);
 
   socket1->unregister();
   socket2->unregister();
@@ -639,7 +639,7 @@ bool DebuggerClient::tryConnect(const std::string &host, int port,
   /* try possible families (v4, v6) until we get a connection */
   struct addrinfo *cur;
   for (cur = ai; cur; cur = cur->ai_next) {
-    auto sock = req::make<Socket>(
+    auto sock = req::make<StreamSocket>(
       socket(cur->ai_family, cur->ai_socktype, 0),
       cur->ai_family,
       cur->ai_addr->sa_data,

hphp/runtime/debugger/debugger_server.cpp

@@ -104,7 +104,7 @@ bool DebuggerServer::start() {
     if (s_fd < 0 && errno == EAFNOSUPPORT) {
       continue;
     }
-    auto m_sock = req::make<Socket>(
+    auto m_sock = req::make<StreamSocket>(
       s_fd, cur->ai_family, cur->ai_addr->sa_data, port);
 
     int yes = 1;
@@ -171,7 +171,7 @@ void DebuggerServer::accept() {
         socklen_t salen = sizeof(sa);
         try {
           auto sock = nthSocket(i);
-          auto new_sock = req::make<Socket>(
+          auto new_sock = req::make<StreamSocket>(
             ::accept(sock->fd(), &sa, &salen), sock->getType());
           if (new_sock->valid()) {
             Debugger::CreateProxy(new_sock, false);

hphp/runtime/debugger/debugger_server.h

@@ -50,7 +50,7 @@ struct DebuggerServer {
   static DebuggerServer s_debugger_server;
 
   req::ptr<Socket> nthSocket(unsigned i) const {
-    return req::make<Socket>(m_socks[i]);
+    return req::make<StreamSocket>(m_socks[i]);
   }
 
   AsyncFunc<DebuggerServer> m_serverThread;

hphp/runtime/debugger/debugger_thrift_buffer.h

@@ -35,7 +35,7 @@ struct DebuggerThriftBuffer : ThriftBuffer {
     : ThriftBuffer(BUFFER_SIZE, VariableSerializer::Type::DebuggerSerialize) {}
 
   req::ptr<Socket> getSocket() {
-    return req::make<Socket>(m_socket);
+    return req::make<StreamSocket>(m_socket);
   }
 
   void create(req::ptr<Socket> socket) {

hphp/runtime/ext/sockets/ext_sockets.cpp

@@ -398,8 +398,8 @@ static req::ptr<Socket> create_new_socket(
   if (sslsock) {
     sock = sslsock;
   } else {
-    sock = req::make<Socket>(fd, domain, hosturl.getHost().c_str(),
-                             hosturl.getPort());
+    sock = req::make<StreamSocket>(fd, domain, hosturl.getHost().c_str(),
+                                   hosturl.getPort());
   }
 
   if (!sock->valid()) {
@@ -489,7 +489,7 @@ static Variant new_socket_connect(const HostURL &hosturl, double timeout,
     size_t sa_size;
 
     fd = socket(domain, type, 0);
-    sock = req::make<Socket>(
+    sock = req::make<StreamSocket>(
       fd, domain, hosturl.getHost().c_str(), hosturl.getPort());
 
     if (!set_sockaddr(sa_storage, sock, hosturl.getHost().c_str(),
@@ -540,10 +540,10 @@ static Variant new_socket_connect(const HostURL &hosturl, double timeout,
     if (sslsock) {
       sock = sslsock;
     } else {
-      sock = req::make<Socket>(fd,
-                                  domain,
-                                  hosturl.getHost().c_str(),
-                                  hosturl.getPort());
+      sock = req::make<StreamSocket>(fd,
+                                     domain,
+                                     hosturl.getHost().c_str(),
+                                     hosturl.getPort());
     }
   }
 
@@ -572,12 +572,12 @@ Variant HHVM_FUNCTION(socket_create,
   check_socket_parameters(domain, type);
   int socketId = socket(domain, type, protocol);
   if (socketId == -1) {
-    SOCKET_ERROR(req::make<Socket>(),
+    SOCKET_ERROR(req::make<ConcreteSocket>(),
                  "Unable to create socket",
                  errno);
     return false;
   }
-  return Variant(req::make<Socket>(socketId, domain));
+  return Variant(req::make<ConcreteSocket>(socketId, domain));
 }
 
 Variant HHVM_FUNCTION(socket_create_listen,
@@ -594,7 +594,7 @@ Variant HHVM_FUNCTION(socket_create_listen,
   la.sin_family = result.hostbuf.h_addrtype;
   la.sin_port = htons((unsigned short)port);
 
-  auto sock = req::make<Socket>(
+  auto sock = req::make<ConcreteSocket>(
     socket(PF_INET, SOCK_STREAM, 0), PF_INET, "0.0.0.0", port);
 
   if (!sock->valid()) {
@@ -618,30 +618,44 @@ Variant HHVM_FUNCTION(socket_create_listen,
 const StaticString
   s_socktype_generic("generic_socket");
 
-bool HHVM_FUNCTION(socket_create_pair,
-                   int domain,
-                   int type,
-                   int protocol,
-                   VRefParam fd) {
+bool socket_create_pair_impl(int domain, int type, int protocol, VRefParam fd,
+                             bool asStream) {
   check_socket_parameters(domain, type);
 
   int fds_array[2];
   if (socketpair(domain, type, protocol, fds_array) != 0) {
-    SOCKET_ERROR(req::make<Socket>(),
+    SOCKET_ERROR(req::make<StreamSocket>(),
                  "unable to create socket pair",
                  errno);
     return false;
   }
 
-  fd.assignIfRef(make_packed_array(
-    Variant(req::make<Socket>(fds_array[0], domain, nullptr, 0, 0.0,
-                              s_socktype_generic)),
-    Variant(req::make<Socket>(fds_array[1], domain, nullptr, 0, 0.0,
-                              s_socktype_generic))
-  ));
+  if (asStream) {
+    fd.assignIfRef(make_packed_array(
+      Variant(req::make<StreamSocket>(fds_array[0], domain, nullptr, 0, 0.0,
+                                      s_socktype_generic)),
+      Variant(req::make<StreamSocket>(fds_array[1], domain, nullptr, 0, 0.0,
+                                      s_socktype_generic))
+    ));
+  } else {
+    fd.assignIfRef(make_packed_array(
+      Variant(req::make<ConcreteSocket>(fds_array[0], domain, nullptr, 0, 0.0,
+                                        s_socktype_generic)),
+      Variant(req::make<ConcreteSocket>(fds_array[1], domain, nullptr, 0, 0.0,
+                                        s_socktype_generic))
+    ));
+  }
   return true;
 }
 
+bool HHVM_FUNCTION(socket_create_pair,
+                   int domain,
+                   int type,
+                   int protocol,
+                   VRefParam fd) {
+  return socket_create_pair_impl(domain, type, protocol, fd, false);
+}
+
 const StaticString
   s_l_onoff("l_onoff"),
   s_l_linger("l_linger"),
@@ -1068,7 +1082,7 @@ Variant HHVM_FUNCTION(socket_accept,
   auto sock = cast<Socket>(socket);
   struct sockaddr sa;
   socklen_t salen = sizeof(sa);
-  auto new_sock = req::make<Socket>(
+  auto new_sock = req::make<ConcreteSocket>(
     accept(sock->fd(), &sa, &salen), sock->getType());
   if (!new_sock->valid()) {
     SOCKET_ERROR(new_sock, "unable to accept incoming connection", errno);
@@ -1456,7 +1470,7 @@ Variant sockopen_impl(const HostURL &hosturl, VRefParam errnum,
           std::dynamic_pointer_cast<SSLSocketData>(sockItr->second)) {
         sock = req::make<SSLSocket>(sslSocketData);
       } else {
-        sock = req::make<Socket>(sockItr->second);
+        sock = req::make<StreamSocket>(sockItr->second);
       }
 
       if (sock->getError() == 0 && sock->checkLiveness()) {

hphp/runtime/ext/sockets/ext_sockets.h

@@ -25,6 +25,9 @@
 namespace HPHP {
 ///////////////////////////////////////////////////////////////////////////////
 
+bool socket_create_pair_impl(int domain, int type, int protocol, VRefParam fd,
+                             bool asStream);
+
 Variant HHVM_FUNCTION(socket_create,
                       int domain,
                       int type,

hphp/runtime/ext/stream/ext_stream.cpp

@@ -620,7 +620,7 @@ static Variant socket_accept_impl(
   } else {
     auto sock = cast<Socket>(socket);
     auto new_fd = accept(sock->fd(), addr, addrlen);
-    new_sock = req::make<Socket>(new_fd, sock->getType());
+    new_sock = req::make<StreamSocket>(new_fd, sock->getType());
   }
 
   if (!new_sock->valid()) {
@@ -834,7 +834,7 @@ Variant HHVM_FUNCTION(stream_socket_pair,
                       int type,
                       int protocol) {
   Variant fd;
-  if (!HHVM_FN(socket_create_pair)(domain, type, protocol, ref(fd))) {
+  if (!socket_create_pair_impl(domain, type, protocol, ref(fd), true)) {
     return false;
   }
   return fd;

hphp/test/slow/ext_socket/ext_socket.php

@@ -19,10 +19,10 @@ function bind_random_port($socket, $address) {
   return 0;
 }
 
-function create_listen_random_port() {
+function create_listen_random_port(&$sock) {
   for ($i = 0; $i < 100; $i++) {
     $port = get_random_port();
-    if (@socket_create_listen($port)) return $port;
+    if ($sock = @socket_create_listen($port)) return $port;
   }
   return 0;
 }
@@ -55,14 +55,19 @@ function get_client_server() {
 $s = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
 var_dump($s);
 
-var_dump(create_listen_random_port() != 0);
+$s2 = false;
+var_dump(create_listen_random_port($s2) != 0);
+var_dump($s2);
 
 var_dump(socket_create_pair(AF_UNIX, SOCK_STREAM, 0, $fds));
 var_dump(count($fds));
+var_dump($fds[0]);
+var_dump($fds[1]);
 
 var_dump(socket_get_option($s, SOL_SOCKET, SO_TYPE), SOCK_STREAM);
 
 list($client, $s) = get_client_server();
+var_dump($s);
 var_dump(socket_write($client, "hello world"));
 // this could fail with shorter returns, but it never does...
 var_dump(socket_read($s, 100));
@@ -113,12 +118,14 @@ function get_client_server() {
 
 $fsock = false;
 $port = pfsockopen_random_port($fsock, "udp://[0:0:0:0:0:0:0:1]");
+var_dump($fsock);
 var_dump($port != 0);
 var_dump(fwrite($fsock, "foo") > 0);
 
 $errnum = null;
 $errstr = null;
 $fsock2 = pfsockopen("udp://[::1]", $port, $errnum, $errstr);
+var_dump($fsock2);
 var_dump($fsock !== false);
 var_dump($fsock != $fsock2);
 var_dump($errnum);

hphp/test/slow/ext_socket/ext_socket.php.expectf

@@ -1,12 +1,16 @@
-resource(4) of type (stream)
+resource(4) of type (Socket)
 bool(true)
+resource(%d) of type (Socket)
 bool(true)
 int(2)
+resource(%d) of type (Socket)
+resource(%d) of type (Socket)
 int(1)
 int(1)
 bool(true)
 bool(true)
 bool(true)
+resource(%d) of type (Socket)
 int(11)
 string(11) "hello world"
 bool(true)
@@ -35,8 +39,10 @@ bool(true)
 NULL
 bool(true)
 int(0)
+resource(%d) of type (stream)
 bool(true)
 bool(true)
+resource(%d) of type (stream)
 bool(true)
 bool(true)
 int(0)

hphp/test/slow/ext_socket/getsockname.php.expectf

@@ -1,4 +1,4 @@
-resource(%d) of type (stream)
+resource(%d) of type (Socket)
 bool(true)
 bool(true)
 string(%d) "/tmp/socktest%d"