core_filters.c

Checkout Tools
  • last updated 7 hours ago
Constraints
Constraints: committers
 
Constraints: files
Constraints: dates
Fix a signed/unsigned comparison that can never match.

-1 is a valid length value (for socket, pipe and cgi buckets for example)

All path I've checked cast the -1 to (apr_size_t) in order for the comparison to work. So do it as well here.

This has been like that in trunk since r708144, about 11 years ago, so I assume that it is not really an issue.

Spotted by gcc 9.1 and -Wextra

* Ensure that aborted connections are logged as such.

Set c->aborted before apr_brigade_cleanup to have the correct status

when logging the request as apr_brigade_cleanup triggers the logging

of the request if it contains an EOR bucket.

PR: 62823

Submitted by: Arnaud Grandville <contact@grandville.net>

Reviewed by:rpluem

  1. … 1 more file in changeset.
Follow up to r1840265: really privatize ap_filter_{recycle,adopt_brigade}().

Move ap_filter_adopt_brigade()'s declaration to "server/core.h" (private).

For ap_filter_recycle(), make it static/internal to util_filter (renamed to

recycle_dead_filters() which better fits what it does). It's now also called

unconditionally from ap_filter_input_pending() which itself is always called

after the request processing and from MPM event (as input_pending hook).

  1. … 4 more files in changeset.
Follow up to r1840149: core input filter pending data.

Since r1840149 ap_core_input_filter() can't use use f->[priv->]bb directly, so

ap_filter_input_pending() stopped accounting for its pending data.

But ap_core_input_filter() can't (and doesn't need to) setaside its socket

bucket, so ap_filter_setaside_brigade() is not an option. This commit adds

ap_filter_adopt_brigade() which simply moves the given buckets (brigade) into

f->priv->bb, and since this is not something to be done blindly (the buckets

need to have c->pool/bucket_alloc lifetime, which is the case in the core

filter) the function is not AP_DECLAREd/exported thus can be used in core only.

With ap_filter_adopt_brigade() and ap_filter_reinstate_brigade(), the core

input is now ap_filter_input_pending() friendly.

Also, ap_filter_recycle() is no more part of the API (AP_DECLARE removed too),

there really is no point to call it outside core code. MAJOR bumped once again

because of this.

  1. … 4 more files in changeset.
util_filter: protect ap_filter_t private fields from external (ab)use.

Introduce opaque struct ap_filter_private to move ap_filter_t "pending", "bb"

and "deferred_pool" fields to the "priv" side of things.

This allows to trust values set internally (only!) in util_filter code, and

make useful assertions between the different functions calls, along with the

usual nice extensibility property.

Likewise, the private struct ap_filter_conn_ctx in conn_rec (from r1839997)

allows now to implement the new ap_acquire_brigade() and ap_release_brigade()

functions useful to get a brigade with c->pool's lifetime. They obsolete

ap_reuse_brigade_from_pool() which is replaced where previously used.

Some comments added in ap_request_core_filter() regarding the lifetime of the

data it plays with, up to EOR...

MAJOR bumped (once again).

  1. … 7 more files in changeset.
core: always allocate filters (ap_filter_t) on f->c->pool.

When filters are allocated on f->r->pool, they may be destroyed any time

underneath themselves which makes it hard for them to be passed the EOR and

forward it (*f can't be dereferenced anymore when the EOR is destroyed, thus

before request filters return).

On the util_filter side, it also makes it impossible to flush pending request

filters when they have set aside the EOR, since f->bb can't be accessed after

it's passed to the f->next.

So we always use f->c->pool to allocate filters and pending brigades, and to

avoid leaks with keepalive requests (long living connections handling multiple

requests), filters and brigades are recycled with a cleanup on f->r->pool.

Recycling is done (generically) with a spare data ring (void pointers), and a

filter(s) context struct is associated with the conn_rec to maintain the rings

by connection, that is:

struct ap_filter_conn_ctx {

struct ap_filter_ring *pending_input_filters;

struct ap_filter_ring *pending_output_filters;

struct ap_filter_spare_ring *spare_containers,

*spare_brigades,

*spare_filters,

*spare_flushes;

int flushing;

};

