Skip to content

Commit

Permalink
revert signature change to HttpServer#process_client
Browse files Browse the repository at this point in the history
We can force kgio_tryaccept to return an internal class
for TCP objects by subclassing Kgio::TCPServer.

This avoids breakage in any unfortunate projects which depend on
our undocumented internal APIs, such as gctools
<https://github.com/tmm1/gctools>
  • Loading branch information
Eric Wong committed Mar 8, 2017
1 parent 77b9ec2 commit 8ce88a3
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 26 deletions.
10 changes: 5 additions & 5 deletions lib/unicorn/http_request.rb
Expand Up @@ -61,7 +61,7 @@ def self.check_client_connection=(bool)
# returns an environment hash suitable for Rack if successful
# This does minimal exception trapping and it is up to the caller
# to handle any socket errors (e.g. user aborted upload).
def read(socket, listener)
def read(socket)
clear
e = env

Expand All @@ -82,7 +82,7 @@ def read(socket, listener)
false until add_parse(socket.kgio_read!(16384))
end

check_client_connection(socket, listener) if @@check_client_connection
check_client_connection(socket) if @@check_client_connection

e['rack.input'] = 0 == content_length ?
NULL_IO : @@input_class.new(socket, self)
Expand All @@ -105,8 +105,8 @@ def hijacked?
end

if defined?(Raindrops::TCP_Info)
def check_client_connection(socket, listener) # :nodoc:
if Kgio::TCPServer === listener
def check_client_connection(socket) # :nodoc:
if Unicorn::TCPClient === socket
@@tcp_info ||= Raindrops::TCP_Info.new(socket)
@@tcp_info.get!(socket)
raise Errno::EPIPE, "client closed connection".freeze,
Expand All @@ -127,7 +127,7 @@ def closed_state?(state) # :nodoc:
end
end
else
def check_client_connection(socket, listener) # :nodoc:
def check_client_connection(socket) # :nodoc:
write_http_header(socket)
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/unicorn/http_server.rb
Expand Up @@ -558,8 +558,8 @@ def e100_response_write(client, env)

# once a client is accepted, it is processed in its entirety here
# in 3 easy steps: read request, call app, write app response
def process_client(client, listener)
status, headers, body = @app.call(env = @request.read(client, listener))
def process_client(client)
status, headers, body = @app.call(env = @request.read(client))

begin
return if @request.hijacked?
Expand Down Expand Up @@ -655,7 +655,7 @@ def worker_loop(worker)
# Unicorn::Worker#kgio_tryaccept is not like accept(2) at all,
# but that will return false
if client = sock.kgio_tryaccept
process_client(client, sock)
process_client(client)
nr += 1
worker.tick = time_now.to_i
end
Expand Down
4 changes: 2 additions & 2 deletions lib/unicorn/oob_gc.rb
Expand Up @@ -67,8 +67,8 @@ def self.new(app, interval = 5, path = %r{\A/})

#:stopdoc:
PATH_INFO = "PATH_INFO"
def process_client(client, listener)
super(client, listener) # Unicorn::HttpServer#process_client
def process_client(client)
super(client) # Unicorn::HttpServer#process_client
if OOBGC_PATH =~ OOBGC_ENV[PATH_INFO] && ((@@nr -= 1) <= 0)
@@nr = OOBGC_INTERVAL
OOBGC_ENV.clear
Expand Down
16 changes: 14 additions & 2 deletions lib/unicorn/socket_helper.rb
Expand Up @@ -3,6 +3,18 @@
require 'socket'

module Unicorn

# Instead of using a generic Kgio::Socket for everything,
# tag TCP sockets so we can use TCP_INFO under Linux without
# incurring extra syscalls for Unix domain sockets.
# TODO: remove these when we remove kgio
TCPClient = Class.new(Kgio::Socket) # :nodoc:
class TCPSrv < Kgio::TCPServer # :nodoc:
def kgio_tryaccept # :nodoc:
super(TCPClient)
end
end

module SocketHelper

# internal interface
Expand Down Expand Up @@ -148,7 +160,7 @@ def new_tcp_server(addr, port, opt)
end
sock.bind(Socket.pack_sockaddr_in(port, addr))
sock.autoclose = false
Kgio::TCPServer.for_fd(sock.fileno)
TCPSrv.for_fd(sock.fileno)
end

