From 2fc5863719a86176c7045f14bf7d1eef355ea205 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Mon, 20 Feb 2017 19:35:29 +0100 Subject: [PATCH 1/9] Add abstract UNIX socket handling --- spec/std/socket_spec.cr | 87 ++++++++++++++++++++++++++++++++++++++- src/socket/address.cr | 26 ++++++++++-- src/socket/unix_server.cr | 19 +++++---- src/socket/unix_socket.cr | 21 +++++----- 4 files changed, 130 insertions(+), 23 deletions(-) diff --git a/spec/std/socket_spec.cr b/spec/std/socket_spec.cr index 0caa51af5b04..5367dc7bb2bd 100644 --- a/spec/std/socket_spec.cr +++ b/spec/std/socket_spec.cr @@ -175,16 +175,35 @@ describe Socket::UNIXAddress do addr2.family.should eq(addr1.family) addr2.path.should eq(addr1.path) + addr2.abstract?.should eq(addr1.abstract?) addr2.to_s.should eq("/tmp/service.sock") end + it "transforms an abstract address into a C struct and back" do + path = "/abstract-service.sock" + + addr1 = Socket::UNIXAddress.new(path, abstract: true) + addr1.path.should eq(path) + addr1.abstract?.should be_true + + sockaddr_un = addr1.to_unsafe.as(LibC::SockaddrUn*).value + sockaddr_un.sun_path[0].should eq(0_u8) + String.new(sockaddr_un.sun_path.to_unsafe + 1).should eq(path) + + addr2 = Socket::UNIXAddress.new(pointerof(sockaddr_un), nil) + addr2.path.should eq(path) + addr2.abstract?.should be_true + end + it "raises when path is too long" do path = "/tmp/crystal-test-too-long-unix-socket-#{("a" * 2048)}.sock" expect_raises(ArgumentError, "Path size exceeds the maximum size") { Socket::UNIXAddress.new(path) } end it "to_s" do - Socket::UNIXAddress.new("some_path").to_s.should eq("some_path") + path = "some_path" + Socket::UNIXAddress.new(path).to_s.should eq(path) + Socket::UNIXAddress.new(path, abstract: true).to_s.should eq(path) end end @@ -205,6 +224,17 @@ describe UNIXServer do File.exists?(path).should be_false end + it "does not create any file for abstract server" do + path = "/tmp/crystal-test-unix-abstract-sock" + + File.exists?(path).should be_false + + UNIXServer.open(path, abstract: true) do |server| + server.abstract?.should be_true + File.exists?(path).should be_false + end + end + it "deletes socket file on close" do path = "/tmp/crystal-test-unix-sock" @@ -217,6 +247,21 @@ describe UNIXServer do end end + it "does not delete any file on close for abstract server" do + path = "/tmp/crystal-test-close-unix-abstract-sock" + + File.write(path, "") + File.exists?(path).should be_true + + begin + server = UNIXServer.new(path, abstract: true) + server.close + File.exists?(path).should be_true + ensure + File.delete(path) if File.exists?(path) + end + end + it "raises when socket file already exists" do path = "/tmp/crystal-test-unix-sock" server = UNIXServer.new(path) @@ -256,6 +301,19 @@ describe UNIXServer do end end + it "returns an abstract client UNIXSocket for abstract server" do + path = "/tmp/crystal-test-abstract-unix-sock" + + UNIXServer.open(path, abstract: true) do |server| + UNIXSocket.open(path, abstract: true) do |_| + client = server.accept + client.should be_a(UNIXSocket) + client.abstract?.should be_true + client.close + end + end + end + it "raises when server is closed" do server = UNIXServer.new("/tmp/crystal-test-unix-sock") exception = nil @@ -339,6 +397,33 @@ describe UNIXSocket do end end + it "sends and receives messages over an abstract STREAM socket" do + path = "/abstract-service.sock" + + UNIXServer.open(path, abstract: true) do |server| + server.local_address.abstract?.should be_true + server.local_address.path.should eq(path) + + UNIXSocket.open(path, abstract: true) do |client| + client.local_address.abstract?.should be_true + client.local_address.path.should eq(path) + + server.accept do |sock| + sock.local_address.path.should eq("") + sock.local_address.abstract?.should be_true + + sock.remote_address.path.should eq("") + sock.remote_address.abstract?.should be_true + + client << "ping" + sock.gets(4).should eq("ping") + sock << "pong" + client.gets(4).should eq("pong") + end + end + end + end + it "sync flag after accept" do path = "/tmp/crystal-test-unix-sock" diff --git a/src/socket/address.cr b/src/socket/address.cr index 94e7b644a6f7..db446fbab8d0 100644 --- a/src/socket/address.cr +++ b/src/socket/address.cr @@ -179,17 +179,21 @@ class Socket # Holds the local path of an UNIX address, usually coming from an opened # connection (e.g. `Socket#local_address`, `Socket#receive`). # + # You may also declare an abstract UNIX address, that is a virtual file + # that will never be created on the filesystem. + # # Example: # ``` # Socket::UNIXAddress.new("/tmp/my.sock") # ``` struct UNIXAddress < Address getter path : String + getter? abstract : Bool # :nodoc: MAX_PATH_SIZE = LibC::SockaddrUn.new.sun_path.size - 1 - def initialize(@path : String) + def initialize(@path : String, @abstract = false) if @path.bytesize + 1 > MAX_PATH_SIZE raise ArgumentError.new("Path size exceeds the maximum size of #{MAX_PATH_SIZE} bytes") end @@ -204,7 +208,16 @@ class Socket protected def initialize(sockaddr : LibC::SockaddrUn*, size) @family = Family::UNIX - @path = String.new(sockaddr.value.sun_path.to_unsafe) + + path = sockaddr.value.sun_path + if path[0] == 0 + @abstract = true + @path = String.new(path.to_unsafe + 1) + else + @abstract = false + @path = String.new(path.to_unsafe) + end + @size = size || sizeof(LibC::SockaddrUn) end @@ -219,7 +232,14 @@ class Socket def to_unsafe : LibC::Sockaddr* sockaddr = Pointer(LibC::SockaddrUn).malloc sockaddr.value.sun_family = family - sockaddr.value.sun_path.to_unsafe.copy_from(@path.to_unsafe, @path.bytesize + 1) + + destination = sockaddr.value.sun_path.to_unsafe + if @abstract + destination[0] = 0_u8 + destination += 1 + end + destination.copy_from(@path.to_unsafe, @path.bytesize + 1) + sockaddr.as(LibC::Sockaddr*) end end diff --git a/src/socket/unix_server.cr b/src/socket/unix_server.cr index ea8f758e4257..8e6c1328cfc8 100644 --- a/src/socket/unix_server.cr +++ b/src/socket/unix_server.cr @@ -24,17 +24,18 @@ class UNIXServer < UNIXSocket # Creates a named UNIX socket, listening on a filesystem pathname. # # Always deletes any existing filesystam pathname first, in order to cleanup - # any leftover socket file. + # + # Note: An abstract UNIX server act on virtual files, thus not creating nor deleting anything. # # The server is of stream type by default, but this can be changed for # another type. For example datagram messages: # ``` # UNIXServer.new("/tmp/dgram.sock", Socket::Type::DGRAM) # ``` - def initialize(@path : String, type : Type = Type::STREAM, backlog = 128) - super(Family::UNIX, type) + def initialize(@path : String, type : Type = Type::STREAM, backlog = 128, abstract is_abstract = false) + super(Family::UNIX, type, is_abstract) - bind(UNIXAddress.new(path)) do |error| + bind(UNIXAddress.new(path, is_abstract)) do |error| close(delete: false) raise error end @@ -49,8 +50,8 @@ class UNIXServer < UNIXSocket # server socket when the block returns. # # Returns the value of the block. - def self.open(path, type : Type = Type::STREAM, backlog = 128) - server = new(path, type, backlog) + def self.open(path, type : Type = Type::STREAM, backlog = 128, abstract is_abstract = false) + server = new(path, type, backlog, is_abstract) begin yield server ensure @@ -64,7 +65,7 @@ class UNIXServer < UNIXSocket # this method. def accept? : UNIXSocket? if client_fd = accept_impl - sock = UNIXSocket.new(client_fd, type) + sock = UNIXSocket.new(client_fd, type, @abstract) sock.sync = sync? sock end @@ -74,9 +75,9 @@ class UNIXServer < UNIXSocket def close(delete = true) super() ensure - if delete && (path = @path) + if !abstract? && delete && (path = @path) File.delete(path) if File.exists?(path) - @path = nil end + @path = nil end end diff --git a/src/socket/unix_socket.cr b/src/socket/unix_socket.cr index 777941efca6f..40afe3769638 100644 --- a/src/socket/unix_socket.cr +++ b/src/socket/unix_socket.cr @@ -1,4 +1,4 @@ -# A local interprocess communication clientsocket. +# A local interprocess communication client socket. # # Only available on UNIX and UNIX-like operating systems. # @@ -13,22 +13,23 @@ # ``` class UNIXSocket < Socket getter path : String? + getter? abstract : Bool - # Connects a named UNIX socket, bound to a filesystem pathname. - def initialize(@path : String, type : Type = Type::STREAM) + # Connects a named UNIX socket, bound to a filesystem. + def initialize(@path : String, type : Type = Type::STREAM, @abstract = false) super(Family::UNIX, type, Protocol::IP) - connect(UNIXAddress.new(path)) do |error| + connect(UNIXAddress.new(path, @abstract)) do |error| close raise error end end - protected def initialize(family : Family, type : Type) + protected def initialize(family : Family, type : Type, @abstract = false) super family, type, Protocol::IP end - protected def initialize(fd : Int32, type : Type) + protected def initialize(fd : Int32, type : Type, @abstract = false) super fd, Family::UNIX, type, Protocol::IP end @@ -36,8 +37,8 @@ class UNIXSocket < Socket # eventually closes the socket when the block returns. # # Returns the value of the block. - def self.open(path, type : Type = Type::STREAM) - sock = new(path, type) + def self.open(path, type : Type = Type::STREAM, abstract is_abstract = false) + sock = new(path, type, is_abstract) begin yield sock ensure @@ -89,11 +90,11 @@ class UNIXSocket < Socket end def local_address - UNIXAddress.new(path.to_s) + UNIXAddress.new(path.to_s, @abstract) end def remote_address - UNIXAddress.new(path.to_s) + UNIXAddress.new(path.to_s, @abstract) end def receive From 66bd0ef225598ce82de21aebb2c0b5fe22236ea2 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Tue, 28 Nov 2017 01:39:21 +0100 Subject: [PATCH 2/9] Allow abstract UNIX socket with '\0` path prefix' --- spec/std/socket_spec.cr | 72 +++++++++++++++++++++++++++------------ src/socket/address.cr | 20 ++++++++--- src/socket/unix_server.cr | 13 ++++--- src/socket/unix_socket.cr | 25 ++++++++++---- 4 files changed, 92 insertions(+), 38 deletions(-) diff --git a/spec/std/socket_spec.cr b/spec/std/socket_spec.cr index 5367dc7bb2bd..4346866d8524 100644 --- a/spec/std/socket_spec.cr +++ b/spec/std/socket_spec.cr @@ -170,19 +170,23 @@ end describe Socket::UNIXAddress do it "transforms into a C struct and back" do - addr1 = Socket::UNIXAddress.new("/tmp/service.sock") - addr2 = Socket::UNIXAddress.from(addr1.to_unsafe, addr1.size) + path = "/tmp/service.sock" + + addr1 = Socket::UNIXAddress.new(path) + addr1.abstract?.should be_false + addr1.path.should eq(path) + addr2 = Socket::UNIXAddress.from(addr1.to_unsafe, addr1.size) addr2.family.should eq(addr1.family) addr2.path.should eq(addr1.path) addr2.abstract?.should eq(addr1.abstract?) - addr2.to_s.should eq("/tmp/service.sock") + addr2.to_s.should eq(path) end it "transforms an abstract address into a C struct and back" do path = "/abstract-service.sock" - addr1 = Socket::UNIXAddress.new(path, abstract: true) + addr1 = Socket::UNIXAddress.new('\0' + path) addr1.path.should eq(path) addr1.abstract?.should be_true @@ -191,8 +195,8 @@ describe Socket::UNIXAddress do String.new(sockaddr_un.sun_path.to_unsafe + 1).should eq(path) addr2 = Socket::UNIXAddress.new(pointerof(sockaddr_un), nil) - addr2.path.should eq(path) - addr2.abstract?.should be_true + addr2.path.should eq(addr1.path) + addr2.abstract?.should eq(addr1.abstract?) end it "raises when path is too long" do @@ -201,9 +205,11 @@ describe Socket::UNIXAddress do end it "to_s" do - path = "some_path" - Socket::UNIXAddress.new(path).to_s.should eq(path) - Socket::UNIXAddress.new(path, abstract: true).to_s.should eq(path) + # normal UNIX address + Socket::UNIXAddress.new("some_path").to_s.should eq("some_path") + + # abstract UNIX address + Socket::UNIXAddress.new("\0some_path").to_s.should eq("@some_path") end end @@ -214,6 +220,22 @@ describe UNIXServer do File.exists?(path).should be_false end + it "is not abstract when path is not an abstract address" do + path = "/tmp/crystal-test-unix-sock" + + UNIXServer.open(path) do |server| + server.abstract?.should be_false + end + end + + it "is abstract when path is an abstract address" do + path = "/tmp/crystal-test-unix-abstract-sock" + + UNIXServer.open('\0' + path) do |server| + server.abstract?.should be_true + end + end + it "creates the socket file" do path = "/tmp/crystal-test-unix-sock" @@ -229,8 +251,7 @@ describe UNIXServer do File.exists?(path).should be_false - UNIXServer.open(path, abstract: true) do |server| - server.abstract?.should be_true + UNIXServer.open('\0' + path) do File.exists?(path).should be_false end end @@ -250,11 +271,11 @@ describe UNIXServer do it "does not delete any file on close for abstract server" do path = "/tmp/crystal-test-close-unix-abstract-sock" - File.write(path, "") + File.touch(path) File.exists?(path).should be_true begin - server = UNIXServer.new(path, abstract: true) + server = UNIXServer.new('\0' + path) server.close File.exists?(path).should be_true ensure @@ -276,7 +297,7 @@ describe UNIXServer do it "won't delete existing file on bind failure" do path = "/tmp/crystal-test-unix.sock" - File.write(path, "") + File.touch(path) File.exists?(path).should be_true begin @@ -292,10 +313,13 @@ describe UNIXServer do describe "accept" do it "returns the client UNIXSocket" do - UNIXServer.open("/tmp/crystal-test-unix-sock") do |server| - UNIXSocket.open("/tmp/crystal-test-unix-sock") do |_| + path = "/tmp/crystal-test-unix-sock" + + UNIXServer.open(path) do |server| + UNIXSocket.open(path) do |_| client = server.accept client.should be_a(UNIXSocket) + client.abstract?.should be_false client.close end end @@ -303,9 +327,10 @@ describe UNIXServer do it "returns an abstract client UNIXSocket for abstract server" do path = "/tmp/crystal-test-abstract-unix-sock" + abstract_path = '\0' + path - UNIXServer.open(path, abstract: true) do |server| - UNIXSocket.open(path, abstract: true) do |_| + UNIXServer.open(abstract_path) do |server| + UNIXSocket.open(abstract_path) do |_| client = server.accept client.should be_a(UNIXSocket) client.abstract?.should be_true @@ -338,8 +363,10 @@ describe UNIXServer do describe "accept?" do it "returns the client UNIXSocket" do - UNIXServer.open("/tmp/crystal-test-unix-sock") do |server| - UNIXSocket.open("/tmp/crystal-test-unix-sock") do |_| + path = "/tmp/crystal-test-unix-sock" + + UNIXServer.open(path) do |server| + UNIXSocket.open(path) do |_| client = server.accept?.not_nil! client.should be_a(UNIXSocket) client.close @@ -399,12 +426,13 @@ describe UNIXSocket do it "sends and receives messages over an abstract STREAM socket" do path = "/abstract-service.sock" + abstract_path = '\0' + path - UNIXServer.open(path, abstract: true) do |server| + UNIXServer.open(abstract_path) do |server| server.local_address.abstract?.should be_true server.local_address.path.should eq(path) - UNIXSocket.open(path, abstract: true) do |client| + UNIXSocket.open(abstract_path) do |client| client.local_address.abstract?.should be_true client.local_address.path.should eq(path) diff --git a/src/socket/address.cr b/src/socket/address.cr index db446fbab8d0..fff0228352f2 100644 --- a/src/socket/address.cr +++ b/src/socket/address.cr @@ -180,7 +180,9 @@ class Socket # connection (e.g. `Socket#local_address`, `Socket#receive`). # # You may also declare an abstract UNIX address, that is a virtual file - # that will never be created on the filesystem. + # that will never be created on the filesystem. An abstract UNIX address + # path has a NUL byte (`\0`) prefix. + # NOTE: abstract UNIX addresses are supported only on some Linux systems. # # Example: # ``` @@ -193,10 +195,17 @@ class Socket # :nodoc: MAX_PATH_SIZE = LibC::SockaddrUn.new.sun_path.size - 1 - def initialize(@path : String, @abstract = false) - if @path.bytesize + 1 > MAX_PATH_SIZE + def initialize(path : String) + if path.bytesize + 1 > MAX_PATH_SIZE raise ArgumentError.new("Path size exceeds the maximum size of #{MAX_PATH_SIZE} bytes") end + if path[0]? == '\0' + @abstract = true + @path = path[1..-1] + else + @abstract = false + @path = path + end @family = Family::UNIX @size = sizeof(LibC::SockaddrUn) end @@ -210,7 +219,7 @@ class Socket @family = Family::UNIX path = sockaddr.value.sun_path - if path[0] == 0 + if path[0]? == 0 @abstract = true @path = String.new(path.to_unsafe + 1) else @@ -226,6 +235,9 @@ class Socket end def to_s(io) + if abstract? + io << '@' + end io << path end diff --git a/src/socket/unix_server.cr b/src/socket/unix_server.cr index 8e6c1328cfc8..11af17d62480 100644 --- a/src/socket/unix_server.cr +++ b/src/socket/unix_server.cr @@ -32,10 +32,13 @@ class UNIXServer < UNIXSocket # ``` # UNIXServer.new("/tmp/dgram.sock", Socket::Type::DGRAM) # ``` - def initialize(@path : String, type : Type = Type::STREAM, backlog = 128, abstract is_abstract = false) - super(Family::UNIX, type, is_abstract) + def initialize(path : String, type : Type = Type::STREAM, backlog = 128) + super(Family::UNIX, type) - bind(UNIXAddress.new(path, is_abstract)) do |error| + addr = UNIXAddress.new(path) + @abstract = addr.abstract? + @path = addr.path + bind(addr) do |error| close(delete: false) raise error end @@ -50,8 +53,8 @@ class UNIXServer < UNIXSocket # server socket when the block returns. # # Returns the value of the block. - def self.open(path, type : Type = Type::STREAM, backlog = 128, abstract is_abstract = false) - server = new(path, type, backlog, is_abstract) + def self.open(path, type : Type = Type::STREAM, backlog = 128) + server = new(path, type, backlog) begin yield server ensure diff --git a/src/socket/unix_socket.cr b/src/socket/unix_socket.cr index 40afe3769638..c41555ad8aef 100644 --- a/src/socket/unix_socket.cr +++ b/src/socket/unix_socket.cr @@ -15,11 +15,14 @@ class UNIXSocket < Socket getter path : String? getter? abstract : Bool - # Connects a named UNIX socket, bound to a filesystem. - def initialize(@path : String, type : Type = Type::STREAM, @abstract = false) + # Connects a named UNIX socket, bound to a path. + def initialize(path : String, type : Type = Type::STREAM) super(Family::UNIX, type, Protocol::IP) - connect(UNIXAddress.new(path, @abstract)) do |error| + addr = UNIXAddress.new(path) + @abstract = addr.abstract? + @path = addr.path + connect(addr) do |error| close raise error end @@ -37,8 +40,8 @@ class UNIXSocket < Socket # eventually closes the socket when the block returns. # # Returns the value of the block. - def self.open(path, type : Type = Type::STREAM, abstract is_abstract = false) - sock = new(path, type, is_abstract) + def self.open(path, type : Type = Type::STREAM) + sock = new(path, type) begin yield sock ensure @@ -90,11 +93,19 @@ class UNIXSocket < Socket end def local_address - UNIXAddress.new(path.to_s, @abstract) + if abstract? + UNIXAddress.new('\0' + @path.to_s) + else + UNIXAddress.new(@path.to_s) + end end def remote_address - UNIXAddress.new(path.to_s, @abstract) + if abstract? + UNIXAddress.new('\0' + @path.to_s) + else + UNIXAddress.new(@path.to_s) + end end def receive From a02c69ebab8dea953ac7c90b4d4cc0097b4eaf06 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Tue, 28 Nov 2017 14:30:49 +0100 Subject: [PATCH 3/9] Allow UNIX address on Linux only, raise on other plateform --- src/socket/address.cr | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/socket/address.cr b/src/socket/address.cr index fff0228352f2..435a096c923d 100644 --- a/src/socket/address.cr +++ b/src/socket/address.cr @@ -187,6 +187,9 @@ class Socket # Example: # ``` # Socket::UNIXAddress.new("/tmp/my.sock") + # + # # Abstract UNIX socket on Linux only + # Socket::UNIXAddress.new("\0/my.sock") # ``` struct UNIXAddress < Address getter path : String @@ -199,13 +202,19 @@ class Socket if path.bytesize + 1 > MAX_PATH_SIZE raise ArgumentError.new("Path size exceeds the maximum size of #{MAX_PATH_SIZE} bytes") end + if path[0]? == '\0' - @abstract = true - @path = path[1..-1] + {% if flag?(:linux) %} + @abstract = true + @path = path[1..-1] + {% else %} + raise ArgumentError.new("Unsupported: abstract UNIX socket are not supported on non-Linux") + {% end %} else @abstract = false @path = path end + @family = Family::UNIX @size = sizeof(LibC::SockaddrUn) end @@ -220,8 +229,12 @@ class Socket path = sockaddr.value.sun_path if path[0]? == 0 - @abstract = true - @path = String.new(path.to_unsafe + 1) + {% if flag?(:linux) %} + @abstract = true + @path = String.new(path.to_unsafe + 1) + {% else %} + raise ArgumentError.new("Unsupported: abstract UNIX socket are not supported on non-Linux") + {% end %} else @abstract = false @path = String.new(path.to_unsafe) From 54d6dda6f687fdae199549b40adc8c2ed788ad6c Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Tue, 28 Nov 2017 14:31:00 +0100 Subject: [PATCH 4/9] Run abstract UNIX specs on Linux only --- spec/std/socket_spec.cr | 182 ++++++++++++++++++++++++---------------- 1 file changed, 110 insertions(+), 72 deletions(-) diff --git a/spec/std/socket_spec.cr b/spec/std/socket_spec.cr index 4346866d8524..7871d173c3cd 100644 --- a/spec/std/socket_spec.cr +++ b/spec/std/socket_spec.cr @@ -169,6 +169,28 @@ describe Socket::IPAddress do end describe Socket::UNIXAddress do + {% if flag?(:linux) %} + it "can be abstract on Linux" do + path = "/abstract.sock" + addr = Socket::UNIXAddress.new('\0' + path) + addr.abstract?.should be_true + addr.path.should eq(path) + end + {% else %} + it "raises for abstract UNIX address on non-Linux" do + expect_raises(ArgumentError, /not supported/) do + Socket::UNIXAddress.new("\0/abstract.sock") + end + end + + it "is not abstract on non-Linux" do + path = "/tmp/unix.sock" + addr = Socket::UNIXAddress.new(path) + addr.abstract?.should be_false + addr.path.should eq(path) + end + {% end %} + it "transforms into a C struct and back" do path = "/tmp/service.sock" @@ -183,21 +205,23 @@ describe Socket::UNIXAddress do addr2.to_s.should eq(path) end - it "transforms an abstract address into a C struct and back" do - path = "/abstract-service.sock" + {% if flag?(:linux) %} + it "transforms an abstract address into a C struct and back" do + path = "/abstract-service.sock" - addr1 = Socket::UNIXAddress.new('\0' + path) - addr1.path.should eq(path) - addr1.abstract?.should be_true + addr1 = Socket::UNIXAddress.new('\0' + path) + addr1.path.should eq(path) + addr1.abstract?.should be_true - sockaddr_un = addr1.to_unsafe.as(LibC::SockaddrUn*).value - sockaddr_un.sun_path[0].should eq(0_u8) - String.new(sockaddr_un.sun_path.to_unsafe + 1).should eq(path) + sockaddr_un = addr1.to_unsafe.as(LibC::SockaddrUn*).value + sockaddr_un.sun_path[0].should eq(0_u8) + String.new(sockaddr_un.sun_path.to_unsafe + 1).should eq(path) - addr2 = Socket::UNIXAddress.new(pointerof(sockaddr_un), nil) - addr2.path.should eq(addr1.path) - addr2.abstract?.should eq(addr1.abstract?) - end + addr2 = Socket::UNIXAddress.new(pointerof(sockaddr_un), nil) + addr2.path.should eq(addr1.path) + addr2.abstract?.should eq(addr1.abstract?) + end + {% end %} it "raises when path is too long" do path = "/tmp/crystal-test-too-long-unix-socket-#{("a" * 2048)}.sock" @@ -205,12 +229,14 @@ describe Socket::UNIXAddress do end it "to_s" do - # normal UNIX address Socket::UNIXAddress.new("some_path").to_s.should eq("some_path") - - # abstract UNIX address - Socket::UNIXAddress.new("\0some_path").to_s.should eq("@some_path") end + + {% if flag?(:linux) %} + it "to_s for abstract UNIX address" do + Socket::UNIXAddress.new("\0some_path").to_s.should eq("@some_path") + end + {% end %} end describe UNIXServer do @@ -220,21 +246,25 @@ describe UNIXServer do File.exists?(path).should be_false end - it "is not abstract when path is not an abstract address" do - path = "/tmp/crystal-test-unix-sock" + {% if flag?(:linux) %} + it "is not abstract when path is not an abstract address" do + path = "/tmp/crystal-test-unix-sock" - UNIXServer.open(path) do |server| - server.abstract?.should be_false + UNIXServer.open(path) do |server| + server.abstract?.should be_false + end end - end + {% end %} - it "is abstract when path is an abstract address" do - path = "/tmp/crystal-test-unix-abstract-sock" + {% if flag?(:linux) %} + it "is abstract when path is an abstract address" do + path = "/tmp/crystal-test-unix-abstract-sock" - UNIXServer.open('\0' + path) do |server| - server.abstract?.should be_true + UNIXServer.open('\0' + path) do |server| + server.abstract?.should be_true + end end - end + {% end %} it "creates the socket file" do path = "/tmp/crystal-test-unix-sock" @@ -246,15 +276,17 @@ describe UNIXServer do File.exists?(path).should be_false end - it "does not create any file for abstract server" do - path = "/tmp/crystal-test-unix-abstract-sock" + {% if flag?(:linux) %} + it "does not create any file for abstract server" do + path = "/tmp/crystal-test-unix-abstract-sock" - File.exists?(path).should be_false - - UNIXServer.open('\0' + path) do File.exists?(path).should be_false + + UNIXServer.open('\0' + path) do + File.exists?(path).should be_false + end end - end + {% end %} it "deletes socket file on close" do path = "/tmp/crystal-test-unix-sock" @@ -268,20 +300,22 @@ describe UNIXServer do end end - it "does not delete any file on close for abstract server" do - path = "/tmp/crystal-test-close-unix-abstract-sock" - - File.touch(path) - File.exists?(path).should be_true + {% if flag?(:linux) %} + it "does not delete any file on close for abstract server" do + path = "/tmp/crystal-test-close-unix-abstract-sock" - begin - server = UNIXServer.new('\0' + path) - server.close + File.touch(path) File.exists?(path).should be_true - ensure - File.delete(path) if File.exists?(path) + + begin + server = UNIXServer.new('\0' + path) + server.close + File.exists?(path).should be_true + ensure + File.delete(path) if File.exists?(path) + end end - end + {% end %} it "raises when socket file already exists" do path = "/tmp/crystal-test-unix-sock" @@ -325,19 +359,21 @@ describe UNIXServer do end end - it "returns an abstract client UNIXSocket for abstract server" do - path = "/tmp/crystal-test-abstract-unix-sock" - abstract_path = '\0' + path + {% if flag?(:linux) %} + it "returns an abstract client UNIXSocket for abstract server" do + path = "/tmp/crystal-test-abstract-unix-sock" + abstract_path = '\0' + path - UNIXServer.open(abstract_path) do |server| - UNIXSocket.open(abstract_path) do |_| - client = server.accept - client.should be_a(UNIXSocket) - client.abstract?.should be_true - client.close + UNIXServer.open(abstract_path) do |server| + UNIXSocket.open(abstract_path) do |_| + client = server.accept + client.should be_a(UNIXSocket) + client.abstract?.should be_true + client.close + end end end - end + {% end %} it "raises when server is closed" do server = UNIXServer.new("/tmp/crystal-test-unix-sock") @@ -424,33 +460,35 @@ describe UNIXSocket do end end - it "sends and receives messages over an abstract STREAM socket" do - path = "/abstract-service.sock" - abstract_path = '\0' + path + {% if flag?(:linux) %} + it "sends and receives messages over an abstract STREAM socket" do + path = "/abstract-service.sock" + abstract_path = '\0' + path - UNIXServer.open(abstract_path) do |server| - server.local_address.abstract?.should be_true - server.local_address.path.should eq(path) + UNIXServer.open(abstract_path) do |server| + server.local_address.abstract?.should be_true + server.local_address.path.should eq(path) - UNIXSocket.open(abstract_path) do |client| - client.local_address.abstract?.should be_true - client.local_address.path.should eq(path) + UNIXSocket.open(abstract_path) do |client| + client.local_address.abstract?.should be_true + client.local_address.path.should eq(path) - server.accept do |sock| - sock.local_address.path.should eq("") - sock.local_address.abstract?.should be_true + server.accept do |sock| + sock.local_address.path.should eq("") + sock.local_address.abstract?.should be_true - sock.remote_address.path.should eq("") - sock.remote_address.abstract?.should be_true + sock.remote_address.path.should eq("") + sock.remote_address.abstract?.should be_true - client << "ping" - sock.gets(4).should eq("ping") - sock << "pong" - client.gets(4).should eq("pong") + client << "ping" + sock.gets(4).should eq("ping") + sock << "pong" + client.gets(4).should eq("pong") + end end end end - end + {% end %} it "sync flag after accept" do path = "/tmp/crystal-test-unix-sock" From a802ee927364f388c05d9a8f6137dabd7c4ef8e2 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Tue, 28 Nov 2017 19:31:18 +0100 Subject: [PATCH 5/9] Use `@` as address prefix for abstract sockets --- spec/std/socket_spec.cr | 37 ++++++++++++++++++++++++++++--------- src/socket/address.cr | 16 ++++++++++------ src/socket/unix_socket.cr | 4 ++-- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/spec/std/socket_spec.cr b/spec/std/socket_spec.cr index 7871d173c3cd..362e3ad72ced 100644 --- a/spec/std/socket_spec.cr +++ b/spec/std/socket_spec.cr @@ -172,14 +172,14 @@ describe Socket::UNIXAddress do {% if flag?(:linux) %} it "can be abstract on Linux" do path = "/abstract.sock" - addr = Socket::UNIXAddress.new('\0' + path) + addr = Socket::UNIXAddress.new('@' + path) addr.abstract?.should be_true addr.path.should eq(path) end {% else %} it "raises for abstract UNIX address on non-Linux" do expect_raises(ArgumentError, /not supported/) do - Socket::UNIXAddress.new("\0/abstract.sock") + Socket::UNIXAddress.new("@/abstract.sock") end end @@ -191,6 +191,13 @@ describe Socket::UNIXAddress do end {% end %} + it "can create a non-abstract UNIX address starting with @" do + path = "./@non-abstract.sock" + addr = Socket::UNIXAddress.new(path) + addr.abstract?.should be_false + addr.path.should eq(path) + end + it "transforms into a C struct and back" do path = "/tmp/service.sock" @@ -209,7 +216,7 @@ describe Socket::UNIXAddress do it "transforms an abstract address into a C struct and back" do path = "/abstract-service.sock" - addr1 = Socket::UNIXAddress.new('\0' + path) + addr1 = Socket::UNIXAddress.new('@' + path) addr1.path.should eq(path) addr1.abstract?.should be_true @@ -234,7 +241,7 @@ describe Socket::UNIXAddress do {% if flag?(:linux) %} it "to_s for abstract UNIX address" do - Socket::UNIXAddress.new("\0some_path").to_s.should eq("@some_path") + Socket::UNIXAddress.new("@some_path").to_s.should eq("@some_path") end {% end %} end @@ -260,7 +267,7 @@ describe UNIXServer do it "is abstract when path is an abstract address" do path = "/tmp/crystal-test-unix-abstract-sock" - UNIXServer.open('\0' + path) do |server| + UNIXServer.open('@' + path) do |server| server.abstract?.should be_true end end @@ -276,13 +283,25 @@ describe UNIXServer do File.exists?(path).should be_false end + it "creates the socket file with path starting with @" do + path = "./@crystal-test-unix-sock-starting-with-at-symbol" + + File.exists?(path).should be_false + + UNIXServer.open(path) do + File.exists?(path).should be_true + end + + File.exists?(path).should be_false + end + {% if flag?(:linux) %} it "does not create any file for abstract server" do path = "/tmp/crystal-test-unix-abstract-sock" File.exists?(path).should be_false - UNIXServer.open('\0' + path) do + UNIXServer.open('@' + path) do File.exists?(path).should be_false end end @@ -308,7 +327,7 @@ describe UNIXServer do File.exists?(path).should be_true begin - server = UNIXServer.new('\0' + path) + server = UNIXServer.new('@' + path) server.close File.exists?(path).should be_true ensure @@ -362,7 +381,7 @@ describe UNIXServer do {% if flag?(:linux) %} it "returns an abstract client UNIXSocket for abstract server" do path = "/tmp/crystal-test-abstract-unix-sock" - abstract_path = '\0' + path + abstract_path = '@' + path UNIXServer.open(abstract_path) do |server| UNIXSocket.open(abstract_path) do |_| @@ -463,7 +482,7 @@ describe UNIXSocket do {% if flag?(:linux) %} it "sends and receives messages over an abstract STREAM socket" do path = "/abstract-service.sock" - abstract_path = '\0' + path + abstract_path = '@' + path UNIXServer.open(abstract_path) do |server| server.local_address.abstract?.should be_true diff --git a/src/socket/address.cr b/src/socket/address.cr index 435a096c923d..6326e1649777 100644 --- a/src/socket/address.cr +++ b/src/socket/address.cr @@ -181,15 +181,19 @@ class Socket # # You may also declare an abstract UNIX address, that is a virtual file # that will never be created on the filesystem. An abstract UNIX address - # path has a NUL byte (`\0`) prefix. - # NOTE: abstract UNIX addresses are supported only on some Linux systems. + # path is prefixed by a `@` character. + # + # NOTE: + # - Abstract UNIX addresses are supported only on some Linux systems. + # - If you want to use a non-abstract UNIX address starting with a `@`, + # prefix it with `./`, like `./@foo`. # # Example: # ``` # Socket::UNIXAddress.new("/tmp/my.sock") # # # Abstract UNIX socket on Linux only - # Socket::UNIXAddress.new("\0/my.sock") + # Socket::UNIXAddress.new("@/my.sock") # ``` struct UNIXAddress < Address getter path : String @@ -203,12 +207,12 @@ class Socket raise ArgumentError.new("Path size exceeds the maximum size of #{MAX_PATH_SIZE} bytes") end - if path[0]? == '\0' + if path[0]? == '@' {% if flag?(:linux) %} @abstract = true @path = path[1..-1] {% else %} - raise ArgumentError.new("Unsupported: abstract UNIX socket are not supported on non-Linux") + raise ArgumentError.new("Unsupported: cannot use abstract UNIX socket on non-Linux") {% end %} else @abstract = false @@ -233,7 +237,7 @@ class Socket @abstract = true @path = String.new(path.to_unsafe + 1) {% else %} - raise ArgumentError.new("Unsupported: abstract UNIX socket are not supported on non-Linux") + raise ArgumentError.new("Unsupported: cannot use abstract UNIX socket on non-Linux") {% end %} else @abstract = false diff --git a/src/socket/unix_socket.cr b/src/socket/unix_socket.cr index c41555ad8aef..b7a47001886d 100644 --- a/src/socket/unix_socket.cr +++ b/src/socket/unix_socket.cr @@ -94,7 +94,7 @@ class UNIXSocket < Socket def local_address if abstract? - UNIXAddress.new('\0' + @path.to_s) + UNIXAddress.new('@' + @path.to_s) else UNIXAddress.new(@path.to_s) end @@ -102,7 +102,7 @@ class UNIXSocket < Socket def remote_address if abstract? - UNIXAddress.new('\0' + @path.to_s) + UNIXAddress.new('@' + @path.to_s) else UNIXAddress.new(@path.to_s) end From e93d6d71bf96601492aa04997fdeea0bfd6f0284 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Wed, 29 Nov 2017 02:00:07 +0100 Subject: [PATCH 6/9] Various fixes, based on @straight-shoota comments --- spec/std/socket_spec.cr | 28 ++++++++++++++++------------ src/socket/address.cr | 4 ++-- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/spec/std/socket_spec.cr b/spec/std/socket_spec.cr index 362e3ad72ced..02a8db38233f 100644 --- a/spec/std/socket_spec.cr +++ b/spec/std/socket_spec.cr @@ -178,20 +178,20 @@ describe Socket::UNIXAddress do end {% else %} it "raises for abstract UNIX address on non-Linux" do - expect_raises(ArgumentError, /not supported/) do + expect_raises(ArgumentError, "not supported") do Socket::UNIXAddress.new("@/abstract.sock") end end - - it "is not abstract on non-Linux" do - path = "/tmp/unix.sock" - addr = Socket::UNIXAddress.new(path) - addr.abstract?.should be_false - addr.path.should eq(path) - end {% end %} - it "can create a non-abstract UNIX address starting with @" do + it "is not abstract for a non-abstract address" do + path = "/tmp/unix.sock" + addr = Socket::UNIXAddress.new(path) + addr.abstract?.should be_false + addr.path.should eq(path) + end + + it "creates a non-abstract UNIX address starting with @" do path = "./@non-abstract.sock" addr = Socket::UNIXAddress.new(path) addr.abstract?.should be_false @@ -284,11 +284,12 @@ describe UNIXServer do end it "creates the socket file with path starting with @" do - path = "./@crystal-test-unix-sock-starting-with-at-symbol" + path = "@crystal-test-unix-sock-starting-with-at-symbol" + forced_non_abstract_path = "./#{path}" File.exists?(path).should be_false - UNIXServer.open(path) do + UNIXServer.open(forced_non_abstract_path) do File.exists?(path).should be_true end @@ -298,11 +299,14 @@ describe UNIXServer do {% if flag?(:linux) %} it "does not create any file for abstract server" do path = "/tmp/crystal-test-unix-abstract-sock" + abstract_path = '@' + path File.exists?(path).should be_false + File.exists?(abstract_path).should be_false - UNIXServer.open('@' + path) do + UNIXServer.open(abstract_path) do File.exists?(path).should be_false + File.exists?(abstract_path).should be_false end end {% end %} diff --git a/src/socket/address.cr b/src/socket/address.cr index 6326e1649777..522870e7bf83 100644 --- a/src/socket/address.cr +++ b/src/socket/address.cr @@ -207,7 +207,7 @@ class Socket raise ArgumentError.new("Path size exceeds the maximum size of #{MAX_PATH_SIZE} bytes") end - if path[0]? == '@' + if path.starts_with?('@') {% if flag?(:linux) %} @abstract = true @path = path[1..-1] @@ -232,7 +232,7 @@ class Socket @family = Family::UNIX path = sockaddr.value.sun_path - if path[0]? == 0 + if path[0]? == 0_u8 {% if flag?(:linux) %} @abstract = true @path = String.new(path.to_unsafe + 1) From 277770baa98126cdba431eb331508be4418558ed Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Wed, 29 Nov 2017 11:04:26 +0100 Subject: [PATCH 7/9] fixup! --- spec/std/socket_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/std/socket_spec.cr b/spec/std/socket_spec.cr index 02a8db38233f..0d2169edd420 100644 --- a/spec/std/socket_spec.cr +++ b/spec/std/socket_spec.cr @@ -178,7 +178,7 @@ describe Socket::UNIXAddress do end {% else %} it "raises for abstract UNIX address on non-Linux" do - expect_raises(ArgumentError, "not supported") do + expect_raises(ArgumentError, "Unsupported") do Socket::UNIXAddress.new("@/abstract.sock") end end From 5f2f22399492460ae474c1e3928e8e53f985e804 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Fri, 1 Dec 2017 13:59:32 +0100 Subject: [PATCH 8/9] Safer low-level manipulations --- src/socket/address.cr | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/socket/address.cr b/src/socket/address.cr index 522870e7bf83..ecc7f0946356 100644 --- a/src/socket/address.cr +++ b/src/socket/address.cr @@ -262,12 +262,13 @@ class Socket sockaddr = Pointer(LibC::SockaddrUn).malloc sockaddr.value.sun_family = family - destination = sockaddr.value.sun_path.to_unsafe + destination = sockaddr.value.sun_path.to_slice if @abstract destination[0] = 0_u8 destination += 1 end - destination.copy_from(@path.to_unsafe, @path.bytesize + 1) + destination.copy_from(@path.to_unsafe, @path.bytesize) + destination[@path.bytesize] = 0_u8 sockaddr.as(LibC::Sockaddr*) end From 7cddb29883948ef826578049fae4dd46a6cc3e26 Mon Sep 17 00:00:00 2001 From: Benoit de Chezelles Date: Fri, 22 Dec 2017 01:56:31 +0100 Subject: [PATCH 9/9] Note -> NOTE --- src/socket/unix_server.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/socket/unix_server.cr b/src/socket/unix_server.cr index 11af17d62480..f33e6de716ce 100644 --- a/src/socket/unix_server.cr +++ b/src/socket/unix_server.cr @@ -25,7 +25,7 @@ class UNIXServer < UNIXSocket # # Always deletes any existing filesystam pathname first, in order to cleanup # - # Note: An abstract UNIX server act on virtual files, thus not creating nor deleting anything. + # NOTE: An abstract UNIX server act on virtual files, thus not creating nor deleting anything. # # The server is of stream type by default, but this can be changed for # another type. For example datagram messages: