Merge pull request #77 from celluloid/separate-requests-and-websockets

Separate Requests from WebSockets

CHANGES.md

@@ -2,6 +2,9 @@
 -----
 * Pipelining support
 * Reel::Request#body now returns a Reel::RequestBody object instead of a String
+* New WebSocket API: obtain WebSockets through Reel::Request#websocket instead
+  of through Reel::Connection#request. Allows processing of WebSockets through
+  other means than the built-in WebSocket support
 
 0.4.0
 ----

README.md

@@ -75,15 +75,16 @@ require 'reel'
 
 Reel::Server.supervise("0.0.0.0", 3000) do |connection|
   while request = connection.request
-    case request
-    when Reel::Request
-      puts "Client requested: #{request.method} #{request.url}"
-      request.respond :ok, "Hello, world!"
-    when Reel::WebSocket
+    if request.websocket?
       puts "Client made a WebSocket request to: #{request.url}"
-      request << "Hello everyone out there in WebSocket land"
-      request.close
+      websocket = request.websocket
+
+      websocket << "Hello everyone out there in WebSocket land"
+      websocket.close
       break
+    else
+      puts "Client requested: #{request.method} #{request.url}"
+      request.respond :ok, "Hello, world!"
     end
   end
 end
@@ -109,11 +110,11 @@ class MyServer < Reel::Server
 
   def on_connection(connection)
     while request = connection.request
-      case request
-      when Reel::Request
-        handle_request(request)
-      when Reel::WebSocket
+      if request.websocket?
         handle_websocket(request)
+        break
+      else
+        handle_request(request)
       end
     end
   end
@@ -133,6 +134,7 @@ MyServer.run
 
 Framework Adapters
 ------------------
+
 ### Rack
 
 Reel can be used as a standard Rack server via the "reel" command line
@@ -179,7 +181,7 @@ MyApp = Webmachine::Application.new do |app|
     config.port    = MYAPP_PORT
     config.adapter = :Reel
 
-    # Optional: (WM master only) handler for incoming websockets
+    # Optional: handler for incoming websockets
     config.adapter_options[:websocket_handler] = proc do |websocket|
       # socket is a Reel::WebSocket
       socket << "hello, world"

examples/websockets.rb

@@ -48,12 +48,13 @@ def initialize(host = "127.0.0.1", port = 1234)
 
   def on_connection(connection)
     while request = connection.request
-      case request
-      when Reel::Request
-        route_request connection, request
-      when Reel::WebSocket
+      if request.websocket?
         info "Received a WebSocket connection"
-        route_websocket request
+        connection.detach
+        route_websocket request.websocket
+        return
+      else
+        route_request connection, request
       end
     end
   end

lib/reel/connection.rb

@@ -59,20 +59,12 @@ def current_request
 
     # Read a request object from the connection
     def request
-      return if @request_state == :websocket
-      raise StateError, "current request not responded to" if current_request
-      req = @parser.current_request
+      raise StateError, "already processing a request" if current_request
 
-      case req
-      when Request
-        @request_state = :ready
-        @keepalive = false if req[CONNECTION] == CLOSE || req.version == HTTP_VERSION_1_0
-        @current_request = req
-      when WebSocket
-        @request_state = @response_state = :websocket
-        @socket = SocketUpgradedError
-      else raise "unexpected request type: #{req.class}"
-      end
+      req = @parser.current_request
+      @request_state = :ready
+      @keepalive = false if req[CONNECTION] == CLOSE || req.version == HTTP_VERSION_1_0
+      @current_request = req
 
       req
     rescue IOError, Errno::ECONNRESET, Errno::EPIPE
@@ -141,9 +133,30 @@ def finish_response
 
     # Close the connection
     def close
-      raise StateError, "connection upgraded to Reel::WebSocket, call close on the websocket instance" if @response_state == :websocket
+      raise StateError, "socket has been hijacked from this connection" unless @socket
+
       @keepalive = false
       @socket.close unless @socket.closed?
     end
+
+    # Hijack the socket from the connection
+    def hijack_socket
+      # FIXME: this doesn't do a great job of ensuring we can hijack the socket
+      # in its current state. Improve the state detection.
+      if @request_state != :ready && @response_state != :header
+        raise StateError, "connection is not in a hijackable state"
+      end
+
+      @request_state = @response_state = :hijacked
+      socket  = @socket
+      @socket = nil
+      socket
+    end
+
+    # Raw access to the underlying socket
+    def socket
+      raise StateError, "socket has already been hijacked" unless @socket
+      @socket
+    end
   end
 end