MMN major bumped (again).

  1. … 7 more files in changeset.
core: follow up to r1836237: core filter's tmp_flush_bb not used anymore.

core: follow up to r1836237: whitelist in-memory buckets.

As Eric noted, this is safer w.r.t. third party buckets which could be

uncought by the previous backlist and get out of memory limits.

core: core output filter optimizations.

The core output filter used to determine first if it needed to block before

trying to send its data (including set aside ones), and if so it did call

send_brigade_blocking().

This can be avoided by making send_brigade_nonblocking() send as much data as

possible (nonblocking), and only if data remain check whether they should be

flushed (blocking), according to the same ap_filter_reinstate_brigade()

heuristics but afterward.

This allows both to simplify the code (axe send_brigade_blocking and some

duplicated logic) and optimize sends since send_brigade_nonblocking() is now

given all the buckets so it can make use of scatter/gather (iovec) or NOPUSH

option with the whole picture.

When sendfile is available and/or with fine tuning of FlushMaxThreshold (and

ReadBufferSize) from r1836032, one can now take advantage of modern network

speeds and bandwidth.

This commit also adds some APLOG_TRACE6 messages for outputed bytes (including

at mod_ssl level since splitting happens there when it's active).

  1. … 2 more files in changeset.
core: Extend support for setting aside data from the network input filter

to any connection or request input filter.

  1. … 13 more files in changeset.
Remain compatible with legacy MPMs that passed NULL to the network filter

core: Extend support for asynchronous write completion from the

network filter to any connection or request filter.

  1. … 15 more files in changeset.
core_filters: restore/disable TCP_NOPUSH option after non-blocking sendfile.
core: Cleanup the request soon/even if some output filter fails to

handle the EOR bucket.

  1. … 2 more files in changeset.
Turn some APR_BUCKET_REMOVE(e)+apr_bucket_destroy(e) into the equivalent apr_bucket_delete(e) to reduce code verbosity

  1. … 4 more files in changeset.
Core filters do not need mod_proxy.h
Strip useless apr_brigade_cleanup() calls.
  1. … 6 more files in changeset.
core: Don't truncate output when sending is interrupted by a signal,

such as from an exiting CGI process.

PR: 55643

  1. … 1 more file in changeset.
add high trace level log messages for debugging buffering and write completion

  1. … 1 more file in changeset.
restore "core_output_filter: writing data to the network" message

when c->aborted is set in the core output filter, but now at TRACE1.

  1. … 1 more file in changeset.
Windows: Fix SSL failures on windows with AcceptFilter https none.

The now-stray apr_socket_opt_set(APR_SO_NONBLOCK=On) call was

originally added with r327872. The call was harmless on Unix

due to APR's use of non-blocking sockets when implementing timeouts

on that platform, but harmful on Windows since it collided with

APR's different implementation of timeouts on that platform.

PR: 52476

  1. … 1 more file in changeset.
Replace ap_create_core_ctx()/ap_core_ctx_get_bb() with a hook

  1. … 7 more files in changeset.
Fix various filter functions to return apr_status_t instead of int

  1. … 6 more files in changeset.
* Don't typedef twice (in .c and .h file).
Make the core input/output filter contexts private and provide accessor APIs

for mpm_winnt and mod_ftp.

This allows to add members to the context structs without breaking binary

compatibility.

  1. … 4 more files in changeset.
* server/core_filters.c (send_brigade_nonblocking): Use a non-blocking

bucket read, allowing any pending data to be flushed before trying a

(potentially slow) blocking read.

* server/core_filters.c (ap_core_output_filter): Don't read the entire

output of a morphing bucket into RAM.

Submitted by: jorton, sf

ctx->bytes_in is never used. Remove a useless iteration through the brigade

Document the rather surprising code path on Windows

* server/core_filters.c (ap_core_input_filter): Only treat EAGAIN as

success if a non-blocking read was requested; for a blocking read,

it is an error condition.