Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JRuby 1.7.20.1 Socket.tcp, Addrinfo.tcp, etc. mistake port for protocol #3067

Closed
aetherknight opened this Issue Jun 18, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@aetherknight
Copy link

aetherknight commented Jun 18, 2015

Observed Behavior

$ jruby --version
jruby 1.7.20.1 (1.9.3p551) 2015-06-10 d7c8c27 on Java HotSpot(TM) 64-Bit Server VM 1.7.0_55-b13 +jit [darwin-x86_64]

$ jruby -rsocket -e "sock = Socket.tcp('www.google.com', 443); p sock"
ArgumentError: unsupported protocol family `__UNKNOWN_CONSTANT__'
        initialize at org/jruby/ext/socket/RubySocket.java:191
               new at org/jruby/RubyIO.java:853
  connect_internal at /Users/bjorvis/.rbenv/versions/jruby-1.7.20.1/lib/ruby/1.9/socket.rb:39
           connect at /Users/bjorvis/.rbenv/versions/jruby-1.7.20.1/lib/ruby/1.9/socket.rb:97
               tcp at /Users/bjorvis/.rbenv/versions/jruby-1.7.20.1/lib/ruby/1.9/socket.rb:268
              each at org/jruby/RubyArray.java:1613
           foreach at /Users/bjorvis/.rbenv/versions/jruby-1.7.20.1/lib/ruby/1.9/socket.rb:178
               tcp at /Users/bjorvis/.rbenv/versions/jruby-1.7.20.1/lib/ruby/1.9/socket.rb:260
            (root) at -e:1

$ jruby -rsocket -e "ai = Addrinfo.tcp('www.google.com', 443); ai.connect; p ai"
ArgumentError: unsupported protocol family `__UNKNOWN_CONSTANT__'
        initialize at org/jruby/ext/socket/RubySocket.java:191
               new at org/jruby/RubyIO.java:853
  connect_internal at /Users/bjorvis/.rbenv/versions/jruby-1.7.20.1/lib/ruby/1.9/socket.rb:39
           connect at /Users/bjorvis/.rbenv/versions/jruby-1.7.20.1/lib/ruby/1.9/socket.rb:97
            (root) at -e:1

Root Cause

JRuby's Addrinfo implementation incorrectly returns the port number as the protocol:

$ jruby -rsocket -e "ai = Addrinfo.tcp('www.google.com', 443); puts ai.afamily; puts ai.socktype; puts ai.protocol"
2
1
443

Source 1.7.20.1:
https://github.com/jruby/jruby/blob/1.7.20.1/core/src/main/java/org/jruby/ext/socket/Addrinfo.java#L225-L228

    @JRubyMethod(notImplemented = true)
    public IRubyObject protocol(ThreadContext context) {
        return context.runtime.newFixnum(port);
    }

It still is a problem in 9.0.0.0.rc1: https://github.com/jruby/jruby/blob/9.0.0.0.rc1/core/src/main/java/org/jruby/ext/socket/Addrinfo.java#L263-L266

MRI Behavior

$ ruby --version
ruby 2.0.0p645 (2015-04-13 revision 50299) [x86_64-darwin14.3.0]

$ ruby -rsocket -e "sock = Socket.tcp('www.google.com', 443); p sock"
#<Socket:fd 9>

$ ruby -rsocket -e "ai = Addrinfo.tcp('www.google.com', 443); ai.connect; p ai"
#<Addrinfo: 216.58.192.4:443 TCP (www.google.com)>

$ ruby -rsocket -e "ai = Addrinfo.tcp('www.google.com', 443); puts ai.afamily; puts ai.socktype; puts ai.protocol"
2
1
6

afamily 2 is PF_INET (ipv4), socktype of 1 is SOCK_STREAM, and protocol 6 is TCP (according to /etc/protocols on Mac OS X)

@headius

This comment has been minimized.

Copy link
Member

headius commented Jul 2, 2015

Nice! Investigating.

@headius

This comment has been minimized.

Copy link
Member

headius commented Jul 2, 2015

The code as written in JRuby was attempting to use the "protocol" argument (port) as "protocol family". This was a misunderstanding on my part. Protocol here means "service" and is passed as the third argument to socket(2).

Interestingly, simply ignoring this parameter appears to work fine. The socket still must be bound to a remote host and port anyway. I believe the socket(2) parameter is used for purposes that have no equivalent on JDK.

@headius

This comment has been minimized.

Copy link
Member

headius commented Jul 2, 2015

Ok, I've figured it out.

The logic here goes like this:

  • Socket.tcp or Addrinfo.tcp create a new Addrinfo using TCP-appropriate address family, domain, and protocol. We were putting port into protocol, when it should actually be something like IPPROTO_TCP.
  • Socket.tcp's connection logic or Addrinfo's connect then pass that protocol through to socket(2). Most domain+family combinations appear to only have one type of protocol, and many just pass zero (e.g. UNIX sockets).

Since we already have branching logic based on domain and family to choose the right kind of IO channel, I am not sure we need to do anything with the protocol (and I'm not sure we can do anything). There are probably some exotic domain+family combinations that have multiple protocols, but it appears to be a relic.

My change will properly save and report protocol from Addrinfo and within Socket constructors, but will not use that protocol for any purpose. This appears to pass your case and may be as close as we need to get.

@headius headius closed this in d12556a Jul 2, 2015

@headius

This comment has been minimized.

Copy link
Member

headius commented Jul 2, 2015

I have fixed this for 9k. I'll see if it's easy to do for 1.7.x as well.

headius added a commit that referenced this issue Jul 2, 2015

Properly store and pass protocol for Addrinfo/Socket. Fixes #3067.
Conflicts:
	core/src/main/java/org/jruby/ext/socket/Addrinfo.java

@headius headius added this to the JRuby 9.0.0.0.rc2 milestone Jul 2, 2015

@headius

This comment has been minimized.

Copy link
Member

headius commented Jul 2, 2015

Fixed for both 9k.rc2 and 1.7.21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.