raindrops RubyGem user+dev discussion/patches/pulls/bugs/help
 help / Atom feed
* [PATCH] Raindrops currently fails when provided a symlink to a socket. As this is a common practice for many deployment tools (Vlad, etc.) this patch adds support for finding the realpath prior to looking the socket up in /proc/net/unix
@ 2012-06-05 15:46 Brian Corrigan
  2012-06-05 17:57 ` Re: [PATCH] ... Eric Wong
  0 siblings, 1 reply; 7+ messages in thread
From: Brian Corrigan @ 2012-06-05 15:46 UTC (permalink / raw)
  To: raindrops

---
 lib/raindrops/linux.rb |    3 +++
 test/test_linux.rb     |   26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/lib/raindrops/linux.rb b/lib/raindrops/linux.rb
index fe2af09..a198253 100644
--- a/lib/raindrops/linux.rb
+++ b/lib/raindrops/linux.rb
@@ -8,6 +8,8 @@
 # Instead of snapshotting, Raindrops::Aggregate::LastDataRecv may be used
 # to aggregate statistics from +all+ accepted sockets as they arrive
 # based on the +last_data_recv+ field in Raindrops::TCP_Info
+require 'pathname'
+
 module Raindrops::Linux

   # The standard proc path for active UNIX domain sockets, feel free to call
@@ -41,6 +43,7 @@ module Raindrops::Linux
     else
       paths = paths.map do |path|
         path = path.dup
+        path = Pathname.new(path).realpath.to_s
         path.force_encoding(Encoding::BINARY) if defined?(Encoding)
         rv[path]
         Regexp.escape(path)
diff --git a/test/test_linux.rb b/test/test_linux.rb
index 65f25e0..81463c9 100644
--- a/test/test_linux.rb
+++ b/test/test_linux.rb
@@ -67,6 +67,32 @@ class TestLinux < Test::Unit::TestCase
     assert_equal 1, stats[tmp.path].queued
   end

+  def test_unix_resolves_symlinks
+    tmp = Tempfile.new("\xde\xad\xbe\xef") # valid path, really :)
+    File.unlink(tmp.path)
+    us = UNIXServer.new(tmp.path)
+
+    # Create a symlink
+    destination = Tempfile.new("somethingelse")
+    destination.unlink # We need an available name, not an actual file
+    link = File.symlink(tmp, destination)
+
+    @to_close << UNIXSocket.new(tmp.path)
+    stats = unix_listener_stats
+    assert_equal 0, stats[link.path].active
+    assert_equal 1, stats[link.path].queued
+
+    @to_close << UNIXSocket.new(tmp.path)
+    stats = unix_listener_stats
+    assert_equal 0, stats[link.path].active
+    assert_equal 2, stats[link.path].queued
+
+    @to_close << us.accept
+    stats = unix_listener_stats
+    assert_equal 1, stats[link.path].active
+    assert_equal 1, stats[link.path].queued
+  end
+
   def test_tcp
     s = TCPServer.new(TEST_ADDR, 0)
     port = s.addr[1]
-- 
1.7.0.4


^ permalink raw reply	[flat|threaded] 7+ messages in thread

* Re: [PATCH] ...
  2012-06-05 15:46 [PATCH] Raindrops currently fails when provided a symlink to a socket. As this is a common practice for many deployment tools (Vlad, etc.) this patch adds support for finding the realpath prior to looking the socket up in /proc/net/unix Brian Corrigan
@ 2012-06-05 17:57 ` Eric Wong
  2012-06-05 23:59   ` Brian Corrigan
  0 siblings, 1 reply; 7+ messages in thread
From: Eric Wong @ 2012-06-05 17:57 UTC (permalink / raw)
  To: raindrops

Brian Corrigan <brian@genexp.me> wrote:
> Subject: [PATCH] Raindrops currently fails when provided a
>  symlink to a socket. As this is a common practice for many deployment
>  tools (Vlad, etc.) this patch adds support for finding the realpath prior
>  to looking the socket up in /proc/net/unix

Erhm, I meant write the commit message with a separate short Subject:
line and body (like an email).  Most of the existing commits in
raindrops are written this way, and it's the standard for git.git.

I can rewrite the commit message if you don't have time, the rest
of the patch looks fine.  Thanks!


^ permalink raw reply	[flat|threaded] 7+ messages in thread

* Re: [PATCH] ...
  2012-06-05 17:57 ` Re: [PATCH] ... Eric Wong
