| Date | Commit message (Collapse) |
|---|
|
If we already started writing a response, we cannot be sending a
502 to inform a user a connection has failed. This should
prevent errors from the OpenSSL layer about mismatched buffers
due to the combination of slow clients and upstreams prematurely
aborting.
|
|
Introduced in commit 08cea89483e90c79daa2c8efe70da6bdeba71147
('response: only stream "file" responses on known length')
|
|
This means we need to check Content-Length and use it
properly (unless Content-Range is set).
|
|
This is not a part of any Rack specification, and can lead to
interesting bugs with wrapped classes for gzip and TLS. AFAIK,
.to_io was meant to support IO#wait_*able and IO.select methods,
not to actually perform read/writes on the return value.
|
|
HTTPS adds some level of privacy protection and helps marketing
(because we care soooo much about good marketing! :P).
Performance-wise, this reduces subjectAltName bloat when
negotiating connections and will also speed up occasional
certificate renewals when/if we drop the old name.
Also, not occupying the document root of a domain will make it
easier to add alternative site locations in the future, because
centralization sucks and I don't like the idea of anybody paying
ICANN or similar entities for domain names.
|
|
Sometimes apps may trigger zero-byte chunks in the response
body for whatever reason. We should maintain consistent
behavior with the rest of kgio; and Ruby OpenSSL::SSL::SSLSocket
should maintain consistent behavior with the core IO class:
https://bugs.ruby-lang.org/issues/12660
|
|
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.
|
|
This makes it easier to add more parameters to
http_response_write and simplifies current callers.
|
|
Clients are not able to handle persistent connections unless
the client knows the length of the response.
|
|
Rack::Lint-compliant applications wouldn't have this problem;
but apparently public-facing Rack servers (webrick/puma/thin)
all implement this; so there is precedence for implementing
this in yahns itself.
|
|
This allows us to speed up subsequent calls to wbuf_write when
the client socket buffers are cleared. This doesn't affect
functionality outside of performance.
|
|
Since we use wbuf_lite for long, streaming requests, we need to
reset the offset and counts when aborting the existing wbuf and
not assume the wbuf goes out-of-scope and expires when we
are done using it. Fix stupid bugs in BUG notices while
we're at it :x
|
|
StringIO can never be truncated outside our control, so it is
a bug if we see EOF on trysendio, here.
|
|
All of our wbuf code assumes we append to existing buffers
(files) since sendfile cannot deal otherwise. We also
follow this pattern for StringIO to avoid extra data copies.
|
|
And explain why this is doable for StringIO and not TmpIO,
which is file-backed.
|
|
This allows us to work transparently with our OpenSSL
workaround[*] while allowing us to reuse our non-sendfile
compatibility code. Unfortunately, this means we duplicate a
lot of code from the normal wbuf code for now; but that should
be fairly stable at this point.
[*] https://bugs.ruby-lang.org/issues/12085
|
|
Another critical bugfix for this yet-to-be-released feature.
By the time we call proxy_unbuffer in proxy_read_body,
the req_res socket may be completely drained of readable data
and a persistent-connection-capable backend will be waiting
for the next request (not knowing we do not yet support
persistent connections).
This affects both chunked and identity responses from the
upstream.
|
|
The HTTP state (@hs) object could be replaced in proxy_wait_next
causing @hs.env['rack.logger'] to become inaccessible.
|
|
proxy_unbuffer is vulnerable to the same race condition
we avoided in commit 5328992829b2
("proxy_pass: fix race condition due to flawed hijack check")
|
|
When breaking early out of the upstream response body loop, we
must ensure we know we've hit the parser body EOF and preserve
the buffer for trailer parsing. Otherwise, reentering
proxy_read_body will put us in a bad state and corrupt
responses.
This is a critical bugfix which only affects users of
the soon-to-be-released "proxy_buffering: false" feature
of proxy_pass.
|
|
When we call shutdown on bad upstream responses, FD limits,
or server termination, we need to ensure the TLS connection
is terminated properly by calling SSL_shutdown and avoiding
confusion on the client side (or violating TLS specs!).
|
|
This fixes a case where "proxy_buffering: false" users may
encounter a "upstream error: BUG: EOF on tmpio sf_offset="
as a wbuf may be reused.
Oddly, it took over one week of running the latest proxy_pass
as of commit 616e42c8d609905d9355bb5db726a5348303ffae
("proxy_pass: fix HTTP/1.0 backends on EOF w/o buffering")
|
|
We must ensure we properly close connections to HTTP/1.0
backends even if we blocked writing on outgoing data.
|
|
This should make it easier to figure out where certain
errors are coming from and perhaps fix problems with
upstreams, too.
This helped me track down the problem causing public-inbox WWW
component running under Perl v5.20.2 on my Debian jessie system
to break and drop connections going through
Plack::Middleware::Deflater with gzip:
https://public-inbox.org/meta/20160607071401.29325-1-e@80x24.org/
Perl 5.14.2 on Debian wheezy did not detect this problem :x
|
|
Using a 10ms tick was too little, use 100ms instead to avoid
burning CPU. Ideally, we would not tick at all during shutdown
(we're normally tickless); but the common case could be
slightly more expensive; and shutdowns are rare (I hope).
Then, change our process title to indicate we're shutting down,
and finally, cut down on repeated log spew during shutdown and
only log dropping changes.
This mean we could potentially take 90ms longer to notice when
we can do a graceful shutdown, but oh well...
While we're at it, add a test to ensure graceful shutdowns
work as intended with multiple processes.
|
|
Using a high max_events may mean some IO objects are closed
after they're retrieved from the kernel but before our Ruby
process has had a chance to get to them.
|
|
OpenSSL can't handle write retries if we append to an existing
string. Thus we must preserve the same string object upon
retrying.
Do that by utilizing the underlying Wbuf class which could
already handles it transparently using trysendfile.
However, we still avoiding the subtlety of wbuf_close_common
reliance we previously used.
ref: commit 551e670281bea77e727a732ba94275265ccae5f6
("fix output buffering with SSL_write")
|
|
We can retrieve it when we actually need to create the
temporary file. This saves an ivar slot and method dispatch
parameters.
This patch is nice, unfortunately the patch which follows is
not :P
|
|
@busy will be reset on wbuf_write anyways, since there is no
initial data and we will always attempt to write to the socket
aggressively.
|
|
Relying on @body.close in Yahns::WbufCommon#wbuf_close_common to
resume reading the upstream response was too subtle and
potentially racy.
Instead use a new Yahns::WbufLite class which does exactly what
we want for implementing this feature, and nothing more.
|
|
We cannot rely on env being available after proxy_wait_next
|
|
"proxy_response_start" caller is ReqRes#yahns_step which
already does a global rescue. Furthermore, it clobbers
backtraces on network errors which programmers can do
nothing to fix.
|
|
This may be useful to avoid wasting resources when proxying for
an upstream which can already handle slow clients itself.
It is impossible to completely disable buffering, this merely
prevents gigantic amounts of buffering.
This may be useful when an upstream can generate a gigantic
response which would cause excessive disk I/O traffic if
buffered by yahns. An example of this would be an upstream
dynamically-generating a pack for a giant git (clone|fetch)
operation.
In other words, this option allows the upstream to react to
backpressure from slow clients. It is not recommended to
enable this unless your upstream server is capable of
supporting slow clients.
|
|
This will allow us to add extra options at the response
layer without taking up extra env hash keys.
|
|
Ugh, this is a little slower, but some people will want to
forward through multiple proxies.
|
|
This fixes a bug where a cleared wbuf would kill the
process after the response got flushed out to the client
as `wbuf.busy' becomes `false'.
This fixes a regression introduced in
"proxy_pass: trim down proxy_response_finish, too"
which was never in a stable release
|
|
This makes the ReqRes class easier-to-find and hopefully
maintain when using with other parts of yahns, although there
may be no reason to use this class outside of ProxyPass.
|
|
After the previous commits, we've added more branches
and made the existing response handling more generic;
so we can remove some duplicated logic and increase
review-ability.
|
|
Hopefully this increases readability as well and allows
us to make easier adjustments to support new features in
the future.
|
|
Again, the cost of having extra branches within a loop is
probably neglible compared to having bigger bytecode resulting
in worse CPU cache performance and increased maintenance
overhead for extra code.
|
|
proxy_response_start is gigantic an hard-to-read, we can
more clearly see the lifetimes of some objects, now, and
hopefully shorten some of them.
|
|
The cost of extra branches inside a loop is negligible compared
to the cost of all the other method calls we make. Favor
smaller code instead and inline some (now) single-use methods.
Furthermore, this allows us to reuse the request header buffer
instead of relying on thread-local storage and potentially
having to to swap buffers.
|
|
Instead, we must drop non-terminated responses since
HTTP/1.0 clients do not understand chunked encoding.
This is necessary for "ab -k" which still uses HTTP/1.0.
|
|
We need to set state as early as possible as any modification of our
HttpClient object is unsafe after it is handed over to the
underlying application. Otherwise, we could be clobbering a state
set inside the hijack but before we hit the case statement in
HttpClient#step_write.
This bug should not affect current (known) uses of rack.hijack;
but will be necessary for upcoming proxy_pass to support unbuffered
proxy responses.
|
|
We don't want to wait on GC to reap sockets on errors,
generational GC in Ruby is less aggressive about reaping
long-lived objects such as long-lived HTTP connections.
|
|
For slow clients, we want to be able to drop the connection
to the upstream as soon as we are done buffering and not waste
resources by leaving it in an :ignore state. We also need to
remember the client for the fdmap to prevent shutdowns.
Ugh, this is really hard to test locally.
|
|
We already check for the truthiness of "alive" in the "if"
statement, so re-setting is pointless.
|
|
We cannot maintain a persistent connection to a client if
writing to the client fails; so we can't proceed to let
the app hijack the response.
This may happen in the unlikely case where a response
header needs to be buffered with a Wbuf (and the app
uses response hijacking).
|
|
Without this, non-terminated backends were not properly
supported if they gave tiny responses or responded faster
than we could stream the response to the client.
This is necessary to support fast responses from some non-Rack
HTTP/1.0-only backend servers which rely on connection
termination to terminate responses.
Tested manually with a Perl PSGI application running under
"plackup". Unlike Rack, the PSGI spec does not specify whether
the PSGI application or PSGI server should handle response
termination: git clone https://github.com/plack/psgi-specs.git
Follow-up-to: 8c9f33a5396d2 ("workaround non-terminated backends")
|
|
If a static file response gets truncated while writing, we
set wbuf_persist to false in wbuf_flush of lib/yahns/wbuf_common.rb
Thus, we must check wbuf_persist attribute as late as possible
before yielding control back to the caller in proxy_response_finish
|