lib/reel/mixins.rb

@@ -10,14 +10,14 @@ module ConnectionMixin
 
     # Obtain the IP address of the remote connection
     def remote_ip
-      @socket.peeraddr(false)[3]
+      socket.peeraddr(false)[3]
     end
-    alias_method :remote_addr, :remote_ip
+    alias remote_addr remote_ip
 
     # Obtain the hostname of the remote connection
     def remote_host
       # NOTE: Celluloid::IO does not yet support non-blocking reverse DNS
-      @socket.peeraddr(true)[2]
+      socket.peeraddr(true)[2]
     end
   end
 

lib/reel/rack_worker.rb

@@ -88,8 +88,8 @@ def request_env request, connection
 
     def websocket_env request
       env = env(request)
-      env[REMOTE_ADDR]    = request.remote_ip
-      env[RACK_WEBSOCKET] = request
+      env[RACK_WEBSOCKET] = request.websocket
+      env[REMOTE_ADDR]    = request.websocket.remote_ip
       env
     end
 

lib/reel/request.rb

@@ -5,17 +5,6 @@ class Request
     extend Forwardable
     include RequestMixin
 
-    def self.build(request_info, connection)
-      request_info.method ||
-        raise(ArgumentError, "Unknown Request Method: %s" % request_info.http_method)
-
-      if request_info.websocket_request?
-        WebSocket.new(request_info, connection.socket)
-      else
-        Request.new(request_info, connection)
-      end
-    end
-
     def_delegators :@connection, :<<, :write, :respond, :finish_response
     attr_reader :body
 
@@ -30,6 +19,7 @@ def initialize(request_info, connection = nil)
       @buffer        = ""
       @body          = RequestBody.new(self)
       @finished_read = false
+      @websocket     = nil
     end
 
     # Returns true if request fully finished reading
@@ -89,5 +79,16 @@ def readpartial(length = nil)
       slice && slice.length == 0 ? nil : slice
     end
 
+    # Can the current request be upgraded to a WebSocket?
+    def websocket?; @request_info.websocket_request?; end
+
+    # Return a Reel::WebSocket for this request, hijacking the socket from
+    # the underlying connection
+    def websocket
+      @websocket ||= begin
+        raise StateError, "can't upgrade this request to a websocket" unless websocket?
+        WebSocket.new(@request_info, @connection.hijack_socket)
+      end
+    end
   end
 end

lib/reel/request_parser.rb

@@ -50,7 +50,7 @@ def readpartial(size = @connection.buffer_size)
       #
       def on_headers_complete(headers)
         info = RequestInfo.new(http_method, url, http_version, headers)
-        req = Request.build(info, connection)
+        req = Request.new(info, connection)
         if @currently_reading.nil?
           @currently_reading = req
         else

lib/reel/websocket.rb

@@ -7,6 +7,7 @@ class WebSocket
     include ConnectionMixin
     include RequestMixin
 
+    attr_reader :socket
     def_delegators :@socket, :addr, :peeraddr
 
     def initialize(info, socket)

spec/reel/websocket_spec.rb

@@ -10,7 +10,10 @@
     with_socket_pair do |client, connection|
       client << handshake.to_data
 
-      websocket = connection.request
+      request = connection.request
+      request.should be_websocket
+
+      websocket = request.websocket
       websocket.should be_a Reel::WebSocket
 
       handshake.errors.should be_empty
@@ -21,11 +24,9 @@
     with_socket_pair do |client, connection|
       client << handshake.to_data
 
-      websocket = connection.request
+      websocket = connection.request.websocket
       websocket.should be_a Reel::WebSocket
-      lambda {
-        connection.close
-      }.should raise_error(Reel::Connection::StateError)
+      expect { connection.close }.to raise_error(Reel::Connection::StateError)
     end
   end
 
@@ -80,18 +81,23 @@
 
       remote_host = connection.remote_host
 
-      websocket = connection.request
+      request = connection.request
+      request.should be_websocket
+      websocket = request.websocket
       websocket.should be_a Reel::WebSocket
 
-      lambda { connection.remote_host }.should raise_error(Reel::RequestError)
+      expect { connection.remote_host }.to raise_error(Reel::Connection::StateError)
       websocket.remote_host.should == remote_host
     end
   end
 
   def with_websocket_pair
     with_socket_pair do |client, connection|
       client << handshake.to_data
-      websocket = connection.request
+      request = connection.request
+
+      request.should be_websocket
+      websocket = request.websocket
       websocket.should be_a Reel::WebSocket
 
       # Discard handshake