@ 2012-06-05 23:59   ` Brian Corrigan
  2012-06-06  0:15     ` Eric Wong
  0 siblings, 1 reply; 7+ messages in thread
From: Brian Corrigan @ 2012-06-05 23:59 UTC (permalink / raw)
  To: raindrops

Hey Eric - can you take it for here?  Also, thanks for the patience :).
Fwiw, it's cool to have code in this project.  I learned a lot by reviewing
the source.

On Tuesday, June 5, 2012, Eric Wong wrote:

> Brian Corrigan <brian@genexp.me <javascript:;>> wrote:
> > Subject: [PATCH] Raindrops currently fails when provided a
> >  symlink to a socket. As this is a common practice for many deployment
> >  tools (Vlad, etc.) this patch adds support for finding the realpath
> prior
> >  to looking the socket up in /proc/net/unix
>
> Erhm, I meant write the commit message with a separate short Subject:
> line and body (like an email).  Most of the existing commits in
> raindrops are written this way, and it's the standard for git.git.
>
> I can rewrite the commit message if you don't have time, the rest
> of the patch looks fine.  Thanks!
>


^ permalink raw reply	[flat|threaded] 7+ messages in thread

* Re: [PATCH] ...
  2012-06-05 23:59   ` Brian Corrigan
@ 2012-06-06  0:15     ` Eric Wong
  2012-06-06  0:49       ` Eric Wong
  0 siblings, 1 reply; 7+ messages in thread
From: Eric Wong @ 2012-06-06  0:15 UTC (permalink / raw)
  To: raindrops

Brian Corrigan <brian@genexp.me> wrote:
> Hey Eric - can you take it for here?  Also, thanks for the patience :).
> Fwiw, it's cool to have code in this project.  I learned a lot by reviewing
> the source.

Sure thing.  Btw, don't top post and don't send HTML parts.  There's a
lot of technical mailing lists that don't welcome these things.

Actually, did you get the test you added running at all?

File.symlink returns 0 on success, so using the return value is bogus.
I'll rewrite it an squash a patch on top of it.


^ permalink raw reply	[flat|threaded] 7+ messages in thread

* Re: [PATCH] ...
  2012-06-06  0:15     ` Eric Wong
@ 2012-06-06  0:49       ` Eric Wong
  2012-06-06  0:50         ` Brian Corrigan
  2012-06-06  0:56         ` [PATCH] unix_listener_stats follows and remembers symlinks Eric Wong
  0 siblings, 2 replies; 7+ messages in thread
From: Eric Wong @ 2012-06-06  0:49 UTC (permalink / raw)
  To: raindrops

Eric Wong <normalperson@yhbt.net> wrote:
> Brian Corrigan <brian@genexp.me> wrote:
> > Hey Eric - can you take it for here?  Also, thanks for the patience :).
> > Fwiw, it's cool to have code in this project.  I learned a lot by reviewing
> > the source.
> 
> Sure thing.  Btw, don't top post and don't send HTML parts.  There's a
> lot of technical mailing lists that don't welcome these things.
> 
> Actually, did you get the test you added running at all?
> 
> File.symlink returns 0 on success, so using the return value is bogus.
> I'll rewrite it an squash a patch on top of it.

I'll squash the following on top of your changes.
However, I think the way unix_listener_stats behaves can pose
some confusing usability problems with symlinks

diff --git a/test/test_linux.rb b/test/test_linux.rb
index 81463c9..1da86cf 100644
--- a/test/test_linux.rb
+++ b/test/test_linux.rb
@@ -73,24 +73,24 @@ class TestLinux < Test::Unit::TestCase
     us = UNIXServer.new(tmp.path)
 
     # Create a symlink
-    destination = Tempfile.new("somethingelse")
-    destination.unlink # We need an available name, not an actual file
-    link = File.symlink(tmp, destination)
+    link = Tempfile.new("somethingelse")
+    File.unlink(link.path) # We need an available name, not an actual file
+    File.symlink(tmp.path, link.path)
 
     @to_close << UNIXSocket.new(tmp.path)
     stats = unix_listener_stats
-    assert_equal 0, stats[link.path].active
-    assert_equal 1, stats[link.path].queued
+    assert_equal 0, stats[tmp.path].active
+    assert_equal 1, stats[tmp.path].queued
 
-    @to_close << UNIXSocket.new(tmp.path)
+    @to_close << UNIXSocket.new(link.path)
     stats = unix_listener_stats
-    assert_equal 0, stats[link.path].active
-    assert_equal 2, stats[link.path].queued
+    assert_equal 0, stats[tmp.path].active
+    assert_equal 2, stats[tmp.path].queued
 
     @to_close << us.accept
     stats = unix_listener_stats
-    assert_equal 1, stats[link.path].active
-    assert_equal 1, stats[link.path].queued
+    assert_equal 1, stats[tmp.path].active
+    assert_equal 1, stats[tmp.path].queued
   end
 
   def test_tcp

-- 
Eric Wong


^ permalink raw reply	[flat|threaded] 7+ messages in thread

* Re: [PATCH] ...
  2012-06-06  0:49       ` Eric Wong
