Previously, nginx closed client connection in cases when a response body
from upstream was needed to be cached or stored but shouldn't be sent to
the client. While this is normal for HTTP, it is unacceptable for SPDY.
Fix is to use instead the p->downstream_error flag to prevent nginx from
sending anything downstream. To make this work, the event pipe code was
modified to properly cache empty responses with the flag set.
The ngx_http_upstream_dummy_handler() must be set regardless of
the read event state. This prevents possible additional call of
ngx_http_upstream_send_request_handler().
Previously, last_modified_time was tested against -1 to check if the
not modified filter should be skipped. Notably, this prevented nginx
from additional If-Modified-Since (et al.) checks on proxied responses.
Such behaviour is suboptimal in some cases though, as checks are always
skipped on responses from a cache with ETag only (without Last-Modified),
resulting in If-None-Match being ignored in such cases. Additionally,
it was not possible to return 412 from the If-Unmodified-Since if last
modification time was not known for some reason.
This change introduces explicit r->disable_not_modified flag instead,
which is set by ngx_http_upstream_process_headers().
Previous code in ngx_http_upstream_send_response() used last modified time
from r->headers_out.last_modified_time after the header filter chain was
already called. At this point, last_modified_time may be already cleared,
e.g., with SSI, resulting in incorrect last modified time stored in a
cache file. Fix is to introduce u->headers_in.last_modified_time instead.
Clearing of the r->headers_out.last_modified_time field if a response
isn't cacheable in ngx_http_upstream_send_response() was introduced
in 3b6afa999c2f, the commit to enable not modified filter for cacheable
responses. It doesn't make sense though, as at this point header was
already sent, and not modified filter was already executed. Therefore,
the line was removed to simplify code.
The 7022564a9e0e changeset made ineffective workaround from 2464ccebdb52
to avoid NULL pointer dereference with "if". It is now restored by
moving the u->ssl_name initialization after the check.
Found by Coverity (CID 1210408).
These directives allow to switch on Server Name Indication (SNI) while
connecting to upstream servers.
By default, proxy_ssl_server_name is currently off (that is, no SNI) and
proxy_ssl_name is set to a host used in the proxy_pass directive.
If set, it means that response body is going to be in more than one buffer,
hence only range requests with a single range should be honored.
The flag is now used by mp4 and cacheable upstream responses, thus allowing
range requests of mp4 files with start/end, as well as range processing
on a first request to a not-yet-cached files with proxy_cache.
Notably this makes it possible to play mp4 files (with proxy_cache, or with
mp4 module) on iOS devices, as byte-range support is required by Apple.
If a request is finalized in the first call to the
ngx_http_upstream_process_upgraded() function, e.g., because upstream
server closed the connection for some reason, in the second call
the u->peer.connection pointer will be null, resulting in segmentation
fault.
Fix is to avoid second direct call, and post event instead. This ensures
that ngx_http_upstream_process_upgraded() won't be called again if
a request is finalized.
If "proxy_pass" is specified with variables, the resulting
hostname is looked up in the list of upstreams defined in
configuration. The search was case-sensitive, as opposed
to the case of "proxy_pass" specified without variables.
Read event on a client connection might have been disabled during
previous processing, and we at least need to handle events. Calling
ngx_http_upstream_process_upgraded() is a simpliest way to do it.
Notably this change is needed for select, poll and /dev/poll event
methods.
Previous version of this patch was posted here:
http://mailman.nginx.org/pipermail/nginx/2014-January/041839.html
Not really a strict check (as X-Accel-Expires might be ignored or
contain invalid value), but quite simple to implement and better
than what we have now.
The following new directives are introduced: proxy_cache_revalidate,
fastcgi_cache_revalidate, scgi_cache_revalidate, uwsgi_cache_revalidate.
Default is off. When set to on, they enable cache revalidation using
conditional requests with If-Modified-Since for expired cache items.
As of now, no attempts are made to merge headers given in a 304 response
during cache revalidation with headers previously stored in a cache item.
Headers in a 304 response are only used to calculate new validity time
of a cache item.
With previous code only part of u->buffer might be emptied in case
of special responses, resulting in partial responses seen by SSI set
in case of simple protocols, or spurious errors like "upstream sent
invalid chunked response" in case of complex ones.
Without u->header_sent set a special response might be generated following
an upgraded connection. The problem appeared in 1ccdda1f37f3 (1.5.3).
Catched by "header already sent" alerts in 1.5.4 after upstream timeouts.
Missing call to ngx_http_run_posted_request() resulted in a main request hang
if subrequest's ssl handshake with an upstream server failed for some reason.
Reported by Aviram Cohen.
Previously, after sending a header we always sent a last buffer and
finalized a request with code 0, even in case of errors. In some cases
this resulted in a loss of ability to detect the response wasn't complete
(e.g. if Content-Length was removed from a response by gzip filter).
This change tries to propogate to a client information that a response
isn't complete in such cases. In particular, with this change we no longer
pretend a returned response is complete if we wasn't able to create
a temporary file.
If an error code suggests the error wasn't fatal, we flush buffered data
and disable keepalive, then finalize request normally. This allows to to
propogate information about a problem to a client, while still sending all
the data we've got from an upstream.
No semantic changes expected, though some checks are done differently.
In particular, the r->cached flag is no longer explicitly checked. Instead,
we relay on u->header_sent not being set if a response is sent from
a cache.
The NGX_HTTP_CLIENT_CLOSED_REQUEST code is allowed to happen after we
started sending a response (much like NGX_HTTP_REQUEST_TIME_OUT), so there
is no need to reset response code to 0 in this case.
Checks were added to both buffered and unbuffered code paths to detect
and complain if a response is incomplete. Appropriate error codes are
now passed to ngx_http_upstream_finalize_request().
With this change in unbuffered mode we now use u->length set to -1 as an
indicator that EOF is allowed per protocol and used to indicate response
end (much like its with p->length in buffered mode). Proxy module was
changed to set u->length to 1 (instead of previously used -1) in case of
chunked transfer encoding used to comply with the above.
That is, by default we assume that response end is signalled by
a connection close. This seems to be better default, and in line
with u->pipe->length behaviour.
Memcached module was modified accordingly.
In case of upstream eof, only responses with u->pipe->length == -1
are now cached/stored. This ensures that unfinished chunked responses
are not cached.
Note well - previously used checks for u->headers_in.content_length_n are
preserved. This provides an additional level of protection if protol data
disagree with Content-Length header provided (e.g., a FastCGI response
is sent with wrong Content-Length, or an incomple SCGI or uwsgi response),
as well as protects from storing of responses to HEAD requests. This should
be reconsidered if we'll consider caching of responses to HEAD requests.
There is no real difference from previously used 0 as NGX_HTTP_* will
become 0 in ngx_http_upstream_finalize_request(), but the change
preserves information about a timeout a bit longer. Previous use of
ETIMEDOUT in one place was just wrong.
Note well that with cacheable responses there will be a difference
(code in ngx_http_upstream_finalize_request() will store the error
in cache), though this change doesn't touch cacheable case.
Previously, ngx_http_upstream_finalize_request(0) was used in most
cases after errors. While with current code there is no difference,
use of NGX_ERROR allows to pass a bit more information into
ngx_http_upstream_finalize_request().
In all cases ngx_http_upstream_finalize_request() with NGX_ERROR now used.
Previously used NGX_HTTP_INTERNAL_SERVER_ERROR in the subrequest in memory
case don't cause any harm, but inconsistent with other uses.
The parameter is mostly identical to http_404, and is expected to
be used in similar situations. The 403 code might be returned by
a backend instead of 404 on initial sync of new directories with rsync.
See here for feature request and additional details:
http://mailman.nginx.org/pipermail/nginx-ru/2013-April/050920.html
If proxy_pass to a host with dynamic resolution was used to handle
a subrequest, and host resolution failed, the main request wasn't run
till something else happened on the connection. E.g. request to "/zzz"
with the following configuration hanged:
addition_types *;
resolver 8.8.8.8;
location /test {
set $ihost xxx;
proxy_pass http://$ihost;
}
location /zzz {
add_after_body /test;
return 200 "test";
}
Report and original version of the patch by Lanshun Zhou,
http://mailman.nginx.org/pipermail/nginx-devel/2013-March/003476.html.
The c->single_connection was intended to be used as lock mechanism
to serialize modifications of request object from several threads
working with client and upstream connections. The flag is redundant
since threads in nginx have never been used that way.
This allows to proxy WebSockets by using configuration like this:
location /chat/ {
proxy_pass http://backend;
proxy_http_version 1.1;
proxy_set_header Upgrade $http_upgrade;
proxy_set_header Connection "upgrade";
}
Connection upgrade is allowed as long as it was requested by a client
via the Upgrade request header.
The "proxy_bind", "fastcgi_bind", "uwsgi_bind", "scgi_bind" and
"memcached_bind" directives are now inherited; inherited value
can be reset by the "off" parameter. Duplicate directives are
now detected. Parameter value can now contain variables.
Upstreams created by "proxy_pass" with IP address and no port were
broken in 1.3.10, by not initializing port in u->sockaddr.
API change: ngx_parse_url() was modified to always initialize port
(in u->sockaddr and in u->port), even for the u->no_resolve case;
ngx_http_upstream() and ngx_http_upstream_add() were adopted.
Configuration like
location / {
set $true 1;
if ($true) {
proxy_pass http://backend;
}
if ($true) {
# nothing
}
}
resulted in segmentation fault due to NULL pointer dereference as the
upstream configuration wasn't initialized in an implicit location created
by the last if(), but the r->content_handler was set due to first if().
Instead of committing a suicide by dereferencing a NULL pointer, return
500 (Internal Server Error) in such cases, i.e. if uscf is NULL. Better
fix would be to avoid such cases by fixing the "if" directive handling,
but it's out of scope of this patch.
Prodded by Piotr Sikora.
Pending EOF might be reported on both read and write events, whichever
comes first, so check both of them.
Patch by Yichun Zhang (agentzh), slightly modified.
Input filter might free a buffer if there is no data in it, and in case
of first buffer (used for cache header and request header, aka p->buf_to_file)
this resulted in cache corruption. Buffer memory was reused to read upstream
response before headers were written to disk.
Fix is to avoid moving pointers in ngx_event_pipe_add_free_buf() to a buffer
start if we were asked to free a buffer used by p->buf_to_file.
This fixes occasional cache file corruption, usually resulted
in "cache file ... has md5 collision" alerts.
Reported by Anatoli Marinov.
Hide headers and pass headers arrays might not be inherited correctly
into a nested location, e.g. in configuration like
server {
proxy_hide_header X-Foo;
location / {
location /nested/ {
proxy_pass_header X-Pad;
}
}
}
the X-Foo header wasn't hidden in the location /nested/.
Reported by Konstantin Svist,
http://mailman.nginx.org/pipermail/nginx-ru/2012-July/047555.html
If the "proxy_cookie_domain" or "proxy_cookie_path" directive is used and there
are no matches in Set-Cookie header then ngx_http_proxy_rewrite_cookie() returns
NGX_DECLINED to indicate that the header was not rewritten. Returning this value
further from the upstream headers copy handler resulted in 500 error response.
See here for report:
http://mailman.nginx.org/pipermail/nginx/2012-May/033858.html
Nuke NGX_PARSE_LARGE_TIME, it's not used since 0.6.30. The only error
ngx_parse_time() can currently return is NGX_ERROR, check it explicitly
and make sure to cast it to appropriate type (either time_t or ngx_msec_t)
to avoid signedness warnings on platforms with unsigned time_t (notably QNX).
Temporary files might not be removed if the "proxy_store" or "fastcgi_store"
directives were used for subrequests (e.g. ssi includes) and client closed
connection prematurely.
Non-active subrequests are finalized out of the control of the upstream
module when client closes a connection. As a result, the code to remove
unfinished temporary files in ngx_http_upstream_process_request() wasn't
executed.
Fix is to move relevant code into ngx_http_upstream_finalize_request() which
is called in all cases, either directly or via the cleanup handler.
If header filter postponed processing of a header by returning NGX_AGAIN
and not moved u->buffer->pos, previous check incorrectly assumed there
is additional space and did another recv() with zero-size buffer. This
resulted in "upstream prematurely closed connection" error instead
of correct "upstream sent too big header" one.
Patch by Feibo Li.
New directives: proxy_cache_lock on/off, proxy_cache_lock_timeout. With
proxy_cache_lock set to on, only one request will be allowed to go to
upstream for a particular cache item. Others will wait for a response
to appear in cache (or cache lock released) up to proxy_cache_lock_timeout.
Waiting requests will recheck if they have cached response ready (or are
allowed to run) every 500ms.
Note: we intentionally don't intercept NGX_DECLINED possibly returned by
ngx_http_file_cache_read(). This needs more work (possibly safe, but needs
further investigation). Anyway, it's exceptional situation.
Note: probably there should be a way to disable caching of responses
if there is already one request fetching resource to cache (without waiting
at all). Two possible ways include another cache lock option ("no_cache")
or using proxy_no_cache with some supplied variable.
Note: probably there should be a way to lock updating requests as well. For
now "proxy_cache_use_stale updating" is available.
This resolves issue with try_files (see ticket #70), configuration like
location / { try_files $uri /index.php; }
location /index.php { proxy_pass http://backend; }
caused nginx to use original request uri in a request to a backend.
Historically, not clearing of the r->valid_unparsed_uri on internal redirect
was a feature: it allowed to pass the same request to (another) upstream
server via error_page redirection. Since then named locations appeared
though, and it's time to start resetting r->valid_unparsed_uri on internal
redirects. Configurations still using this feature should be converted
to use named locations instead.
Patch by Lanshun Zhou.
Check if received data length match Content-Length header (if present),
don't cache response if no match found. This prevents caching of corrupted
response in case of premature connection close by upstream.
If cache was bypassed with proxy_cache_bypass, cache-controlling headers
(Cache-Control, Expires) wasn't considered and response was cached even
if it was actually non-cacheable.
Patch by John Ferlito.
Configuration with duplicate upstream blocks defined after first use, i.e.
like
server {
...
location / {
proxy_pass http://backend;
}
}
upstream backend { ... }
upstream backend { ... }
now correctly results in "duplicate upstream" error.
Additionally, upstream blocks defined after first use now handle various
server directive parameters ("weight", "max_fails", etc.). Previously
configuration like
server {
...
location / {
proxy_pass http://backend;
}
}
upstream backend {
server 127.0.0.1 max_fails=5;
}
incorrectly resulted in "invalid parameter "max_fails=5"" error.
For normal cached responses ngx_http_cache_send() sends last buffer and then
request finalized via ngx_http_finalize_request() call, i.e. everything is
ok.
But for stale responses (i.e. when upstream died, but we have something in
cache) the same ngx_http_cache_send() sends last buffer, but then in
ngx_http_upstream_finalize_request() another last buffer is send. This
causes duplicate final chunk to appear if chunked encoding is used (and
resulting problems with keepalive connections and so on).
Fix this by not sending in ngx_http_upstream_finalize_request()
another last buffer if we know response was from cache.
This fixes crashes observed with some 3rd party balancer modules. Standard
balancer modules (round-robin and ip hash) explicitly set pc->connection
(aka u->peer.connection) to NULL and aren't affected.
As long as ngx_event_pipe() has more data read from upstream than specified
in p->length it's passed to input filter even if buffer isn't yet full. This
allows to process data with known length without relying on connection close
to signal data end.
By default p->length is set to -1 in upstream module, i.e. end of data is
indicated by connection close. To set it from per-protocol handlers upstream
input_filter_init() now called in buffered mode (as well as in
unbuffered mode).
Previous use of size_t may cause wierd effects on 32bit platforms with certain
big responses transferred in unbuffered mode.
Nuke "if (size > u->length)" check as it's not usefull anyway (preread
body data isn't subject to this check) and now requires additional check
for u->length being positive.
We no longer use r->headers_out.content_length_n as a primary source of
backend's response length. Instead we parse response length to
u->headers_in.content_length_n and copy to r->headers_out.content_length_n
when needed.
