* [PATCH 0/4] remove chunked/Content-Length requirement from apps
@ 2016-08-03 3:19 Eric Wong
2016-08-03 3:19 ` [PATCH 1/4] response: drop clients after HTTP responses of unknown length Eric Wong
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Eric Wong @ 2016-08-03 3:19 UTC (permalink / raw)
To: yahns-public
This better aligns with behavior of other real-world Rack
servers such as puma and Thin. We will also now use kgio writev
functionality to speed up writing "Transfer-Encoding: chunked"
to avoid string copies or extra syscalls for other chunking
solutions.
See [PATCH 3/4] for a small informal benchmark.
4 changes
response: drop clients after HTTP responses of unknown length
response: reduce stack overhead for parameter passing
response: support auto-chunking for HTTP/1.1
Revert "document Rack::Chunked/ContentLength semi-requirements"
Documentation/yahns-rackup.pod | 10 -------
examples/yahns_rack_basic.conf.rb | 6 -----
lib/yahns/chunk_body.rb | 27 +++++++++++++++++++
lib/yahns/http_client.rb | 24 ++++++++---------
lib/yahns/http_response.rb | 47 +++++++++++++++++++++++---------
test/test_auto_chunk.rb | 56 +++++++++++++++++++++++++++++++++++++++
test/test_extras_exec_cgi.rb | 4 +--
7 files changed, 130 insertions(+), 44 deletions(-)
create mode 100644 lib/yahns/chunk_body.rb
create mode 100644 test/test_auto_chunk.rb
--
EW
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/4] response: drop clients after HTTP responses of unknown length
2016-08-03 3:19 [PATCH 0/4] remove chunked/Content-Length requirement from apps Eric Wong
@ 2016-08-03 3:19 ` Eric Wong
2016-08-03 3:19 ` [PATCH 2/4] response: reduce stack overhead for parameter passing Eric Wong
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-08-03 3:19 UTC (permalink / raw)
To: yahns-public
Clients are not able to handle persistent connections unless
the client knows the length of the response.
---
lib/yahns/http_response.rb | 6 ++++++
| 4 +---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/lib/yahns/http_response.rb b/lib/yahns/http_response.rb
index 88ff9f9..f2d9c62 100644
--- a/lib/yahns/http_response.rb
+++ b/lib/yahns/http_response.rb
@@ -126,6 +126,7 @@ def http_response_write(status, headers, body, hdr_only)
k = self.class
alive = @hs.next? && k.persistent_connections
flags = MSG_DONTWAIT
+ term = false
if @hs.headers?
code = status.to_i
@@ -147,14 +148,19 @@ def http_response_write(status, headers, body, hdr_only)
# allow Rack apps to tell us they want to drop the client
alive = false if value =~ /\bclose\b/i
when %r{\AContent-Length\z}i
+ term = true
flags |= MSG_MORE if value.to_i > 0 && !hdr_only
kv_str(buf, key, value)
+ when %r{\ATransfer-Encoding\z}i
+ term = true if value =~ /\bchunked\b/i
+ kv_str(buf, key, value)
when "rack.hijack"
hijack = value
else
kv_str(buf, key, value)
end
end
+ alive &&= term
buf << (alive ? "Connection: keep-alive\r\n\r\n".freeze
: "Connection: close\r\n\r\n".freeze)
case rv = kgio_syssend(buf, flags)
--git a/test/test_extras_exec_cgi.rb b/test/test_extras_exec_cgi.rb
index f9b96d9..3c71fd4 100644
--- a/test/test_extras_exec_cgi.rb
+++ b/test/test_extras_exec_cgi.rb
@@ -169,10 +169,8 @@ def _blocked_zombie(block_on, rtype)
assert_match %r{200 OK}, head
assert_match %r{\A\d+\n\z}, body
exec_pid = body.to_i
- assert_raises(Errno::EAGAIN, IO::WaitReadable) { c.read_nonblock(666) }
poke_until_dead exec_pid
- # still alive?
- assert_raises(Errno::EAGAIN, IO::WaitReadable) { c.read_nonblock(666) }
+ assert_raises(EOFError) { c.read_nonblock(666) }
else
raise "BUG in test, bad rtype"
end
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/4] response: reduce stack overhead for parameter passing
2016-08-03 3:19 [PATCH 0/4] remove chunked/Content-Length requirement from apps Eric Wong
2016-08-03 3:19 ` [PATCH 1/4] response: drop clients after HTTP responses of unknown length Eric Wong
@ 2016-08-03 3:19 ` Eric Wong
2016-08-03 3:19 ` [PATCH 3/4] response: support auto-chunking for HTTP/1.1 Eric Wong
2016-08-03 3:19 ` [PATCH 4/4] Revert "document Rack::Chunked/ContentLength semi-requirements" Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-08-03 3:19 UTC (permalink / raw)
To: yahns-public
This makes it easier to add more parameters to
http_response_write and simplifies current callers.
---
lib/yahns/http_client.rb | 20 ++++++++++----------
lib/yahns/http_response.rb | 7 ++++---
2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/lib/yahns/http_client.rb b/lib/yahns/http_client.rb
index 873dd73..7a1bac1 100644
--- a/lib/yahns/http_client.rb
+++ b/lib/yahns/http_client.rb
@@ -191,9 +191,9 @@ def r100_done
else # :lazy, false
env = @hs.env
hdr_only = env['REQUEST_METHOD'] == 'HEAD'.freeze
- status, headers, body = k.app.call(env)
- return :ignore if app_hijacked?(env, body)
- http_response_write(status, headers, body, hdr_only)
+ res = k.app.call(env)
+ return :ignore if app_hijacked?(env, res)
+ http_response_write(res, hdr_only)
end
end
@@ -224,15 +224,15 @@ def app_call(input)
hdr_only = env['REQUEST_METHOD'] == 'HEAD'.freeze
# run the rack app
- status, headers, body = k.app.call(env)
- return :ignore if app_hijacked?(env, body)
- if status.to_i == 100
+ res = k.app.call(env)
+ return :ignore if app_hijacked?(env, res)
+ if res[0].to_i == 100
rv = http_100_response(env) and return rv
- status, headers, body = k.app.call(env)
+ res = k.app.call(env)
end
# this returns :wait_readable, :wait_writable, :ignore, or nil:
- http_response_write(status, headers, body, hdr_only)
+ http_response_write(res, hdr_only)
end
# called automatically by kgio_write
@@ -308,9 +308,9 @@ def handle_error(e)
return # always drop the connection on uncaught errors
end
- def app_hijacked?(env, body)
+ def app_hijacked?(env, res)
return false unless env.include?('rack.hijack_io'.freeze)
- body.close if body.respond_to?(:close)
+ res[2].close if res && res[2].respond_to?(:close)
true
end
diff --git a/lib/yahns/http_response.rb b/lib/yahns/http_response.rb
index f2d9c62..b157ee4 100644
--- a/lib/yahns/http_response.rb
+++ b/lib/yahns/http_response.rb
@@ -8,8 +8,8 @@
# Writes a Rack response to your client using the HTTP/1.1 specification.
# You use it by simply doing:
#
-# status, headers, body = rack_app.call(env)
-# http_response_write(status, headers, body, env['REQUEST_METHOD']=='HEAD')
+# res = rack_app.call(env)
+# http_response_write(res, env['REQUEST_METHOD']=='HEAD')
#
# Most header correctness (including Content-Length and Content-Type)
# is the job of Rack, with the exception of the "Date" header.
@@ -120,7 +120,8 @@ def kv_str(buf, key, value)
# writes the rack_response to socket as an HTTP response
# returns :wait_readable, :wait_writable, :forget, or nil
- def http_response_write(status, headers, body, hdr_only)
+ def http_response_write(res, hdr_only)
+ status, headers, body = res
offset = 0
count = hijack = nil
k = self.class
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/4] response: support auto-chunking for HTTP/1.1
2016-08-03 3:19 [PATCH 0/4] remove chunked/Content-Length requirement from apps Eric Wong
2016-08-03 3:19 ` [PATCH 1/4] response: drop clients after HTTP responses of unknown length Eric Wong
2016-08-03 3:19 ` [PATCH 2/4] response: reduce stack overhead for parameter passing Eric Wong
@ 2016-08-03 3:19 ` Eric Wong
2016-08-03 3:19 ` [PATCH 4/4] Revert "document Rack::Chunked/ContentLength semi-requirements" Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-08-03 3:19 UTC (permalink / raw)
To: yahns-public
We might as well do it since puma and thin both do(*),
and we can still do writev for now to get some speedups
by avoiding Rack::Chunked overhead.
timing runs of "curl --no-buffer http://127.0.0.1:9292/ >/dev/null"
results in a best case drop from ~260ms to ~205ms on one VM
by disabling Rack::Chunked in the below config.ru
$ ruby -I lib bin/yahns-rackup -E none config.ru
==> config.ru <==
class Body
STR = ' ' * 1024 * 16
def each
10000.times { yield STR }
end
end
use Rack::Chunked if ENV['RACK_CHUNKED']
run(lambda do |env|
[ 200, [ %w(Content-Type text/plain) ], Body.new ]
end)
(*) they can do Content-Length, but I don't think it's
worth the effort at the server level.
---
lib/yahns/chunk_body.rb | 27 ++++++++++++++++++++++
lib/yahns/http_client.rb | 8 +++----
lib/yahns/http_response.rb | 38 +++++++++++++++++++++----------
test/test_auto_chunk.rb | 56 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 113 insertions(+), 16 deletions(-)
create mode 100644 lib/yahns/chunk_body.rb
create mode 100644 test/test_auto_chunk.rb
diff --git a/lib/yahns/chunk_body.rb b/lib/yahns/chunk_body.rb
new file mode 100644
index 0000000..6e56a18
--- /dev/null
+++ b/lib/yahns/chunk_body.rb
@@ -0,0 +1,27 @@
+# -*- encoding: binary -*-
+# Copyright (C) 2016 all contributors <yahns-public@yhbt.net>
+# License: GPL-3.0+ <https://www.gnu.org/licenses/gpl-3.0.txt>
+# frozen_string_literal: true
+class Yahns::ChunkBody
+ def initialize(body, vec)
+ @body = body
+ @vec = vec
+ end
+
+ def each
+ vec = @vec
+ vec[2] = "\r\n".freeze
+ @body.each do |chunk|
+ vec[0] = "#{chunk.bytesize.to_s(16)}\r\n"
+ vec[1] = chunk
+ # vec[2] never changes: "\r\n" above
+ yield vec
+ end
+ vec.clear
+ yield "0\r\n\r\n".freeze
+ end
+
+ def close
+ @body.close if @body.respond_to?(:close)
+ end
+end
diff --git a/lib/yahns/http_client.rb b/lib/yahns/http_client.rb
index 7a1bac1..d8154a4 100644
--- a/lib/yahns/http_client.rb
+++ b/lib/yahns/http_client.rb
@@ -190,10 +190,10 @@ def r100_done
true
else # :lazy, false
env = @hs.env
- hdr_only = env['REQUEST_METHOD'] == 'HEAD'.freeze
+ opt = http_response_prep(env)
res = k.app.call(env)
return :ignore if app_hijacked?(env, res)
- http_response_write(res, hdr_only)
+ http_response_write(res, opt)
end
end
@@ -222,7 +222,7 @@ def app_call(input)
env['SERVER_PORT'] = '443'.freeze
end
- hdr_only = env['REQUEST_METHOD'] == 'HEAD'.freeze
+ opt = http_response_prep(env)
# run the rack app
res = k.app.call(env)
return :ignore if app_hijacked?(env, res)
@@ -232,7 +232,7 @@ def app_call(input)
end
# this returns :wait_readable, :wait_writable, :ignore, or nil:
- http_response_write(res, hdr_only)
+ http_response_write(res, opt)
end
# called automatically by kgio_write
diff --git a/lib/yahns/http_response.rb b/lib/yahns/http_response.rb
index b157ee4..32d1a45 100644
--- a/lib/yahns/http_response.rb
+++ b/lib/yahns/http_response.rb
@@ -4,12 +4,14 @@
# frozen_string_literal: true
require_relative 'stream_file'
require_relative 'wbuf_str'
+require_relative 'chunk_body'
# Writes a Rack response to your client using the HTTP/1.1 specification.
# You use it by simply doing:
#
+# opt = http_response_prep(env)
# res = rack_app.call(env)
-# http_response_write(res, env['REQUEST_METHOD']=='HEAD')
+# http_response_write(res, opt)
#
# Most header correctness (including Content-Length and Content-Type)
# is the job of Rack, with the exception of the "Date" header.
@@ -120,14 +122,14 @@ def kv_str(buf, key, value)
# writes the rack_response to socket as an HTTP response
# returns :wait_readable, :wait_writable, :forget, or nil
- def http_response_write(res, hdr_only)
+ def http_response_write(res, opt)
status, headers, body = res
offset = 0
count = hijack = nil
- k = self.class
- alive = @hs.next? && k.persistent_connections
+ alive = @hs.next? && self.class.persistent_connections
flags = MSG_DONTWAIT
term = false
+ hdr_only, chunk_ok = opt
if @hs.headers?
code = status.to_i
@@ -161,6 +163,11 @@ def http_response_write(res, hdr_only)
kv_str(buf, key, value)
end
end
+ if !term && chunk_ok
+ term = true
+ body = Yahns::ChunkBody.new(body, opt)
+ buf << "Transfer-Encoding: chunked\r\n".freeze
+ end
alive &&= term
buf << (alive ? "Connection: keep-alive\r\n\r\n".freeze
: "Connection: close\r\n\r\n".freeze)
@@ -173,7 +180,7 @@ def http_response_write(res, hdr_only)
flags = MSG_DONTWAIT
buf = rv # unlikely, hope the skb grows
when :wait_writable, :wait_readable # unlikely
- if k.output_buffering
+ if self.class.output_buffering
alive = hijack ? hijack : alive
rv = response_header_blocked(buf, body, alive, offset, count)
body = nil # ensure we do not close body in ensure
@@ -193,19 +200,19 @@ def http_response_write(res, hdr_only)
end
wbuf = rv = nil
- body.each do |chunk|
+ body.each do |x|
if wbuf
- rv = wbuf.wbuf_write(self, chunk)
+ rv = wbuf.wbuf_write(self, x)
else
- case rv = kgio_trywrite(chunk)
+ case rv = String === x ? kgio_trywrite(x) : kgio_trywritev(x)
when nil # all done, likely and good!
break
- when String
- chunk = rv # hope the skb grows when we loop into the trywrite
+ when String, Array
+ x = rv # hope the skb grows when we loop into the trywrite
when :wait_writable, :wait_readable
- if k.output_buffering
+ if self.class.output_buffering
wbuf = Yahns::Wbuf.new(body, alive)
- rv = wbuf.wbuf_write(self, chunk)
+ rv = wbuf.wbuf_write(self, x)
break
else
response_wait_write(rv) or return :close
@@ -278,4 +285,11 @@ def http_100_response(env)
return rv
end while true
end
+
+ # must be called before app dispatch, since the app can
+ # do all sorts of nasty things to env
+ def http_response_prep(env)
+ [ env['REQUEST_METHOD'] == 'HEAD'.freeze, # hdr_only
+ env['HTTP_VERSION'] == 'HTTP/1.1'.freeze ] # chunk_ok
+ end
end
diff --git a/test/test_auto_chunk.rb b/test/test_auto_chunk.rb
new file mode 100644
index 0000000..a97fe26
--- /dev/null
+++ b/test/test_auto_chunk.rb
@@ -0,0 +1,56 @@
+# Copyright (C) 2013-2016 all contributors <yahns-public@yhbt.net>
+# License: GPL-3.0+ <https://www.gnu.org/licenses/gpl-3.0.txt>
+# frozen_string_literal: true
+require_relative 'server_helper'
+
+class TestAutoChunk < Testcase
+ ENV["N"].to_i > 1 and parallelize_me!
+ include ServerHelper
+ alias setup server_helper_setup
+ alias teardown server_helper_teardown
+
+ def test_auto_head
+ err = @err
+ cfg = Yahns::Config.new
+ host, port = @srv.addr[3], @srv.addr[1]
+ cfg.instance_eval do
+ GTL.synchronize do
+ app = Rack::Builder.new do
+ use Rack::ContentType, "text/plain"
+ run(lambda do |env|
+ [ 200, {}, %w(a b c) ]
+ end)
+ end
+ app(:rack, app) { listen "#{host}:#{port}" }
+ end
+ logger(Logger.new(err.path))
+ end
+ pid = mkserver(cfg)
+ s = TCPSocket.new(host, port)
+ s.write("GET / HTTP/1.0\r\n\r\n")
+ assert s.wait(30), "IO wait failed"
+ buf = s.read
+ assert_match %r{\r\n\r\nabc\z}, buf
+ s.close
+
+ s = TCPSocket.new(host, port)
+ s.write("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")
+ buf = ''.dup
+ Timeout.timeout(30) do
+ until buf =~ /\r\n\r\n1\r\na\r\n1\r\nb\r\n1\r\nc\r\n0\r\n\r\n\z/
+ buf << s.readpartial(16384)
+ end
+ end
+ assert_match(%r{^Transfer-Encoding: chunked\r\n}, buf)
+ s.close
+
+ Net::HTTP.start(host, port) do |http|
+ req = Net::HTTP::Get.new("/")
+ res = http.request(req)
+ assert_equal 200, res.code.to_i
+ assert_equal 'abc', res.body
+ end
+ ensure
+ quit_wait(pid)
+ end
+end
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] Revert "document Rack::Chunked/ContentLength semi-requirements"
2016-08-03 3:19 [PATCH 0/4] remove chunked/Content-Length requirement from apps Eric Wong
` (2 preceding siblings ...)
2016-08-03 3:19 ` [PATCH 3/4] response: support auto-chunking for HTTP/1.1 Eric Wong
@ 2016-08-03 3:19 ` Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2016-08-03 3:19 UTC (permalink / raw)
To: yahns-public
Actually, I guess I misread, rack (starting at) 1.0 stopped
requiring Content-Length/Chunked headers but I never noticed.
Oh well.
This reverts commit 4968041a7e1ff90b920704f50fccb9e7968d0d99.
---
Documentation/yahns-rackup.pod | 10 ----------
examples/yahns_rack_basic.conf.rb | 6 ------
2 files changed, 16 deletions(-)
diff --git a/Documentation/yahns-rackup.pod b/Documentation/yahns-rackup.pod
index 6172661..efdfb6d 100644
--- a/Documentation/yahns-rackup.pod
+++ b/Documentation/yahns-rackup.pod
@@ -159,16 +159,6 @@ The RACK_ENV variable is set by the aforementioned -E switch.
If RACK_ENV is already set, it will be used unless -E is used.
See rackup documentation for more details.
-=head1 CAVEATS
-
-yahns is strict about buggy, non-compliant Rack applications.
-Some existing servers work fine without "Content-Length" or
-"Transfer-Encoding: chunked" response headers enforced by Rack::Lint.
-Forgetting these headers with yahns causes clients to stall as they
-assume more data is coming. Loading the Rack::ContentLength and/or
-Rack::Chunked middlewares will set the necessary response headers
-and fix your app.
-
=head1 CONTACT
All feedback welcome via plain-text mail to L<mailto:yahns-public@yhbt.net>
diff --git a/examples/yahns_rack_basic.conf.rb b/examples/yahns_rack_basic.conf.rb
index 610a482..f3f8e6a 100644
--- a/examples/yahns_rack_basic.conf.rb
+++ b/examples/yahns_rack_basic.conf.rb
@@ -30,12 +30,6 @@
worker_threads 50
end
-# note: Rack requires responses set "Content-Length" or use
-# "Transfer-Encoding: chunked". Some Rack servers tolerate
-# the lack of these, yahns does not. Thus you should load
-# Rack::Chunked and/or Rack::ContentLength middleware in your
-# config.ru to ensure clients know when your application
-# responses terminate.
app(:rack, "config.ru", preload: false) do
listen 80
--
EW
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-08-03 3:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-03 3:19 [PATCH 0/4] remove chunked/Content-Length requirement from apps Eric Wong
2016-08-03 3:19 ` [PATCH 1/4] response: drop clients after HTTP responses of unknown length Eric Wong
2016-08-03 3:19 ` [PATCH 2/4] response: reduce stack overhead for parameter passing Eric Wong
2016-08-03 3:19 ` [PATCH 3/4] response: support auto-chunking for HTTP/1.1 Eric Wong
2016-08-03 3:19 ` [PATCH 4/4] Revert "document Rack::Chunked/ContentLength semi-requirements" Eric Wong
Code repositories for project(s) associated with this public inbox
http://yhbt.net/yahns.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).