# returns rfc2732-style (e.g. "[::1]:666") addresses for IPv6
Expand Down Expand Up @@ -185,7 +197,7 @@ def sock_name(sock)
def server_cast(sock)
begin
Socket.unpack_sockaddr_in(sock.getsockname)
Kgio::TCPServer.for_fd(sock.fileno)
TCPSrv.for_fd(sock.fileno)
rescue ArgumentError
Kgio::UNIXServer.for_fd(sock.fileno)
end
Expand Down
28 changes: 14 additions & 14 deletions test/unit/test_request.rb
Expand Up @@ -30,7 +30,7 @@ def setup
def test_options
client = MockRequest.new("OPTIONS * HTTP/1.1\r\n" \
"Host: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal '', env['REQUEST_PATH']
assert_equal '', env['PATH_INFO']
assert_equal '*', env['REQUEST_URI']
Expand All @@ -40,7 +40,7 @@ def test_options
def test_absolute_uri_with_query
client = MockRequest.new("GET http://e:3/x?y=z HTTP/1.1\r\n" \
"Host: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal '/x', env['REQUEST_PATH']
assert_equal '/x', env['PATH_INFO']
assert_equal 'y=z', env['QUERY_STRING']
Expand All @@ -50,7 +50,7 @@ def test_absolute_uri_with_query
def test_absolute_uri_with_fragment
client = MockRequest.new("GET http://e:3/x#frag HTTP/1.1\r\n" \
"Host: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal '/x', env['REQUEST_PATH']
assert_equal '/x', env['PATH_INFO']
assert_equal '', env['QUERY_STRING']
Expand All @@ -61,7 +61,7 @@ def test_absolute_uri_with_fragment
def test_absolute_uri_with_query_and_fragment
client = MockRequest.new("GET http://e:3/x?a=b#frag HTTP/1.1\r\n" \
"Host: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal '/x', env['REQUEST_PATH']
assert_equal '/x', env['PATH_INFO']
assert_equal 'a=b', env['QUERY_STRING']
Expand All @@ -73,15 +73,15 @@ def test_absolute_uri_unsupported_schemes
%w(ssh+http://e/ ftp://e/x http+ssh://e/x).each do |abs_uri|
client = MockRequest.new("GET #{abs_uri} HTTP/1.1\r\n" \
"Host: foo\r\n\r\n")
assert_raises(HttpParserError) { @request.read(client, nil) }
assert_raises(HttpParserError) { @request.read(client) }
end
end

def test_x_forwarded_proto_https
client = MockRequest.new("GET / HTTP/1.1\r\n" \
"X-Forwarded-Proto: https\r\n" \
"Host: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal "https", env['rack.url_scheme']
res = @lint.call(env)
end
Expand All @@ -90,7 +90,7 @@ def test_x_forwarded_proto_http
client = MockRequest.new("GET / HTTP/1.1\r\n" \
"X-Forwarded-Proto: http\r\n" \
"Host: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal "http", env['rack.url_scheme']
res = @lint.call(env)
end
Expand All @@ -99,38 +99,38 @@ def test_x_forwarded_proto_invalid
client = MockRequest.new("GET / HTTP/1.1\r\n" \
"X-Forwarded-Proto: ftp\r\n" \
"Host: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal "http", env['rack.url_scheme']
res = @lint.call(env)
end

def test_rack_lint_get
client = MockRequest.new("GET / HTTP/1.1\r\nHost: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal "http", env['rack.url_scheme']
assert_equal '127.0.0.1', env['REMOTE_ADDR']
res = @lint.call(env)
end

def test_no_content_stringio
client = MockRequest.new("GET / HTTP/1.1\r\nHost: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal StringIO, env['rack.input'].class
end

def test_zero_content_stringio
client = MockRequest.new("PUT / HTTP/1.1\r\n" \
"Content-Length: 0\r\n" \
"Host: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal StringIO, env['rack.input'].class
end

def test_real_content_not_stringio
client = MockRequest.new("PUT / HTTP/1.1\r\n" \
"Content-Length: 1\r\n" \
"Host: foo\r\n\r\n")
env = @request.read(client, nil)
env = @request.read(client)
assert_equal Unicorn::TeeInput, env['rack.input'].class
end

Expand All @@ -141,7 +141,7 @@ def test_rack_lint_put
"Content-Length: 5\r\n" \
"\r\n" \
"abcde")
env = @request.read(client, nil)
env = @request.read(client)
assert ! env.include?(:http_body)
res = @lint.call(env)
end
Expand All @@ -167,7 +167,7 @@ def client.kgio_read!(*args)
"\r\n")
count.times { assert_equal bs, client.syswrite(buf) }
assert_equal 0, client.sysseek(0)
env = @request.read(client, nil)
env = @request.read(client)
assert ! env.include?(:http_body)
assert_equal length, env['rack.input'].size
count.times {
Expand Down

0 comments on commit 8ce88a3

Please sign in to comment.