* ext/socket/socket.c: fix sockaddr_un handling.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@10212 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

3 files changed, 175 insertions, 26 deletions

diff --git a/ChangeLog b/ChangeLog
index 595298005f..99222fc41a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+Sun Jun  4 20:40:19 2006  Tanaka Akira  <akr@m17n.org>
+
+	* ext/socket/socket.c: fix sockaddr_un handling.
+
 Fri Jun  2 19:00:40 2006  GOTOU Yuuzou  <gotoyuzo@notwork.org>
 
 	* ext/openssl/extconf.rb: use create_header.
diff --git a/ext/socket/socket.c b/ext/socket/socket.c
index 299de63818..0d0b4b3d06 100644
--- a/ext/socket/socket.c
+++ b/ext/socket/socket.c
@@ -543,7 +543,7 @@ bsock_do_not_reverse_lookup_set(sock, state)
 
 static VALUE ipaddr _((struct sockaddr*, int));
 #ifdef HAVE_SYS_UN_H
-static VALUE unixaddr _((struct sockaddr_un*));
+static VALUE unixaddr _((struct sockaddr_un*, socklen_t));
 #endif
 
 enum sock_recv_type {
@@ -620,10 +620,7 @@ s_recvfrom(sock, argc, argv, from)
 
 #ifdef HAVE_SYS_UN_H
       case RECV_UNIX:
-        if (alen) /* connection-oriented socket may not return a from result */
-            return rb_assoc_new(str, unixaddr((struct sockaddr_un*)buf));
-        else
-            return rb_assoc_new(str, Qnil);
+        return rb_assoc_new(str, unixaddr((struct sockaddr_un*)buf, alen));
 #endif
       case RECV_SOCKET:
 	return rb_assoc_new(str, rb_str_new(buf, alen));
@@ -685,8 +682,7 @@ s_recvfrom_nonblock(int argc, VALUE *argv, VALUE sock, enum sock_recv_type from)
 
 #ifdef HAVE_SYS_UN_H
       case RECV_UNIX:
-        if (alen) /* connection-oriented socket may not return a from result */
-            addr = unixaddr((struct sockaddr_un*)buf);
+        addr = unixaddr((struct sockaddr_un*)buf, alen);
         break;
 #endif
 
@@ -1607,8 +1603,11 @@ init_unixsock(sock, path, server)
 
     MEMZERO(&sockaddr, struct sockaddr_un, 1);
     sockaddr.sun_family = AF_UNIX;
-    strncpy(sockaddr.sun_path, RSTRING(path)->ptr, sizeof(sockaddr.sun_path)-1);
-    sockaddr.sun_path[sizeof(sockaddr.sun_path)-1] = '\0';
+    if (sizeof(sockaddr.sun_path) <= RSTRING(path)->len) {
+        rb_raise(rb_eArgError, "too long unix socket path (max: %dbytes)",
+            (int)sizeof(sockaddr.sun_path)-1);
+    }
+    strcpy(sockaddr.sun_path, StringValueCStr(path));
 
     if (server) {
         status = bind(fd, (struct sockaddr*)&sockaddr, sizeof(sockaddr));
@@ -1634,7 +1633,9 @@ init_unixsock(sock, path, server)
 
     init_sock(sock, fd);
     GetOpenFile(sock, fptr);
-    fptr->path = strdup(RSTRING(path)->ptr);
+    if (server) {
+        fptr->path = strdup(RSTRING(path)->ptr);
+    }
 
     return sock;
 }
@@ -1866,6 +1867,15 @@ unix_init(sock, path)
     return init_unixsock(sock, path, 0);
 }
 
+static char *
+unixpath(struct sockaddr_un *sockaddr, socklen_t len)
+{
+    if (sockaddr->sun_path < (char*)sockaddr + len)
+        return sockaddr->sun_path;
+    else
+        return "";
+}
+
 static VALUE
 unix_path(sock)
     VALUE sock;
@@ -1878,7 +1888,7 @@ unix_path(sock)
 	socklen_t len = sizeof(addr);
 	if (getsockname(fptr->fd, (struct sockaddr*)&addr, &len) < 0)
 	    rb_sys_fail(0);
-	fptr->path = strdup(addr.sun_path);
+	fptr->path = strdup(unixpath(&addr, len));
     }
     return rb_str_new2(fptr->path);
 }
