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.

Sign up for GitHub

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

Jump to bottom

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
Closed

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

aetherknight opened this issue Jun 18, 2015 · 5 comments
Labels
JRuby 1.7.x JRuby 9000 stdlib
Milestone
JRuby 9.0.0.0.rc2

Comments

@aetherknight
Copy link

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
Copy link
Member

headius commented Jul 2, 2015

Nice! Investigating.

@headius
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
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 as completed in d12556a Jul 2, 2015
@headius
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
@headius
Properly store and pass protocol for Addrinfo/Socket. Fixes #3067.
4b0fc45 
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
Copy link
Member

headius commented Jul 2, 2015

Fixed for both 9k.rc2 and 1.7.21.

@petermumford petermumford mentioned this issue Jun 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
JRuby 1.7.x JRuby 9000 stdlib
Projects
None yet
Milestone
JRuby 9.0.0.0.rc2
Development

No branches or pull requests
2 participants
@headius @aetherknight