update SocketAddress::setFromPath() to take a StringPiece

Summary:Update setFromPath() to accept a StringPiece rather than just std::string
or a plain const char*.

Also fix two other minor issues:
- Leave the old address untouched on failure.  Previously it could leave the
  SocketAddress in a partially updated state.
- Don't assume the input is nul terminated.  Previously the input code read
  one past the specified input length, and copied this into the address,
  assuming it was a nul terminator.  The new code explicitly writes a 0 byte.

Reviewed By: yfeldblum

Differential Revision: D3119882

fb-gh-sync-id: 3e2258f42034b4f470ade0a23ea085e132a3dd0f
fbshipit-source-id: 3e2258f42034b4f470ade0a23ea085e132a3dd0f

diff --git a/folly/SocketAddress.cpp b/folly/SocketAddress.cpp
index ce804bdefef6689bf125f4d8f3c392cb900d6969..d3614d6cc28145a186fe390ed5a46036ff1b52f8 100644 (file)
--- a/folly/SocketAddress.cpp
+++ b/folly/SocketAddress.cpp
@@ -180,24 +180,29 @@ void SocketAddress::setFromHostPort(const char* hostAndPort) {
   setFromAddrInfo(results.info);
 }
 
-void SocketAddress::setFromPath(const char* path, size_t len) {
+void SocketAddress::setFromPath(StringPiece path) {
+  // Before we touch storage_, check to see if the length is too big.
+  // Note that "storage_.un.addr->sun_path" may not be safe to evaluate here,
+  // but sizeof() just uses its type, and does't evaluate it.
+  if (path.size() > sizeof(storage_.un.addr->sun_path)) {
+    throw std::invalid_argument(
+        "socket path too large to fit into sockaddr_un");
+  }
+
   if (!external_) {
     storage_.un.init();
     external_ = true;
   }
 
+  size_t len = path.size();
   storage_.un.len = offsetof(struct sockaddr_un, sun_path) + len;
-  if (len > sizeof(storage_.un.addr->sun_path)) {
-    throw std::invalid_argument(
-      "socket path too large to fit into sockaddr_un");
-  } else if (len == sizeof(storage_.un.addr->sun_path)) {
-    // Note that there will be no terminating NUL in this case.
-    // We allow this since getsockname() and getpeername() may return
-    // Unix socket addresses with paths that fit exactly in sun_path with no
-    // terminating NUL.
-    memcpy(storage_.un.addr->sun_path, path, len);
-  } else {
-    memcpy(storage_.un.addr->sun_path, path, len + 1);
+  memcpy(storage_.un.addr->sun_path, path.data(), len);
+  // If there is room, put a terminating NUL byte in sun_path.  In general the
+  // path should be NUL terminated, although getsockname() and getpeername()
+  // may return Unix socket addresses with paths that fit exactly in sun_path
+  // with no terminating NUL.
+  if (len < sizeof(storage_.un.addr->sun_path)) {
+    storage_.un.addr->sun_path[len] = '\0';
   }
 }
 

diff --git a/folly/SocketAddress.h b/folly/SocketAddress.h
index 4ff1fd18f6b9511192352ca9184683446b452feb..12d8cae7ffb7be85232cec0758042b29a4dbc5fb 100644 (file)
--- a/folly/SocketAddress.h
+++ b/folly/SocketAddress.h
@@ -27,6 +27,7 @@
 
 #include <folly/IPAddress.h>
 #include <folly/Portability.h>
+#include <folly/Range.h>
 
 namespace folly {
 
@@ -278,16 +279,12 @@ class SocketAddress {
    *
    * Raises std::invalid_argument on error.
    */
-  void setFromPath(const char* path) {
-    setFromPath(path, strlen(path));
-  }
+  void setFromPath(StringPiece path);
 
-  void setFromPath(const std::string& path) {
-    setFromPath(path.data(), path.length());
+  void setFromPath(const char* path, size_t length) {
+    setFromPath(StringPiece{path, length});
   }
 
-  void setFromPath(const char* path, size_t length);
-
   // a typedef that allow us to compile against both winsock & POSIX sockets:
   using SocketDesc = decltype(socket(0,0,0)); // POSIX: int, winsock: unsigned