@@ -2177,7 +2187,7 @@ unix_accept_nonblock(sock)
     VALUE sock;
 {
     OpenFile *fptr;
-    struct sockaddr_storage from;
+    struct sockaddr_un from;
     socklen_t fromlen;
 
     GetOpenFile(sock, fptr);
@@ -2201,11 +2211,12 @@ unix_sysaccept(sock)
 
 #ifdef HAVE_SYS_UN_H
 static VALUE
-unixaddr(sockaddr)
+unixaddr(sockaddr, len)
     struct sockaddr_un *sockaddr;
+    socklen_t len;
 {
     return rb_assoc_new(rb_str_new2("AF_UNIX"),
-			rb_str_new2(sockaddr->sun_path));
+                        rb_str_new2(unixpath(sockaddr, len)));
 }
 #endif
 
@@ -2221,9 +2232,7 @@ unix_addr(sock)
 
     if (getsockname(fptr->fd, (struct sockaddr*)&addr, &len) < 0)
 	rb_sys_fail("getsockname(2)");
-    if (len == 0)
-        addr.sun_path[0] = '\0';
-    return unixaddr(&addr);
+    return unixaddr(&addr, len);
 }
 
 static VALUE
@@ -2238,9 +2247,7 @@ unix_peeraddr(sock)
 
     if (getpeername(fptr->fd, (struct sockaddr*)&addr, &len) < 0)
 	rb_sys_fail("getsockname(2)");
-    if (len == 0)
-        addr.sun_path[0] = '\0';
-    return unixaddr(&addr);
+    return unixaddr(&addr, len);
 }
 #endif
 