@ 2012-06-06  0:50         ` Brian Corrigan
  2012-06-06  0:56         ` [PATCH] unix_listener_stats follows and remembers symlinks Eric Wong
  1 sibling, 0 replies; 7+ messages in thread
From: Brian Corrigan @ 2012-06-06  0:50 UTC (permalink / raw)
  To: raindrops

On Tue, Jun 5, 2012 at 8:49 PM, Eric Wong <normalperson@yhbt.net> wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
>> Brian Corrigan <brian@genexp.me> wrote:
>> > Hey Eric - can you take it for here?  Also, thanks for the patience :).
>> > Fwiw, it's cool to have code in this project.  I learned a lot by reviewing
>> > the source.
>>
>> Sure thing.  Btw, don't top post and don't send HTML parts.  There's a
>> lot of technical mailing lists that don't welcome these things.
>>
>> Actually, did you get the test you added running at all?
>>
>> File.symlink returns 0 on success, so using the return value is bogus.
>> I'll rewrite it an squash a patch on top of it.
>
> I'll squash the following on top of your changes.
> However, I think the way unix_listener_stats behaves can pose
> some confusing usability problems with symlinks
>
> diff --git a/test/test_linux.rb b/test/test_linux.rb
> index 81463c9..1da86cf 100644
> --- a/test/test_linux.rb
> +++ b/test/test_linux.rb
> @@ -73,24 +73,24 @@ class TestLinux < Test::Unit::TestCase
>     us = UNIXServer.new(tmp.path)
>
>     # Create a symlink
> -    destination = Tempfile.new("somethingelse")
> -    destination.unlink # We need an available name, not an actual file
> -    link = File.symlink(tmp, destination)
> +    link = Tempfile.new("somethingelse")
> +    File.unlink(link.path) # We need an available name, not an actual file
> +    File.symlink(tmp.path, link.path)
>
>     @to_close << UNIXSocket.new(tmp.path)
>     stats = unix_listener_stats
> -    assert_equal 0, stats[link.path].active
> -    assert_equal 1, stats[link.path].queued
> +    assert_equal 0, stats[tmp.path].active
> +    assert_equal 1, stats[tmp.path].queued
>
> -    @to_close << UNIXSocket.new(tmp.path)
> +    @to_close << UNIXSocket.new(link.path)
>     stats = unix_listener_stats
> -    assert_equal 0, stats[link.path].active
> -    assert_equal 2, stats[link.path].queued
> +    assert_equal 0, stats[tmp.path].active
> +    assert_equal 2, stats[tmp.path].queued
>
>     @to_close << us.accept
>     stats = unix_listener_stats
> -    assert_equal 1, stats[link.path].active
> -    assert_equal 1, stats[link.path].queued
> +    assert_equal 1, stats[tmp.path].active
> +    assert_equal 1, stats[tmp.path].queued
>   end
>
>   def test_tcp
>
> --
> Eric Wong

Cheers Eric!

-- 
Brian

^ permalink raw reply	[flat|threaded] 7+ messages in thread

* [PATCH] unix_listener_stats follows and remembers symlinks
  2012-06-06  0:49       ` Eric Wong
  2012-06-06  0:50         ` Brian Corrigan
