Markus Linnala [Fri, 20 Sep 2013 14:57:21 +0000 (17:57 +0300)]
Core: fix misallocation at ngx_crypt_apr1 (ticket #412).
Found by using auth_basic.t from mdounin nginx-tests under valgrind.
==10470== Invalid write of size 1
==10470== at 0x43603D: ngx_crypt_to64 (ngx_crypt.c:168)
==10470== by 0x43648E: ngx_crypt (ngx_crypt.c:153)
==10470== by 0x489D8B: ngx_http_auth_basic_crypt_handler (ngx_http_auth_basic_module.c:297)
==10470== by 0x48A24A: ngx_http_auth_basic_handler (ngx_http_auth_basic_module.c:240)
==10470== by 0x44EAB9: ngx_http_core_access_phase (ngx_http_core_module.c:1121)
==10470== by 0x44A822: ngx_http_core_run_phases (ngx_http_core_module.c:895)
==10470== by 0x44A932: ngx_http_handler (ngx_http_core_module.c:878)
==10470== by 0x455EEF: ngx_http_process_request (ngx_http_request.c:1852)
==10470== by 0x456527: ngx_http_process_request_headers (ngx_http_request.c:1283)
==10470== by 0x456A91: ngx_http_process_request_line (ngx_http_request.c:964)
==10470== by 0x457097: ngx_http_wait_request_handler (ngx_http_request.c:486)
==10470== by 0x4411EE: ngx_epoll_process_events (ngx_epoll_module.c:691)
==10470== Address 0x5866fab is 0 bytes after a block of size 27 alloc'd
==10470== at 0x4A074CD: malloc (vg_replace_malloc.c:236)
==10470== by 0x43B251: ngx_alloc (ngx_alloc.c:22)
==10470== by 0x421B0D: ngx_malloc (ngx_palloc.c:119)
==10470== by 0x421B65: ngx_pnalloc (ngx_palloc.c:147)
==10470== by 0x436368: ngx_crypt (ngx_crypt.c:140)
==10470== by 0x489D8B: ngx_http_auth_basic_crypt_handler (ngx_http_auth_basic_module.c:297)
==10470== by 0x48A24A: ngx_http_auth_basic_handler (ngx_http_auth_basic_module.c:240)
==10470== by 0x44EAB9: ngx_http_core_access_phase (ngx_http_core_module.c:1121)
==10470== by 0x44A822: ngx_http_core_run_phases (ngx_http_core_module.c:895)
==10470== by 0x44A932: ngx_http_handler (ngx_http_core_module.c:878)
==10470== by 0x455EEF: ngx_http_process_request (ngx_http_request.c:1852)
==10470== by 0x456527: ngx_http_process_request_headers (ngx_http_request.c:1283)
==10470==
On win32, time_t is 64 bits wide by default, and passing an ngx_msec_int_t
argument for %T format specifier doesn't work. This doesn't manifest itself
on other platforms as time_t and ngx_msec_int_t are usually of the same size.
SPDY: fixed segfault with "client_body_in_file_only" enabled.
It is possible to send FLAG_FIN in additional empty data frame, even if it is
known from the content-length header that request body is empty. And Firefox
actually behaves like this (see ticket #357).
To simplify code we sacrificed our microoptimization that did not work right
due to missing check in the ngx_http_spdy_state_data() function for rb->buf
set to NULL.
Maxim Dounin [Fri, 31 May 2013 10:59:26 +0000 (14:59 +0400)]
Win32: accept_mutex now always disabled (ticket #362).
Use of accept mutex on win32 may result in a deadlock if there are multiple
worker_processes configured and the mutex is grabbed by a process which
can't accept connections.
Maxim Dounin [Tue, 21 May 2013 17:47:50 +0000 (21:47 +0400)]
Upstream: fixed fail_timeout and max_fails > 1.
Due to peer->checked always set since rev. c90801720a0c (1.3.0)
by round-robin and least_conn balancers (ip_hash not affected),
the code in ngx_http_upstream_free_round_robin_peer() function
incorrectly reset peer->fails too often.
Reported by Dmitry Popov,
http://mailman.nginx.org/pipermail/nginx-devel/2013-May/003720.html
Piotr Sikora [Thu, 16 May 2013 22:37:13 +0000 (15:37 -0700)]
OCSP stapling: fix error logging of successful OCSP responses.
Due to a bad argument list, nginx worker would crash (SIGSEGV) while
trying to log the fact that it received OCSP response with "revoked"
or "unknown" certificate status.
While there, fix similar (but non-crashing) error a few lines above.
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
Maxim Dounin [Sat, 11 May 2013 17:12:24 +0000 (21:12 +0400)]
Proxy: $proxy_internal_body_length fixed.
The $proxy_internal_body_length value might change during request lifetime,
notably if proxy_set_body used, and use of a cached value might result in
incorrect upstream requests.
Maxim Dounin [Sat, 11 May 2013 14:49:30 +0000 (18:49 +0400)]
Fixed build with --with-mail_ssl_module.
If nginx was compiled without --with-http_ssl_module, but with some
other module which uses OpenSSL (e.g. --with-mail_ssl_module), insufficient
preprocessor check resulted in build failure. The problem was introduced
by e0a3714a36f8 (1.3.14).
SPDY: set NGX_TCP_NODELAY_DISABLED for fake connections.
This is to avoid setting the TCP_NODELAY flag on SPDY socket in
ngx_http_upstream_send_response(). The latter works per request,
but in SPDY case it might affect other streams in connection.
As of 1.3.9, chunked request body may be available with
r->headers_in.content_length_n <= 0. Additionally, request body
may be in multiple buffers even if r->request_body_in_single_buf
was requested.
Dependancy tracking introduced in r5169 were not handled absolute path
names properly. Absolute names might appear in CORE_DEPS if --with-openssl
or --with-pcre configure arguments are used to build OpenSSL/PCRE
libraries.
Additionally, revert part of r5169 to set NGX_INCS from Makefile
variables. Makefile variables have $ngx_include_opt in them, which
might result in wrong include paths being used. As a side effect,
this also restores build with --with-http_perl_module and --without-http
at the same time.
Request body: only read body in main request (ticket #330).
Before 1.3.9 an attempt to read body in a subrequest only caused problems
if body wasn't already read or discarded in a main request. Starting with
1.3.9 it might also cause problems if body was discarded by a main request
before subrequest start.
Fix is to just ignore attempts to read request body in a subrequest, which
looks like right thing to do anyway.
We generate both read and write events if an error event was returned by
port_getn() without POLLIN/POLLOUT, but we should not try to handle inactive
events, they may even have no handler.
Configure: fixed nginx.so rebuild (broken by r5145).
To avoid further breaks it's now done properly, all the dependencies
are now passed to Makefile.PL. While here, fixed include list passed to
Makefile.PL to use Makefile variables rather than a list expanded during
configure.
Problems with setsockopt(TCP_NODELAY) and setsockopt(TCP_NOPUSH), as well
as sendfile() syscall on Solaris, are specific to UNIX-domain sockets.
Other address families, i.e. AF_INET and AF_INET6, are fine.
Use NGX_FILE_ERROR for handling file operations errors.
On Win32 platforms 0 is used to indicate errors in file operations, so
comparing against -1 is not portable.
This was not much of an issue in patched code, since only ngx_fd_info() test
is actually reachable on Win32 and in worst case it might result in bogus
error log entry.
Maxim Dounin [Fri, 22 Mar 2013 15:14:07 +0000 (15:14 +0000)]
Misc: switch to single export operation in "zip" target.
While exporting parts of the tree might be better in some cases, it
is awfully slow overseas, and also requires unlocking ssh key multiple
times. Exporting the whole repo and removing directories not needed for
zip is faster here.
It is also a required step before we can switch to Mercurial.
Status: introduced the "ngx_stat_waiting" counter.
And corresponding variable $connections_waiting was added.
Previously, waiting connections were counted as the difference between
active connections and the sum of reading and writing connections.
That made it impossible to count more than one request in one connection
as reading or writing (as is the case for SPDY).
Also, we no longer count connections in handshake state as waiting.
Allow to reuse connections that wait their first request.
This should improve behavior under deficiency of connections.
Since SSL handshake usually takes significant amount of time,
we exclude connections from reusable queue during this period
to avoid premature flush of them.
Maxim Dounin [Thu, 14 Mar 2013 12:37:54 +0000 (12:37 +0000)]
Upstream: call ngx_http_run_posted_requests() on resolve errors.
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;
}
Maxim Dounin [Thu, 14 Mar 2013 12:30:26 +0000 (12:30 +0000)]
Request body: avoid linking rb->buf to r->header_in.
Code to reuse of r->request_body->buf in upstream module assumes it's
dedicated buffer, hence after 1.3.9 (r4931) it might reuse r->header_in
if client_body_in_file_only was set, resulting in original request
corruption. It is considered to be safer to always create a dedicated
buffer for rb->bufs to avoid such problems.
Maxim Dounin [Thu, 14 Mar 2013 12:28:53 +0000 (12:28 +0000)]
Request body: next upstream fix.
After introduction of chunked request body handling in 1.3.9 (r4931),
r->request_body->bufs buffers have b->start pointing to original buffer
start (and b->pos pointing to real data of this particular buffer).
While this is ok as per se, it caused bad things (usually original request
headers included before the request body) after reinit of the request
chain in ngx_http_upstream_reinit() while sending the request to a next
upstream server (which used to do b->pos = b->start for each buffer
in the request chain).
Maxim Dounin [Tue, 12 Mar 2013 13:38:04 +0000 (13:38 +0000)]
Fixed logging in ngx_http_wait_request_handler().
If c->recv() returns 0 there is no sense in using ngx_socket_errno for
logging, its value meaningless. (The code in question was copied from
ngx_http_keepalive_handler(), but ngx_socket_errno makes sense there as it's
used as a part of ECONNRESET handling, and the c->recv() call is preceeded
by the ngx_set_socket_errno(0) call.)
In r2411 setting of NGX_HTTP_GZIP_BUFFERED in c->buffered was moved from
ngx_http_gzip_filter_deflate_start() to ngx_http_gzip_filter_buffer() since
it was always called first. But in r2543 the "postpone_gzipping" directive
was introduced, and if postponed gzipping is disabled (the default setting),
ngx_http_gzip_filter_buffer() is not called at all.
We must always set NGX_HTTP_GZIP_BUFFERED after the start of compression
since there is always a trailer that is buffered.
There are no known cases when it leads to any problem with current code.
But we already had troubles in upcoming SPDY implementation.
Not only this is useful for the upcoming SPDY support, but it can
also help to improve HTTPS performance by enabling TLS False Start
in Chrome/Chromium browsers [1]. So, we always enable NPN for HTTPS
if it is supported by OpenSSL.
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.
In Linux 2.6.32, TCP_DEFER_ACCEPT was changed to accept connections
after the deferring period is finished without any data available.
(Reading from the socket returns EAGAIN in this case.)
Since in nginx TCP_DEFER_ACCEPT is set to "post_accept_timeout", we
do not need to wait longer if deferred accept returns with no data.
Use "client_header_timeout" for all requests in a connection.
Previously, only the first request in a connection used timeout
value from the "client_header_timeout" directive while reading
header. All subsequent requests used "keepalive_timeout" for
that.
It happened because timeout of the read event was set to the
value of "keepalive_timeout" in ngx_http_set_keepalive(), but
was not removed when the next request arrived.
Create request object only after the first byte was received.
Previously, we always created an object and logged 400 (Bad Request)
in access log if a client closed connection without sending any data.
Such a connection was counted as "reading".
Since it's common for modern browsers to behave like this, it's no
longer considered an error if a client closes connection without
sending any data, and such a connection will be counted as "waiting".
Now, we do not log 400 (Bad Request) and keep memory footprint as
small as possible.
SNI: added restriction on requesting host other than negotiated.
According to RFC 6066, client is not supposed to request a different server
name at the application layer. Server implementations that rely upon these
names being equal must validate that a client did not send a different name
in HTTP request. Current versions of Apache HTTP server always return 400
"Bad Request" in such cases.
There exist implementations however (e.g., SPDY) that rely on being able to
request different host names in one connection. Given this, we only reject
requests with differing host names if verification of client certificates
is enabled in a corresponding server configuration.
An example of configuration that might not work as expected: