Merge pull request #2235 from deiv/fix-abstract-unix-socket-name

Fix abstract unix socket name

7 files changed, 158 insertions, 69 deletions

diff --git a/lib/cpp/CMakeLists.txt b/lib/cpp/CMakeLists.txt
index 29caad475..b3a34d095 100755
--- a/lib/cpp/CMakeLists.txt
+++ b/lib/cpp/CMakeLists.txt
@@ -59,6 +59,7 @@ set( thriftcpp_SOURCES
    src/thrift/transport/TBufferTransports.cpp
    src/thrift/transport/TWebSocketServer.h
    src/thrift/transport/TWebSocketServer.cpp
+   src/thrift/transport/SocketCommon.cpp
    src/thrift/server/TConnectedClient.cpp
    src/thrift/server/TServerFramework.cpp
    src/thrift/server/TSimpleServer.cpp
diff --git a/lib/cpp/Makefile.am b/lib/cpp/Makefile.am
index 3a0c4e63b..eab2e217e 100755
--- a/lib/cpp/Makefile.am
+++ b/lib/cpp/Makefile.am
@@ -88,6 +88,7 @@ libthrift_la_SOURCES = src/thrift/TApplicationException.cpp \
                        src/thrift/transport/TTransportUtils.cpp \
                        src/thrift/transport/TBufferTransports.cpp \
                        src/thrift/transport/TWebSocketServer.cpp \
+                       src/thrift/transport/SocketCommon.cpp \
                        src/thrift/server/TConnectedClient.cpp \
                        src/thrift/server/TServer.cpp \
                        src/thrift/server/TServerFramework.cpp \
@@ -206,7 +207,8 @@ include_transport_HEADERS = \
                          src/thrift/transport/TBufferTransports.h \
                          src/thrift/transport/TShortReadTransport.h \
                          src/thrift/transport/TZlibTransport.h \
-                         src/thrift/transport/TWebSocketServer.h
+                         src/thrift/transport/TWebSocketServer.h \
+                         src/thrift/transport/SocketCommon.h
 
 include_serverdir = $(include_thriftdir)/server
 include_server_HEADERS = \
diff --git a/lib/cpp/src/thrift/transport/SocketCommon.cpp b/lib/cpp/src/thrift/transport/SocketCommon.cpp
new file mode 100644
index 000000000..570f39a93
--- /dev/null
+++ b/lib/cpp/src/thrift/transport/SocketCommon.cpp
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ * @author: David Suárez <david.sephirot@gmail.com>
+ */
+
+#ifndef THRIFT_SOCKETCOMMON_H
+#define THRIFT_SOCKETCOMMON_H
+
+#ifndef _WIN32
+
+#include <thrift/thrift-config.h>
+
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+#ifdef HAVE_SYS_UN_H
+#include <sys/un.h>
+#endif
+
+#include <string>
+
+#include <thrift/transport/PlatformSocket.h>
+#include <thrift/transport/TTransportException.h>
+#include <thrift/TOutput.h>
+
+namespace apache {
+namespace thrift {
+namespace transport {
+
+
+socklen_t fillUnixSocketAddr(struct sockaddr_un& address, std::string& path)
+{
+    // abstract namespace socket ?
+    bool isAbstractNamespace = path[0] == 0;
+
+#ifndef __linux__
+    if (isAbstractNamespace) {
+      GlobalOutput.perror("TSocket::open() Abstract Namespace Domain sockets only supported on linux: ", -99);
+      throw TTransportException(TTransportException::NOT_OPEN,
+                                " Abstract Namespace Domain socket path not supported");
+    }
+#endif
+
+    /*
+     * For abstract namespace sockets, the path string is not null-terminated (as opposite to path based), so we
+     *   rely in pass the string size, to the bind() call.
+     */
+    size_t addr_len = isAbstractNamespace ? path.size() : path.size() + 1;
+
+    if (addr_len > sizeof(((sockaddr_un*)nullptr)->sun_path)) {
+        int errno_copy = THRIFT_GET_SOCKET_ERROR;
+        GlobalOutput.perror("TSocket::open() Unix Domain socket path too long", errno_copy);
+        throw TTransportException(TTransportException::NOT_OPEN, " Unix Domain socket path too long");
+    }
+
+    address.sun_family = AF_UNIX;
+    memcpy(address.sun_path, path.c_str(), addr_len);
+
+    return static_cast<socklen_t>(sizeof((sockaddr_un*)nullptr)->sun_family + addr_len);
+}
+
+}
+}
+} // apache::thrift::transport
+
+#endif // _WIN32
+
+#endif //THRIFT_SOCKETCOMMON_H
diff --git a/lib/cpp/src/thrift/transport/SocketCommon.h b/lib/cpp/src/thrift/transport/SocketCommon.h
new file mode 100644
index 000000000..78839c4e3
--- /dev/null
+++ b/lib/cpp/src/thrift/transport/SocketCommon.h
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ * @author: David Suárez <david.sephirot@gmail.com>
+ */
+
+#ifndef THRIFT_SOCKETCOMMON_H
+#define THRIFT_SOCKETCOMMON_H
+
+#ifndef _WIN32
+
+#include <thrift/thrift-config.h>
+
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+#ifdef HAVE_SYS_UN_H
+#include <sys/un.h>
+#endif
+
+#include <string>
+
+namespace apache {
+namespace thrift {
+namespace transport {
+
+socklen_t fillUnixSocketAddr(struct sockaddr_un& address, std::string& path);
+
+}
+}
+} // apache::thrift::transport
+
+#endif // _WIN32
+
+#endif //THRIFT_SOCKETCOMMON_H
diff --git a/lib/cpp/src/thrift/transport/TNonblockingServerSocket.cpp b/lib/cpp/src/thrift/transport/TNonblockingServerSocket.cpp
index 7bac37eb2..5ef083533 100644
--- a/lib/cpp/src/thrift/transport/TNonblockingServerSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TNonblockingServerSocket.cpp
@@ -48,6 +48,7 @@
 #include <thrift/transport/TNonblockingServerSocket.h>
 #include <thrift/transport/TSocket.h>
 #include <thrift/transport/TSocketUtils.h>
+#include <thrift/transport/SocketCommon.h>
 
 #ifndef AF_LOCAL
 #define AF_LOCAL AF_UNIX
@@ -350,33 +351,14 @@ void TNonblockingServerSocket::listen() {
     _setup_sockopts();
     //_setup_unixdomain_sockopts();
 
+/*
+ * TODO: seems that windows now support unix sockets,
+ *       see: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
+ */
 #ifndef _WIN32
 
-    size_t len = path_.size() + 1;
-    if (len > sizeof(((sockaddr_un*)nullptr)->sun_path)) {
-      errno_copy = THRIFT_GET_SOCKET_ERROR;
-      GlobalOutput.perror("TSocket::listen() Unix Domain socket path too long", errno_copy);
-      throw TTransportException(TTransportException::NOT_OPEN,
-                                "Unix Domain socket path too long",
-                                errno_copy);
-    }
-
     struct sockaddr_un address;
-    address.sun_family = AF_UNIX;
-    memcpy(address.sun_path, path_.c_str(), len);
-
-    auto structlen = static_cast<socklen_t>(sizeof(address));
-
-    if (!address.sun_path[0]) { // abstract namespace socket
-#ifdef __linux__
-      // sun_path is not null-terminated in this case and structlen determines its length
-      structlen -= sizeof(address.sun_path) - len;
-#else
-      GlobalOutput.perror("TSocket::open() Abstract Namespace Domain sockets only supported on linux: ", -99);
-      throw TTransportException(TTransportException::NOT_OPEN,
-                                " Abstract Namespace Domain socket path not supported");
-#endif
-    }
+    socklen_t structlen = fillUnixSocketAddr(address, path_);
 
     do {
       if (0 == ::bind(serverSocket_, (struct sockaddr*)&address, structlen)) {
diff --git a/lib/cpp/src/thrift/transport/TServerSocket.cpp b/lib/cpp/src/thrift/transport/TServerSocket.cpp
index c917538d4..0a47b3fe0 100644
--- a/lib/cpp/src/thrift/transport/TServerSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TServerSocket.cpp
@@ -48,6 +48,7 @@
 #include <thrift/transport/TServerSocket.h>
 #include <thrift/transport/TSocket.h>
 #include <thrift/transport/TSocketUtils.h>
+#include <thrift/transport/SocketCommon.h>
 
 #ifndef AF_LOCAL
 #define AF_LOCAL AF_UNIX
@@ -402,32 +403,14 @@ void TServerSocket::listen() {
     _setup_sockopts();
     _setup_unixdomain_sockopts();
 
+/*
+ * TODO: seems that windows now support unix sockets,
+ *       see: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
+ */
 #ifndef _WIN32
-    size_t len = path_.size() + 1;
-    if (len > sizeof(((sockaddr_un*)nullptr)->sun_path)) {
-      errno_copy = THRIFT_GET_SOCKET_ERROR;
-      GlobalOutput.perror("TSocket::listen() Unix Domain socket path too long", errno_copy);
-      throw TTransportException(TTransportException::NOT_OPEN,
-                                "Unix Domain socket path too long",
-                                errno_copy);
-    }
 
     struct sockaddr_un address;
-    address.sun_family = AF_UNIX;
-    memcpy(address.sun_path, path_.c_str(), len);
-
-    auto structlen = static_cast<socklen_t>(sizeof(address));
-
-    if (!address.sun_path[0]) { // abstract namespace socket
-#ifdef __linux__
-      // sun_path is not null-terminated in this case and structlen determines its length
-      structlen -= sizeof(address.sun_path) - len;
-#else
-      GlobalOutput.perror("TSocket::open() Abstract Namespace Domain sockets only supported on linux: ", -99);
-      throw TTransportException(TTransportException::NOT_OPEN,
-                                " Abstract Namespace Domain socket path not supported");
-#endif
-    }
+    socklen_t structlen = fillUnixSocketAddr(address, path_);
 
     do {
       if (0 == ::bind(serverSocket_, (struct sockaddr*)&address, structlen)) {
diff --git a/lib/cpp/src/thrift/transport/TSocket.cpp b/lib/cpp/src/thrift/transport/TSocket.cpp
index 5000baf44..046c2e685 100644
--- a/lib/cpp/src/thrift/transport/TSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TSocket.cpp
@@ -47,6 +47,7 @@
 #include <thrift/transport/TSocket.h>
 #include <thrift/transport/TTransportException.h>
 #include <thrift/transport/PlatformSocket.h>
+#include <thrift/transport/SocketCommon.h>
 
 #ifndef SOCKOPT_CAST_T
 #ifndef _WIN32
@@ -328,30 +329,14 @@ void TSocket::openConnection(struct addrinfo* res) {
   int ret;
   if (!path_.empty()) {
 
+/*
+ * TODO: seems that windows now support unix sockets,
+ *       see: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
+ */
 #ifndef _WIN32
-    size_t len = path_.size() + 1;
-    if (len > sizeof(((sockaddr_un*)nullptr)->sun_path)) {
-      int errno_copy = THRIFT_GET_SOCKET_ERROR;
-      GlobalOutput.perror("TSocket::open() Unix Domain socket path too long", errno_copy);
-      throw TTransportException(TTransportException::NOT_OPEN, " Unix Domain socket path too long");
-    }
 
     struct sockaddr_un address;
-    address.sun_family = AF_UNIX;
-    memcpy(address.sun_path, path_.c_str(), len);
-
-    auto structlen = static_cast<socklen_t>(sizeof(address));
-
-    if (!address.sun_path[0]) { // abstract namespace socket
-#ifdef __linux__
-      // sun_path is not null-terminated in this case and structlen determines its length
-      structlen -= sizeof(address.sun_path) - len;
-#else
-      GlobalOutput.perror("TSocket::open() Abstract Namespace Domain sockets only supported on linux: ", -99);
-      throw TTransportException(TTransportException::NOT_OPEN,
-                                " Abstract Namespace Domain socket path not supported");
-#endif
-    }
+    socklen_t structlen = fillUnixSocketAddr(address, path_);
 
     ret = connect(socket_, (struct sockaddr*)&address, structlen);
 #else