@ 2012-06-06  0:56         ` Eric Wong
  1 sibling, 0 replies; 7+ messages in thread
From: Eric Wong @ 2012-06-06  0:56 UTC (permalink / raw)
  To: raindrops

Eric Wong <normalperson@yhbt.net> wrote:
> However, I think the way unix_listener_stats behaves can pose
> some confusing usability problems with symlinks

I think the following patch should improve usability with
symlinks and unix_listener_stats:

>From bd7236fe23c4388d2fa42a4f836aca3c796dabab Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Tue, 5 Jun 2012 17:49:43 -0700
Subject: [PATCH] unix_listener_stats follows and remembers symlinks

Teach unix_listener_stats to remember the symlink path
it followed and have it point to the same object as the
resolved (real) socket path.

This allows the case where looking up stats by symlinks
works if the symlink is given to unix_listener_stats:

  File.symlink("/real/path/of.sock", "/path/to/link.sock")
  stats = unix_listener_stats(["/path/to/link.sock"])
  stats["/path/to/link.sock"] => # same as stats["/real/path/of.sock"]
---
 lib/raindrops/linux.rb |  9 +++++++--
 test/test_linux.rb     | 20 +++++++++++---------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/lib/raindrops/linux.rb b/lib/raindrops/linux.rb
index a198253..1752b8a 100644
--- a/lib/raindrops/linux.rb
+++ b/lib/raindrops/linux.rb
@@ -43,9 +43,14 @@ module Raindrops::Linux
     else
       paths = paths.map do |path|
         path = path.dup
-        path = Pathname.new(path).realpath.to_s
         path.force_encoding(Encoding::BINARY) if defined?(Encoding)
-        rv[path]
+        if File.symlink?(path)
+          link = path
+          path = Pathname.new(link).realpath.to_s
+          rv[link] = rv[path] # vivify ListenerStats
+        else
+          rv[path] # vivify ListenerStats
+        end
         Regexp.escape(path)
       end
     end
diff --git a/test/test_linux.rb b/test/test_linux.rb
index 81463c9..a84eecf 100644
--- a/test/test_linux.rb
+++ b/test/test_linux.rb
@@ -73,24 +73,26 @@ class TestLinux < Test::Unit::TestCase
     us = UNIXServer.new(tmp.path)
 
     # Create a symlink
-    destination = Tempfile.new("somethingelse")
-    destination.unlink # We need an available name, not an actual file
-    link = File.symlink(tmp, destination)
+    link = Tempfile.new("somethingelse")
+    File.unlink(link.path) # We need an available name, not an actual file
+    File.symlink(tmp.path, link.path)
 
     @to_close << UNIXSocket.new(tmp.path)
     stats = unix_listener_stats
-    assert_equal 0, stats[link.path].active
-    assert_equal 1, stats[link.path].queued
+    assert_equal 0, stats[tmp.path].active
+    assert_equal 1, stats[tmp.path].queued
 
-    @to_close << UNIXSocket.new(tmp.path)
-    stats = unix_listener_stats
+    @to_close << UNIXSocket.new(link.path)
+    stats = unix_listener_stats([link.path])
     assert_equal 0, stats[link.path].active
     assert_equal 2, stats[link.path].queued
 
+    assert_equal stats[link.path].object_id, stats[tmp.path].object_id
+
     @to_close << us.accept
     stats = unix_listener_stats
-    assert_equal 1, stats[link.path].active
-    assert_equal 1, stats[link.path].queued
+    assert_equal 1, stats[tmp.path].active
+    assert_equal 1, stats[tmp.path].queued
   end
 
   def test_tcp
-- 


^ permalink raw reply	[flat|threaded] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-05 15:46 [PATCH] Raindrops currently fails when provided a symlink to a socket. As this is a common practice for many deployment tools (Vlad, etc.) this patch adds support for finding the realpath prior to looking the socket up in /proc/net/unix Brian Corrigan
2012-06-05 17:57 ` Re: [PATCH] ... Eric Wong
2012-06-05 23:59   ` Brian Corrigan
2012-06-06  0:15     ` Eric Wong
2012-06-06  0:49       ` Eric Wong
2012-06-06  0:50         ` Brian Corrigan
2012-06-06  0:56         ` [PATCH] unix_listener_stats follows and remembers symlinks Eric Wong

raindrops RubyGem user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://bogomips.org/raindrops-public
	git clone --mirror http://ou63pmih66umazou.onion/raindrops-public

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.raindrops
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.raindrops

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox