Instead of independant inheritance of conf->upstream.upstream (proxy_pass
without variables) and conf->proxy_lengths (proxy_pass with variables)
we now test them both and inherit only if neither is set. Additionally,
SSL context is also inherited only in this case now.
Based on the patch by Alexey Radkov.
RFC7232 says:
The 304 (Not Modified) status code indicates that a conditional GET
or HEAD request has been received and would have resulted in a 200
(OK) response if it were not for the fact that the condition
evaluated to false.
which means that there is no reason to send requests with "If-None-Match"
and/or "If-Modified-Since" headers for responses cached with other status
codes.
Also, sending conditional requests for responses cached with other status
codes could result in a strange behavior, e.g. upstream server returning
304 Not Modified for cached 404 Not Found responses, etc.
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
In case of a cache lock timeout and in the aio handler we now call
r->write_event_handler() instead of a connection write handler,
to make sure to run appropriate subrequest. Previous code failed to run
inactive subrequests and hence resulted in suboptimal behaviour, see
report by Yichun Zhang:
http://mailman.nginx.org/pipermail/nginx-devel/2013-October/004435.html
(Infinite hang claimed in the report seems impossible without 3rd party
modules, as subrequests will be eventually woken up by the postpone filter.)
To ensure proper logging make sure to set current_request in all event
handlers, including resolve, ssl handshake, cache lock wait timer and
aio read handlers. A macro ngx_http_set_log_request() introduced to
simplify this.
The alert was introduced in 03ff14058272 (1.5.4), and was triggered on each
post_action invocation.
There is no real need to call header filters in case of post_action,
so return NGX_OK from ngx_http_send_header() if r->post_action is set.
This helps to avoid delays in sending the last chunk of data because
of bad interaction between Nagle's algorithm on nginx side and
delayed ACK on the client side.
Delays could also be caused by TCP_CORK/TCP_NOPUSH if SPDY was
working without SSL and sendfile() was used.
The upstream modules remove and alter a number of client headers
before sending the request to upstream. This set of headers is
smaller or even empty when cache is disabled.
It's still possible that a request in a cache-enabled location is
uncached, for example, if cache entry counter is below min_uses.
In this case it's better to alter a smaller set of headers and
pass more client headers to backend unchanged. One of the benefits
is enabling server-side byte ranges in such requests.
Once this age is reached, the cache lock is discarded and another
request can acquire the lock. Requests which failed to acquire
the lock are not allowed to cache the response.
For further progress a new buffer must be at least two bytes larger than
the remaining unparsed data. One more byte is needed for null-termination
and another one for further progress. Otherwise inflate() fails with
Z_BUF_ERROR.
Previously, nginx would emit empty values in a header with multiple,
NULL-separated values.
This is forbidden by the SPDY specification, which requires headers to
have either a single (possibly empty) value or multiple, NULL-separated
non-empty values.
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
When got multiple upstream IP addresses using DNS resolving, the number of
upstreams tries and the maxinum time spent for these tries were not affected.
This patch fixed it.
Spaces in Accept-Charset, Accept-Encoding, and Accept-Language headers
are now ignored. As per syntax of these headers spaces can only appear
in places where they are optional.
If a variant stored can't be used to respond to a request, the variant
hash is used as a secondary key.
Additionally, if we previously switched to a secondary key, while storing
a response to cache we check if the variant hash still apply. If not, we
switch back to the original key, to handle cases when Vary changes.
To cache responses with Vary, we now calculate hash of headers listed
in Vary, and return the response from cache only if new request headers
match.
As of now, only one variant of the same resource can be stored in cache.
Previous code resulted in transfer stalls when client happened
to read all the data in buffers at once, while all gzip buffers
were exhausted (but ctx->nomem wasn't set). Make sure to call
next body filter at least once per call if there are busy buffers.
Additionally, handling of calls with NULL chain was changed to follow
the same logic, i.e., next body filter is only called with NULL chain
if there are busy buffers. This is expected to fix "output chain is empty"
alerts as reported by some users after c52a761a2029 (1.5.7).
Due to the u->headers_in.last_modified_time not being correctly initialized,
this variable was evaluated to "Thu, 01 Jan 1970 00:00:00 GMT" for responses
cached without the "Last-Modified" header which resulted in subsequent proxy
requests being sent with "If-Modified-Since: Thu, 01 Jan 1970 00:00:00 GMT"
header.
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
The c->sent is reset to 0 on each request by server-side http code,
so do the same on client side. This allows to count number of bytes
sent in a particular request.
One intentional side effect of this change is that key is allowed only
in the first position. Previously, it was possible to specify the key
variable at any position, but that was never documented, and is contrary
with nginx configuration practice for positional parameters.
One intentional side effect of this change is that key is allowed only
in the first position. Previously, it was possible to specify the key
variable at any position, but that was never documented, and is contrary
to nginx configuration practice for positional parameters.
Previously, a file buffer start position was reset to the file start.
Now it's reset to the previous file buffer end. This fixes
reinitialization of requests having multiple successive parts of a
single file. Such requests are generated by fastcgi module.
The new directives {proxy,fastcgi,scgi,uwsgi,memcached}_next_upstream_tries
and {proxy,fastcgi,scgi,uwsgi,memcached}_next_upstream_timeout limit
the number of upstreams tried and the maximum time spent for these tries
when searching for a valid upstream.
When memory allocation failed in ngx_http_upstream_cache(), the connection
would be terminated directly in ngx_http_upstream_init_request().
Return a INTERNAL_SERVER_ERROR response instead.
The etag->hash must be set to 0 to avoid an empty ETag header being
returned with the 500 Internal Server Error page after the memory
allocation failure.
Reported by Markus Linnala.
The messages "ngx_slab_alloc() failed: no memory in cache keys zone"
from the file cache slab allocator are suppressed since the allocation
is likely to succeed after the forced expiration of cache nodes.
The second allocation failure is reported.
In theory, this can provide a bit better distribution of latencies.
Also it simplifies the code, since ngx_queue_t is now used instead
of custom implementation.
Previously, a configuration like
location / {
ssi on;
ssi_types *;
set $http_foo "bar";
return 200 '<!--#echo var="http_foo" -->\n';
}
resulted in NULL pointer dereference in ngx_http_get_variable() as
the variable was explicitly added to the variables hash, but its
get_handler wasn't properly set in the hash. Fix is to make sure
that get_handler is properly set by ngx_http_variables_init_vars().
The SPDY module doesn't expect timers can be set on stream events for reasons
other than delaying output. But ngx_http_writer() could add timer on write
event if the delayed flag wasn't set and nginx is waiting for AIO completion.
That could cause delays in sending response over SPDY when file AIO was used.
perl_parse() function expects argv/argc-style argument list,
which according to the C standard must be NULL-terminated,
that is: argv[argc] == NULL.
This change fixes a crash (SIGSEGV) that could happen because
of the buffer overrun during perl module initialization.
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
Timeout may not be set on an upstream connection when we call
ngx_ssl_handshake() in ngx_http_upstream_ssl_init_connection(),
so make sure to arm it if it's not set.
Based on a patch by Yichun Zhang.
The ngx_http_geoip_city_float_variable and
ngx_http_geoip_city_int_variable functions did not always initialize
all variable fields like "not_found", which could lead to empty values
for those corresponding nginx variables randomly.
Previously, ngx_http_map_uri_to_path() errors were not checked in
ngx_http_upstream_store(). Moreover, in case of errors temporary
files were not deleted, as u->store was set to 0, preventing cleanup
code in ngx_http_upstream_finalize_request() from removing them. With
this patch, u->store is set to 0 only if there were no errors.
Reported by Feng Gu.
This ensures that debug logging and the $uri variable (if used in
400 Bad Request processing) will not try to access uninitialized
memory.
Found by Sergey Bobrov.
Split SPDY header with multiple, NULL-separated values:
cookie: foo\0bar
into two separate HTTP headers with the same name:
cookie: foo
cookie: bar
Even though the logic for this behavior already existed
in the source code, it doesn't look that it ever worked
and SPDY streams with such headers were simply rejected.
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
SSL_SESSION struct is internal part of the OpenSSL library and it's fields
should be accessed via API (when exposed), not directly.
The unfortunate side-effect of this change is that we're losing reference
count that used to be printed at the debug log level, but this seems to be
an acceptable trade-off.
Almost fixes build with -DOPENSSL_NO_SSL_INTERN.
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
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.
log->filter ("if" parameter) was uninitialized when the default value
was being used, which would lead to a crash (SIGSEGV) when access_log
directive wasn't specified in the configuration.
Zero-fill the whole structure instead of zeroing fields one-by-one
in order to prevent similar issues in the future.
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
Since the type cast has precedence higher than the bit shift operator,
all values were truncated to 8 bits.
These macros are used to construct header block for SYN_REPLY frame on
platforms with strict alignment requirements. As a result, any response
that contains a header with name or value longer than 255 bytes was
corrupted on such platforms.
Do not taste the last parameter against directory, as otherwise it would
result in the trailing slash being cut from the parameter value.
Notably, this prevents an internal redirect to an empty URI
if the parameter is set to the literal slash:
location / { try_files $uri /; }
In particular, properly output partial match at the end of a subrequest
response (much like we do at the end of a response), and reset/set the
last_in_chain flag as appropriate.
Reported by KAWAHARA Masashi.
If response is gzipped we can't recode response, but in case it's not
needed we still can add charset to Content-Type.
The r->ignore_content_encoding is dropped accordingly, charset with gzip_static
now properly works without any special flags.
The ngx_http_map_uri_to_path() function used clcf->regex to detect if
it's working within a location given by a regular expression and have
to replace full URI with alias (instead of a part matching the location
prefix). This is incorrect due to clcf->regex being false in implicit
locations created by if and limit_except.
Fix is to preserve relevant information in clcf->alias instead, by setting
it to NGX_MAX_SIZE_T_VALUE if an alias was specified in a regex location.
Handling of PROXY protocol for SPDY connection is currently implemented as
a SPDY state. And while nginx waiting for PROXY protocol data it continues
to process SPDY connection: initializes zlib context, sends control frames.
- Specification-friendly handling of invalid header block or special headers.
Such errors are not fatal for session and shouldn't lead to connection close;
- Avoid mix of NGX_HTTP_PARSE_INVALID_REQUEST/NGX_HTTP_PARSE_INVALID_HEADER.
The function just calls ngx_http_spdy_state_headers_skip() most of the time.
There was also an attempt of optimization to stop parsing if the client already
closed connection, but it looks strange and unfinished anyway.
Now ngx_http_spdy_state_protocol_error() is able to close stream,
so there is no need in a separate call for this.
Also fixed zero status code in logs for some cases.
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).
While managing big caches it is possible that expiring old cache items
in ngx_http_file_cache_expire() will take a while. Added a check for
ngx_quit / ngx_terminate to make sure cache manager can be terminated
while in ngx_http_file_cache_expire().
The ngx_http_proxy_rewrite_cookie() function expects the value of the
"Set-Cookie" header to be null-terminated, and for headers obtained
from proxied server it is usually true.
Now the ngx_http_proxy_rewrite() function preserves the null character
while rewriting headers.
This fixes accessing memory outside of rewritten value if both the
"proxy_cookie_path" and "proxy_cookie_domain" directives are used in
the same location.
There's a race condition between closing a stream by one endpoint
and sending a WINDOW_UPDATE frame by another. So it would be better
to just skip such frames for unknown streams, like is already done
for the DATA frames.
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.
The SSL_CTX_set_cipher_list() may fail if there are no valid ciphers
specified in proxy_ssl_ciphers / uwsgi_ssl_ciphers, resulting in
SSL context leak.
In theory, ngx_pool_cleanup_add() may fail too, but this case is
intentionally left out for now as it's almost impossible and proper fix
will require changes to http ssl and mail ssl code as well.
This should prevent attempts of using pointer before it was checked, since
all modern compilers are able to spot access to uninitialized variable.
No functional changes.
Previously, an empty frame object was created for an output chain that contains
only sync or flush empty buffers. But since 39d7eef2e332 every DATA frame has
the flush flag set on its last buffer, so there's no need any more in additional
flush buffers in the output queue and they can be skipped.
Note that such flush frames caused an incorrect $body_bytes_sent value.
The SPDY draft 2 specification requires that if an endpoint receives a
control frame for a type it does not recognize, it must ignore the frame.
But the 3 and 3.1 drafts don't seem to declare any behavior for such case.
Then sticking with the previous draft in this matter looks to be right.
But previously, only 8 least significant bits of the type field were
parsed while the rest of 16 bits of the field were checked against zero.
Though there are no known frame types bigger than 255, this resulted in
inconsistency in handling of such frames: they were not recognized as
valid frames at all, and the connection was closed.
If start time is within the track but end time is out of it, error
"end time is out mp4 stts samples" is generated. However it's
better to ignore the error and output the track until its end.
The ngx_cacheline_size may be too low on some platforms, resulting
in unexpected hash build problems (as no collisions are tolerated due
to low bucket_size, and max_size isn't big enough to build a hash without
collisions). These problems aren't fatal anymore but nevertheless
need to be addressed.
The flag allows to suppress "ngx_slab_alloc() failed: no memory" messages
from a slab allocator, e.g., if an LRU expiration is used by a consumer
and allocation failures aren't fatal.
The flag is now used in the SSL session cache code, and in the limit_req
module.
Despite introducing start and end crop operations existing log
messages still mostly refer only to start. Logging is improved
to match both cases.
New debug logging is added to track entry count in atoms after
cropping.
Two format type mismatches are fixed as well.
When "start" value is equal to a track duration the request
fails with "time is out mp4 stts" like it did before track
duration check was added. Now such tracks are considered
short and skipped.
The SPDY/3.1 specification requires that the server must respond with
a 400 "Bad request" error if the sum of the data frame payload lengths
does not equal the size of the Content-Length header.
This also fixes "zero size buf in output" alert, that might be triggered
if client sends a greater than zero Content-Length header and closes
stream using the FIN flag with an empty request body.
There are a few cases in ngx_http_spdy_state_read_data() related to error
handling when ngx_http_spdy_state_skip() might be called with an inconsistent
state between *pos and sc->length, that leads to violation of frame layout
parsing and resuted in corruption of spdy connection.
Based on a patch by Xiaochen Wang.
Previously, only one case was checked: if there's more data to parse
in a r->header_in buffer, but the buffer can be filled to the end by
the last parsed entry, so we also need to check that there's no more
data to inflate.
