igrigorik / em-synchrony

Fiber aware EventMachine clients and convenience classes
http://www.igvita.com/2010/03/22/untangling-evented-code-with-ruby-fibers
MIT License
1.04k stars 151 forks source link

SSL doesn't work after setting TCPSocket = EventMachine::Synchrony::TCPSocket #82

Open tobowers opened 12 years ago

tobowers commented 12 years ago

minimum to reproduce:


EM.synchrony do
  TCPSocket = EventMachine::Synchrony::TCPSocket
  obj = Net::HTTP.new("google.com", 443)
  obj.use_ssl = true
  obj.verify_mode = OpenSSL::SSL::VERIFY_NONE
  obj.get("/")
  EM.stop
end

triggers:

ruby-1.9.2-p290/lib/ruby/1.9.1/net/http.rb:658:in `initialize': wrong argument type EventMachine::Synchrony::TCPSocket (expected File) (TypeError)
    from /Users/topper/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/net/http.rb:658:in `new'
    from /Users/topper/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/net/http.rb:658:in `connect'
    from /Users/topper/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/net/http.rb:637:in `do_start'
    from /Users/topper/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/net/http.rb:626:in `start'
    from /Users/topper/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/net/http.rb:1168:in `request'
    from /Users/topper/.rvm/rubies/ruby-1.9.2-p290/lib/ruby/1.9.1/net/http.rb:888:in `get'
igrigorik commented 12 years ago

Unfortunately that's not surprising. Existing TCPSocket functionality is very basic, as in it only implements a few methods of the API. The SSL stuff specifically shouldn't be hard to add, but yeah.. I wouldn't even call it a bug, but rather a feature request.

(I should document that TCPSocket class more to be explicit about the minimal functionality)

CodeMonkeySteve commented 12 years ago

I dug into the Ruby C code and it looks like it's failing in SSLSocket because the passed socket isn't a File, i.e. "Check_Type(io, T_FILE)". I don't know how it can work with the original TCPSocket class (which is also not a File).

For whatever it's worth, it doesn't look like EM::TCPSocket is missing anything, it's just that SSL is unreasonably picky about the socket type.

igrigorik commented 12 years ago

Well, I don't think we would want to try to wrap that, we need to use the built-in EM SSL primitives.. which is actually as simple as conn.start_tls(opts). Having said that, looking at net/http, this may mean that we would have to do some heavy patching of the connect method (ugh): https://github.com/ruby/ruby/blob/trunk/lib/net/http.rb#L760-810

tnachen commented 12 years ago

Is this still on the radar of getting worked on or just sitting in the backlog for now? Would really like this to be implemented. Thanks!

igrigorik commented 12 years ago

It's definitely on the backlog, and to be honest.. unlikely to be resolved. I don't think we'll be able to match the SSL API's.

tnachen commented 12 years ago

Is it because there is no time for this or just technical impossible? From the comments it sounded like it's doable.

igrigorik commented 12 years ago

EM's SSL layer and net/http SSL layer are two entirely different beasts.. Perhaps it come be made to work, but I'm not too hopeful. I have a strong feeling that even if we do get the basic case to work, we'll end up running into all kinds of edge cases.