@@ -3516,11 +3523,17 @@ sock_s_pack_sockaddr_un(self, path)
     VALUE self, path;
 {
     struct sockaddr_un sockaddr;
+    char *sun_path;
     VALUE addr;
 
     MEMZERO(&sockaddr, struct sockaddr_un, 1);
     sockaddr.sun_family = AF_UNIX;
-    strncpy(sockaddr.sun_path, StringValuePtr(path), sizeof(sockaddr.sun_path)-1);
+    sun_path = StringValueCStr(path);
+    if (sizeof(sockaddr.sun_path) <= strlen(sun_path)) {
+        rb_raise(rb_eArgError, "too long unix socket path (max: %dbytes)",
+            (int)sizeof(sockaddr.sun_path)-1);
+    }
+    strncpy(sockaddr.sun_path, sun_path, sizeof(sockaddr.sun_path)-1);
     addr = rb_str_new((char*)&sockaddr, sizeof(sockaddr));
     OBJ_INFECT(addr, path);
 
@@ -3532,15 +3545,21 @@ sock_s_unpack_sockaddr_un(self, addr)
     VALUE self, addr;
 {
     struct sockaddr_un * sockaddr;
+    char *sun_path;
     VALUE path;
 
     sockaddr = (struct sockaddr_un*)StringValuePtr(addr);
-    if (RSTRING(addr)->len != sizeof(struct sockaddr_un)) {
-	rb_raise(rb_eTypeError, "sockaddr_un size differs - %ld required; %d given",
+    if (sizeof(struct sockaddr_un) < RSTRING(addr)->len) {
+	rb_raise(rb_eTypeError, "too long sockaddr_un - %ld longer than %d",
 		 RSTRING(addr)->len, sizeof(struct sockaddr_un));
     }
-    /* xxx: should I check against sun_path size? */
-    path = rb_str_new2(sockaddr->sun_path);
+    sun_path = unixpath(sockaddr, RSTRING(addr)->len);
+    if (sizeof(struct sockaddr_un) == RSTRING(addr)->len &&
+        sun_path == sockaddr->sun_path &&
+        sun_path + strlen(sun_path) == RSTRING(addr)->ptr + RSTRING(addr)->len) {
+        rb_raise(rb_eArgError, "sockaddr_un.sun_path not NUL terminated");
+    }
+    path = rb_str_new2(sun_path);
     OBJ_INFECT(path, addr);
     return path;
 }
diff --git a/test/socket/test_unix.rb b/test/socket/test_unix.rb
index b1c0a38537..081100073a 100644
--- a/test/socket/test_unix.rb
+++ b/test/socket/test_unix.rb
@@ -1,9 +1,11 @@
 begin
   require "socket"
-  require "test/unit"
 rescue LoadError
 end
 
+require "test/unit"
+require "tempfile"
+
 class TestUNIXSocket < Test::Unit::TestCase
   def test_fd_passing
     r1, w = IO.pipe
@@ -27,4 +29,128 @@ class TestUNIXSocket < Test::Unit::TestCase
       r2.close if r2 && !r2.closed?
     end
   end
+
+  def bound_unix_socket(klass)
+    tmpfile = Tempfile.new("testrubysock")
+    path = tmpfile.path
+    tmpfile.close(true)
+    yield klass.new(path), path
+  ensure
+    File.unlink path if path && File.socket?(path)
+  end
+
+  def test_addr
+    bound_unix_socket(UNIXServer) {|serv, path|
+      c = UNIXSocket.new(path)
+      s = serv.accept
+      assert_equal(["AF_UNIX", path], c.peeraddr)
+      assert_equal(["AF_UNIX", ""], c.addr)
+      assert_equal(["AF_UNIX", ""], s.peeraddr)
+      assert_equal(["AF_UNIX", path], s.addr)
+      assert_equal(path, s.path)
+      assert_equal("", c.path)
+    }
+  end
+
+  def test_noname_path
+    s1, s2 = UNIXSocket.pair
+    assert_equal("", s1.path)
+    assert_equal("", s2.path)
+  ensure
+    s1.close
+    s2.close
+  end
+
+  def test_noname_addr
+    s1, s2 = UNIXSocket.pair
+    assert_equal(["AF_UNIX", ""], s1.addr)
+    assert_equal(["AF_UNIX", ""], s2.addr)
+  ensure
+    s1.close
+    s2.close
+  end
+
+  def test_noname_peeraddr
+    s1, s2 = UNIXSocket.pair
+    assert_equal(["AF_UNIX", ""], s1.peeraddr)
+    assert_equal(["AF_UNIX", ""], s2.peeraddr)
+  ensure
+    s1.close
+    s2.close
+  end
+
+  def test_noname_unpack_sockaddr_un
+    s1, s2 = UNIXSocket.pair
+    assert_equal("", Socket.unpack_sockaddr_un(s1.getsockname))
+    assert_equal("", Socket.unpack_sockaddr_un(s2.getsockname))
+    assert_equal("", Socket.unpack_sockaddr_un(s1.getpeername))
+    assert_equal("", Socket.unpack_sockaddr_un(s2.getpeername))
+  ensure
+    s1.close
+    s2.close
+  end
+
+  def test_noname_recvfrom
+    s1, s2 = UNIXSocket.pair
+    s2.write("a")
+    assert_equal(["a", ["AF_UNIX", ""]], s1.recvfrom(10))
+  ensure
+    s1.close
+    s2.close
+  end
+
+  def test_noname_recvfrom_nonblock
+    s1, s2 = UNIXSocket.pair
+    s2.write("a")
+    IO.select [s1]
+    assert_equal(["a", ["AF_UNIX", ""]], s1.recvfrom_nonblock(10))
+  ensure
+    s1.close
+    s2.close
+  end
+
+  def test_too_long_path
+    assert_raise(ArgumentError) { Socket.sockaddr_un("a" * 300) }
+    assert_raise(ArgumentError) { UNIXServer.new("a" * 300) }
+  end
+
+  def test_nul
+    assert_raise(ArgumentError) { Socket.sockaddr_un("a\0b") }
+    assert_raise(ArgumentError) { UNIXServer.new("a\0b") }
+  end
+
+  def test_dgram_pair
+    s1, s2 = UNIXSocket.pair(Socket::SOCK_DGRAM)
+    assert_raise(Errno::EAGAIN) { s1.recvfrom_nonblock(10) }
+    s2.send("", 0)
+    s2.send("haha", 0)
+    s2.send("", 0)
+    s2.send("", 0)
+    assert_equal("", s1.recv(10))
+    assert_equal("haha", s1.recv(10))
+    assert_equal("", s1.recv(10))
+    assert_equal("", s1.recv(10))
+    assert_raise(Errno::EAGAIN) { s1.recvfrom_nonblock(10) }
+  ensure
+    s1.close
+    s2.close
+  end
+
+  def test_seqpacket_pair
+    s1, s2 = UNIXSocket.pair(Socket::SOCK_SEQPACKET)
+    assert_raise(Errno::EAGAIN) { s1.recvfrom_nonblock(10) }
+    s2.send("", 0)
+    s2.send("haha", 0)
+    s2.send("", 0)
+    s2.send("", 0)
+    assert_equal("", s1.recv(10))
+    assert_equal("haha", s1.recv(10))
+    assert_equal("", s1.recv(10))
+    assert_equal("", s1.recv(10))
+    assert_raise(Errno::EAGAIN) { s1.recvfrom_nonblock(10) }
+  ensure
+    s1.close
+    s2.close
+  end
+
 end if defined?(UNIXSocket)