Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Socket.gethostbyname #2125

Closed
wants to merge 6 commits into from

2 participants

@sdaubert

Update Socket.gethostbyname.

First, spec is updated to add tests on address list to verify that these addresses are in packed format.
In a second commit Socket.gethostbyname is updated for all ruby versions to pass spec.

sdaubert added some commits
@sdaubert sdaubert Update spec library/socket/socket/gethostbyname.
Add two new specs to verify that addresses in address list returned
by Socket.gethosbyname are in 'pack' format (one spec for IPv4
addresses an the other for IPv6 ones).
45a1371
@sdaubert sdaubert Update Socket.gethostbyname to respect two new specs.
Socket.gethostbyname is updated for all ruby versions.

Socket.sockaddr_in is used to transform addresses got from
Socket.getaddrinfo to packed strings. For an IPv4 address,
address is from 5th byte and is 4 bytes long. For an IPv6
address, address is from 8th byte and is 16 bytes long.
9bda567
lib/18/socket.rb
@@ -610,7 +610,17 @@ def self.gethostbyname(hostname)
alternatives = []
addrinfos.each do |a|
alternatives << a[2] unless a[2] == hostname
- addresses << a[3] if a[4] == family
+ # transform addresses to packed strings
+ if a[4] == family
+ sockaddr = Socket.sockaddr_in(1, a[3])
+ if sockaddr.length == 16
@dbussink Owner

We shouldn't just check the length and thus assume a certain layout. It's better to switch on the family, whether it's AF_INET or AF_INET6

Good point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/18/socket.rb
@@ -610,7 +610,17 @@ def self.gethostbyname(hostname)
alternatives = []
addrinfos.each do |a|
alternatives << a[2] unless a[2] == hostname
- addresses << a[3] if a[4] == family
+ # transform addresses to packed strings
+ if a[4] == family
+ sockaddr = Socket.sockaddr_in(1, a[3])
+ if sockaddr.length == 16
+ # IPv4 address
+ addresses << sockaddr[4, 4]
@dbussink Owner

We should also find another way of know which bytes are what. We can't be sure about the exact byte layouts of these so we shouldn't hard code these offsets here.

But these offsets are not magic : they come from C struct definitions:

/* man 7 ip */
           struct sockaddr_in {
               sa_family_t    sin_family;   /* AF_INET */
               in_port_t      sin_port;      /* port number */
               struct in_addr sin_addr;   /* internet address */
           };

           struct in_addr {
               uint32_t       s_addr;
           };

and

/* man 7 ipv6 */
           struct sockaddr_in6 {
               sa_family_t     sin6_family;   /* AF_INET6 */
               in_port_t       sin6_port      /* port number */;
               uint32_t        sin6_flowinfo; /* flow information */
               struct in6_addr sin6_addr;     /* IPv6 adress */
               uint32_t        sin6_scope_id; /* Scope ID */
           };

           struct in6_addr {
               unsigned char   s6_addr[16];
           };
@dbussink Owner

That's exactly my point :). We should get these offsets from struct definitions for these types and not hard code these offsets. They might be different due to platform alignment, type sizes etc. that's why we should depend on what information the platform supplies for these types.

OK. So we can use Socket::SockAddr_In. SockAddr_In#offset_of gives offset of sockaddr_in* fields. But i don't known how to get their size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sdaubert

I have studied Rubinius::FFI, and I think I understand the problem :

  • struct sockaddr_in6 must be defined in rakelib/platform.rake.
  • FFI.config must be used to obtain offsets and sizes for sockaddr_in* fields.

I will try to do that this week.

@dbussink
Owner

Yeah, that sounds like the direction I was also thinking about :). If you have any questions, don't hesitate to ask :). You can also try hopping into #rubinius on IRC to see if people are around there.

@sdaubert sdaubert Pass spec gethostbyname cleanly.
* rakelib/platform.rake:
  Add new structure `sockaddr_in6`
* lib/**/socket.rb :
  * Modify test to determine IP version. Use family
    instead of sockaddr string size.
  * Use platform configuration to determine offset and
    size of IP address in sockaddr string. `sockaddr_in`
    and `sockaddr_in6` platform structures are used for
    this purpose.
77a888c
@sdaubert

Yeah, it works !
Last commit uses platform layout to get packed address from sockaddr string.

