On 4/19/20 5:26 PM, Yann Ylavic wrote:
> On Fri, Apr 17, 2020 at 11:50 PM Yann Ylavic <[email protected]> wrote:
>> On Fri, Apr 17, 2020 at 9:43 PM Ruediger Pluem
>>> On 4/17/20 3:07 PM, ylavic
>>>> - apr_brigade_destroy(tmp_bb);
>>> Why don't we destroy it any longer?
>> tmp_bb is reused below in the die_early code, and always _cleanup()
>> after use in ap_read_request().
Thanks for pointing out. I guess what got me confused is that we use the same
variable name tmp_bb at the beginning for the brigade created from r->pool, but
in after die we override it with one we get via ap_acquire_brigade. I know that
we need to do this because we use the brigade to sent the EOR bucket which
destroys
r->pool when destroyed. But maybe we should use a different name in this section
e.g. eor_bb instead of tmp_bb.
>
> ap_read_request.diff
>
> Index: server/protocol.c
> ===================================================================
> --- server/protocol.c (revision 1876675)
> +++ server/protocol.c (working copy)
> @@ -1526,9 +1526,19 @@ request_rec *ap_read_request(conn_rec *conn)
> }
> }
>
> + /*
> + * Add the HTTP_IN filter here to ensure that ap_discard_request_body
> + * called by ap_die and by ap_send_error_response works correctly on
> + * status codes that do not cause the connection to be dropped and
> + * in situations where the connection should be kept alive.
> + */
> + ap_add_input_filter_handle(ap_http_input_filter_handle,
> + NULL, r, r->connection);
> +
> + /* Validate Host/Expect headers and select vhost. */
> if (!ap_check_request_header(r)) {
> access_status = r->status;
> - goto die_early;
> + goto die_before_hooks;
> }
Shouldn't we ensure that
r->per_dir_config = r->server->lookup_defaults;
is done before we jump to die_before_hooks ?
Otherwise I think looks good. As said my biggest worry was that we do not use
vhost specific ErrorDocuments in cases where we used
them.
Regards
Rüdiger
