Commit f26c3267 authored by Jim Jagielski's avatar Jim Jagielski
Browse files

Merge r1657881, r1665643 from trunk:

http: Make ap_die() robust against any HTTP error code and not modify
response status (finally logged) when nothing is to be done.


ap_die(): follow up to r1657881.

Use log level DEBUG for AP_FILTER_ERROR => HTTP_INTERNAL_SERVER_ERROR.
Submitted by: ylavic
Reviewed/backported by: jim


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1681114 13f79535-47bb-0310-9956-ffa450edef68
parent 2e68cdae
Loading
Loading
Loading
Loading
+3 −0
Changes for CHANGES: 3 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -134,6 +134,9 @@ Changes with Apache 2.4.13
     a ProxyRemote forward-proxy.  PR 55892.  [Hendrik Harms <hendrik.harms
     gmail com>, William Rowe, Yann Ylavic]

  *) http: Make ap_die() robust against any HTTP error code and not modify
     response status (finally logged) when nothing is to be done. [Yann Ylavic]

  *) mod_proxy_connect/wstunnel: If both client and backend sides get readable
     at the same time, don't lose errors occuring while forwarding on the first
     side when none occurs next on the other side, and abort.  [Yann Ylavic]
+0 −6
Changes for STATUS: 0 added lines, 6 removed lines.
Original line number Diff line number Diff line
@@ -105,12 +105,6 @@ RELEASE SHOWSTOPPERS:
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
  [ start all new proposals below, under PATCHES PROPOSED. ]

  *) http: Make ap_die() robust against any HTTP error code and not modify
     response status (finally logged) when nothing is to be done. PR 56035.
     trunk patch: http://svn.apache.org/r1657881
                  http://svn.apache.org/r1665643
     2.4.x patch: trunk works (modulo CHANGES)
     +1: ylavic, minfrin, jim


PATCHES PROPOSED TO BACKPORT FROM TRUNK:
+30 −40
Changes for modules/http/http_request.c: 30 added lines, 40 removed lines.
Original line number Diff line number Diff line
@@ -73,19 +73,22 @@ static void update_r_in_filters(ap_filter_t *f,
    }
}

AP_DECLARE(void) ap_die(int type, request_rec *r)
static void ap_die_r(int type, request_rec *r, int recursive_error)
{
    int error_index = ap_index_of_response(type);
    char *custom_response = ap_response_code_string(r, error_index);
    int recursive_error = 0;
    char *custom_response;
    request_rec *r_1st_err = r;

    if (type == AP_FILTER_ERROR) {
    if (type == OK || type == DONE) {
        ap_finalize_request_protocol(r);
        return;
    }

    if (!ap_is_HTTP_VALID_RESPONSE(type)) {
        ap_filter_t *next;

        /*
         * Check if we still have the ap_http_header_filter in place. If
         * this is the case we should not ignore AP_FILTER_ERROR here because
         * this is the case we should not ignore the error here because
         * it means that we have not sent any response at all and never
         * will. This is bad. Sent an internal server error instead.
         */
@@ -99,19 +102,20 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)
         * next->frec == ap_http_header_filter
         */
        if (next) {
            if (type != AP_FILTER_ERROR) {
                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01579)
                          "Custom error page caused AP_FILTER_ERROR");
            type = HTTP_INTERNAL_SERVER_ERROR;
                              "Invalid response status %i", type);
            }
            else {
            return;
                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02831)
                              "Response from AP_FILTER_ERROR");
            }
            type = HTTP_INTERNAL_SERVER_ERROR;
        }

    if (type == DONE) {
        ap_finalize_request_protocol(r);
        else {
            return;
        }
    }

    /*
     * The following takes care of Apache redirects to custom response URLs
@@ -119,9 +123,7 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)
     * error condition, we just report on the original error, and give up on
     * any attempt to handle the other thing "intelligently"...
     */
    if (r->status != HTTP_OK) {
        recursive_error = type;

    if (recursive_error != HTTP_OK) {
        while (r_1st_err->prev && (r_1st_err->prev->status != HTTP_OK))
            r_1st_err = r_1st_err->prev;  /* Get back to original error */

@@ -141,6 +143,11 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)

        custom_response = NULL; /* Do NOT retry the custom thing! */
    }
    else {
        int error_index = ap_index_of_response(type);
        custom_response = ap_response_code_string(r, error_index);
        recursive_error = 0;
    }

    r->status = type;

@@ -216,6 +223,11 @@ AP_DECLARE(void) ap_die(int type, request_rec *r)
    ap_send_error_response(r_1st_err, recursive_error);
}

AP_DECLARE(void) ap_die(int type, request_rec *r)
{
    ap_die_r(type, r, r->status);
}

static void check_pipeline(conn_rec *c, apr_bucket_brigade *bb)
{
    if (c->keepalive != AP_CONN_CLOSE && !c->aborted) {
@@ -346,18 +358,7 @@ void ap_process_async_request(request_rec *r)
    apr_thread_mutex_unlock(r->invoke_mtx);
#endif

    if (access_status == DONE) {
        /* e.g., something not in storage like TRACE */
        access_status = OK;
    }

    if (access_status == OK) {
        ap_finalize_request_protocol(r);
    }
    else {
        r->status = HTTP_OK;
        ap_die(access_status, r);
    }
    ap_die_r(access_status, r, HTTP_OK);

    ap_process_request_after_handler(r);
}
@@ -640,8 +641,8 @@ AP_DECLARE(void) ap_internal_fast_redirect(request_rec *rr, request_rec *r)

AP_DECLARE(void) ap_internal_redirect(const char *new_uri, request_rec *r)
{
    request_rec *new = internal_internal_redirect(new_uri, r);
    int access_status;
    request_rec *new = internal_internal_redirect(new_uri, r);

    AP_INTERNAL_REDIRECT(r->uri, new_uri);

@@ -657,13 +658,8 @@ AP_DECLARE(void) ap_internal_redirect(const char *new_uri, request_rec *r)
            access_status = ap_invoke_handler(new);
        }
    }
    if (access_status == OK) {
        ap_finalize_request_protocol(new);
    }
    else {
    ap_die(access_status, new);
}
}

/* This function is designed for things like actions or CGI scripts, when
 * using AddHandler, and you want to preserve the content type across
@@ -683,16 +679,10 @@ AP_DECLARE(void) ap_internal_redirect_handler(const char *new_uri, request_rec *
        ap_set_content_type(new, r->content_type);
    access_status = ap_process_request_internal(new);
    if (access_status == OK) {
        if ((access_status = ap_invoke_handler(new)) != 0) {
            ap_die(access_status, new);
            return;
        }
        ap_finalize_request_protocol(new);
        access_status = ap_invoke_handler(new);
    }
    else {
    ap_die(access_status, new);
}
}

AP_DECLARE(void) ap_allow_methods(request_rec *r, int reset, ...)
{