lib/18/socket.rb
@@ -610,7 +610,23 @@ def self.gethostbyname(hostname)
alternatives = []
addrinfos.each do |a|
alternatives << a[2] unless a[2] == hostname
- addresses << a[3] if a[4] == family
+ # transform addresses to packed strings
+ if a[4] == family
+ sockaddr = Socket.sockaddr_in(1, a[3])
+ if family == AF_INET
+ # IPv4 address
+ offset = FFI.config("sockaddr_in.sin_addr.offset")
+ size = FFI.config("sockaddr_in.sin_addr.size")
+ addresses << sockaddr[offset, size]
+ elsif family == AF_INET6
+ # Ipv6 address
+ offset = FFI.config("sockaddr_in6.sin6_addr.offset")
+ size = FFI.config("sockaddr_in6.sin6_addr.size")
+ addresses << sockaddr[8,16]
@dbussink Owner

Looks like you're not using the offsets here, but have the hardcoded values left over :)

@dbussink Owner

BTW, since these should be interpreter as binary strings, it's probably better to use byteslice instead of String#[]

Oops ! I missed one.
And i'll add #byteslice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@sdaubert sdaubert Socket.gethostbyname
* Remove hard-coded offset and size for IPv6.
* Replace String#[] by String#byteslice to get address
  from sockaddr string.
e10cc24
@dbussink
Owner

So, one final question, could you perhaps squash the implementation commits into a single commit? So the pull request is 2 commits, one for the specs, one for the implementation?

@sdaubert

I don't know how doing that... I am a git newbie.

sdaubert added some commits
@sdaubert sdaubert Update Socket.gethostbyname to respect two new specs.
Socket.gethostbyname is updated for all ruby versions.

* rakelib/platform.rake:
  Add new structure `sockaddr_in6`

* lib/**/socket.rb :
  Socket.sockaddr_in is used to transform addresses got
  from Socket.getaddrinfo to packed strings. Platform
  configuration is then used to determine offset and size
  of IP address in sockaddr string. `sockaddr_in` and
  `sockaddr_in6` platform structures are used for this
  purpose.
e956c23
@sdaubert sdaubert Merge branch 'socket.gethostbyname' of github.com:sdaubert/rubinius i…
…nto socket.gethostbyname
f5e460f
@sdaubert

I tried something with git rebase, but i'm not sure that's good.
May i do that in another branch ?

@dbussink
Owner

git rebase should work yeah, if you do an interactive rebase you can squash commits together. If you have it right locally, you can force push it to overwrite the changes in the pull request.

Otherwise I can merge it locally on my machine and squash it there.

@sdaubert

It seems I made a mess here. Please merge it locally on your machine.

@dbussink
Owner

Merged this manually

@dbussink dbussink closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 14, 2013
  1. @sdaubert

    Update spec library/socket/socket/gethostbyname.

    sdaubert authored
    Add two new specs to verify that addresses in address list returned
    by Socket.gethosbyname are in 'pack' format (one spec for IPv4
    addresses an the other for IPv6 ones).
  2. @sdaubert

    Update Socket.gethostbyname to respect two new specs.

    sdaubert authored
    Socket.gethostbyname is updated for all ruby versions.
    
    Socket.sockaddr_in is used to transform addresses got from
    Socket.getaddrinfo to packed strings. For an IPv4 address,
    address is from 5th byte and is 4 bytes long. For an IPv6
    address, address is from 8th byte and is 16 bytes long.
Commits on Jan 18, 2013
  1. @sdaubert

    Pass spec gethostbyname cleanly.

    sdaubert authored
    * rakelib/platform.rake:
      Add new structure `sockaddr_in6`
    * lib/**/socket.rb :
      * Modify test to determine IP version. Use family
        instead of sockaddr string size.
      * Use platform configuration to determine offset and
        size of IP address in sockaddr string. `sockaddr_in`
        and `sockaddr_in6` platform structures are used for
        this purpose.
  2. @sdaubert

    Socket.gethostbyname

    sdaubert authored
    * Remove hard-coded offset and size for IPv6.
    * Replace String#[] by String#byteslice to get address
      from sockaddr string.
  3. @sdaubert

    Update Socket.gethostbyname to respect two new specs.

    sdaubert authored
    Socket.gethostbyname is updated for all ruby versions.
    
    * rakelib/platform.rake:
      Add new structure `sockaddr_in6`
    
    * lib/**/socket.rb :
      Socket.sockaddr_in is used to transform addresses got
      from Socket.getaddrinfo to packed strings. Platform
      configuration is then used to determine offset and size
      of IP address in sockaddr string. `sockaddr_in` and
      `sockaddr_in6` platform structures are used for this
      purpose.
  4. @sdaubert
