On 4/17/20 3:07 PM, [email protected] wrote:
> Author: ylavic
> Date: Fri Apr 17 13:07:46 2020
> New Revision: 1876664
>
> URL: http://svn.apache.org/viewvc?rev=1876664&view=rev
> Log:
> core, h2: send EOR for early HTTP request failure.
>
> The core output filters depend on EOR being sent at some point for correct
> accounting of setaside limits and lifetime.
>
> Rework ap_read_request() early failure (including in post_read_request()
> hooks)
> so that it always sends the EOR after ap_die().
>
> Apply the same scheme in h2_request_create_rec() which is the HTTP/2 to HTTP/1
> counterpart.
>
> Modified:
> httpd/httpd/trunk/modules/http2/h2_request.c
> httpd/httpd/trunk/server/protocol.c
>
> Modified: httpd/httpd/trunk/server/protocol.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/protocol.c?rev=1876664&r1=1876663&r2=1876664&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/protocol.c (original)
> +++ httpd/httpd/trunk/server/protocol.c Fri Apr 17 13:07:46 2020
> @@ -1453,14 +1444,12 @@ request_rec *ap_read_request(conn_rec *c
> }
> }
>
> - apr_brigade_destroy(tmp_bb);
Why don't we destroy it any longer?
> -
> /* update what we think the virtual host is based on the headers we've
> * now read. may update status.
> */
> -
> - access_status = ap_update_vhost_from_headers_ex(r,
> conf->strict_host_check == AP_CORE_CONFIG_ON);
> - if (conf->strict_host_check == AP_CORE_CONFIG_ON && access_status !=
> HTTP_OK) {
> + strict_host_check = (conf->strict_host_check == AP_CORE_CONFIG_ON);
> + access_status = ap_update_vhost_from_headers_ex(r, strict_host_check);
> + if (strict_host_check && access_status != HTTP_OK) {
> if (r->server == ap_server_conf) {
> ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(10156)
> "Requested hostname '%s' did not match any
> ServerName/ServerAlias "
> @@ -1472,22 +1461,13 @@ request_rec *ap_read_request(conn_rec *c
> "current connection is %s:%u)",
> r->hostname, r->server->defn_name,
> r->server->defn_line_number);
> }
> - r->status = access_status;
> + goto die_early;
> }
> -
> - access_status = r->status;
> -
> - /* Toggle to the Host:-based vhost's timeout mode to fetch the
> - * request body and send the response body, if needed.
> - */
> - if (cur_timeout != r->server->timeout) {
> - apr_socket_timeout_set(csd, r->server->timeout);
> - cur_timeout = r->server->timeout;
> + if (r->status != HTTP_OK) {
> + access_status = r->status;
> + goto die_early;
Why do we die_early here and not just die later?
> }
>
> - /* we may have switched to another server */
> - r->per_dir_config = r->server->lookup_defaults;
> -
> if ((!r->hostname && (r->proto_num >= HTTP_VERSION(1, 1)))
> || ((r->proto_num == HTTP_VERSION(1, 1))
> && !apr_table_get(r->headers_in, "Host"))) {
> @@ -1498,10 +1478,23 @@ request_rec *ap_read_request(conn_rec *c
> * HTTP/1.1 mentions twice (S9, S14.23) that a request MUST contain
> * a Host: header, and the server MUST respond with 400 if it
> doesn't.
> */
> - access_status = HTTP_BAD_REQUEST;
> ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00569)
> "client sent HTTP/1.1 request without hostname "
> "(see RFC2616 section 14.23): %s", r->uri);
> + access_status = HTTP_BAD_REQUEST;
> + goto die_early;
Why do we die_early here and not just die later?
> + }
> +
> + /* we may have switched to another server */
> + r->per_dir_config = r->server->lookup_defaults;
> + conf = ap_get_core_module_config(r->server->module_config);
> +
> + /* Toggle to the Host:-based vhost's timeout mode to fetch the
> + * request body and send the response body, if needed.
> + */
> + if (cur_timeout != r->server->timeout) {
> + apr_socket_timeout_set(csd, r->server->timeout);
> + cur_timeout = r->server->timeout;
> }
>
> /*
Regards
Rüdiger