This page is out of date. Refresh to see the latest.
View
18 lib/18/socket.rb
@@ -610,7 +610,23 @@ def self.gethostbyname(hostname)
alternatives = []
addrinfos.each do |a|
alternatives << a[2] unless a[2] == hostname
- addresses << a[3] if a[4] == family
+ # transform addresses to packed strings
+ if a[4] == family
+ sockaddr = Socket.sockaddr_in(1, a[3])
+ if family == AF_INET
+ # IPv4 address
+ offset = FFI.config("sockaddr_in.sin_addr.offset")
+ size = FFI.config("sockaddr_in.sin_addr.size")
+ addresses << sockaddr.byteslice(offset, size)
+ elsif family == AF_INET6
+ # Ipv6 address
+ offset = FFI.config("sockaddr_in6.sin6_addr.offset")
+ size = FFI.config("sockaddr_in6.sin6_addr.size")
+ addresses << sockaddr.byteslice(offset, size)
+ else
+ addresses << a[3]
+ end
+ end
end
[hostname, alternatives.uniq, family] + addresses.uniq
View
18 lib/19/socket.rb
@@ -687,7 +687,23 @@ def self.gethostbyname(hostname)
alternatives = []
addrinfos.each do |a|
alternatives << a[2] unless a[2] == hostname
- addresses << a[3] if a[4] == family
+ # transform addresses to packed strings
+ if a[4] == family
+ sockaddr = Socket.sockaddr_in(1, a[3])
+ if family == AF_INET
+ # IPv4 address
+ offset = FFI.config("sockaddr_in.sin_addr.offset")
+ size = FFI.config("sockaddr_in.sin_addr.size")
+ addresses << sockaddr.byteslice(offset, size)
+ elsif family == AF_INET6
+ # Ipv6 address
+ offset = FFI.config("sockaddr_in6.sin6_addr.offset")
+ size = FFI.config("sockaddr_in6.sin6_addr.size")
+ addresses << sockaddr.byteslice(offset, size)
+ else
+ addresses << a[3]
+ end
+ end
end
[hostname, alternatives.uniq, family] + addresses.uniq
View
18 lib/20/socket.rb
@@ -687,7 +687,23 @@ def self.gethostbyname(hostname)
alternatives = []
addrinfos.each do |a|
alternatives << a[2] unless a[2] == hostname
- addresses << a[3] if a[4] == family
+ # transform addresses to packed strings
+ if a[4] == family
+ sockaddr = Socket.sockaddr_in(1, a[3])
+ if family == AF_INET
+ # IPv4 address
+ offset = FFI.config("sockaddr_in.sin_addr.offset")
+ size = FFI.config("sockaddr_in.sin_addr.size")
+ addresses << sockaddr.byteslice(offset, size)
+ elsif family == AF_INET6
+ # Ipv6 address
+ offset = FFI.config("sockaddr_in6.sin6_addr.offset")
+ size = FFI.config("sockaddr_in6.sin6_addr.size")
+ addresses << sockaddr.byteslice(offset, size)
+ else
+ addresses << a[3]
+ end
+ end
end
[hostname, alternatives.uniq, family] + addresses.uniq
View
17 rakelib/platform.rake
@@ -58,6 +58,23 @@ file 'runtime/platform.conf' => deps do |task|
s.field :sin_zero, :char_array
end.write_config(f)
+ Rubinius::FFI::Generators::Structures.new 'sockaddr_in6' do |s|
+ if BUILD_CONFIG[:windows]
+ s.include "ws2tcpip.h"
+ else
+ s.include "netinet/in.h"
+ s.include "sys/socket.h"
+ end
+ s.include "fcntl.h"
+ s.include "sys/stat.h"
+ s.name 'struct sockaddr_in6'
+ s.field :sin6_family, :sa_family_t
+ s.field :sin6_port, :ushort
+ s.field :sin6_flowinfo
+ s.field :sin6_addr, :char_array
+ s.field :sin6_scope_id
+ end.write_config(f)
+
unless BUILD_CONFIG[:windows]
sockaddr_un = Rubinius::FFI::Generators::Structures.new 'sockaddr_un' do |s|
s.include "sys/un.h"
View
10 spec/ruby/library/socket/socket/gethostbyname_spec.rb
@@ -13,4 +13,14 @@
addr = Socket.gethostbyname('<any>').first;
addr.should == "0.0.0.0"
end
+
+ it "returns address list in pack format (IPv4)" do
+ laddr = Socket.gethostbyname('127.0.0.1')[3..-1];
+ laddr.should == ["\x7f\x00\x00\x01"]
+ end
+
+ it "returns address list in pack format (IPv6)" do
+ laddr = Socket.gethostbyname('::1')[3..-1]
+ laddr.should == ["\x00" * 15 + "\x01"]
+ end
end
Something went wrong with that request. Please try again.