Rework of the recursion stopper - collapse recursion counters into one function

Reviewed by: Justin Erenkrantz

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@100095 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/include/http_core.h b/include/http_core.h
index b4d8068bbfd678369b172e801d2b2ab25c956234..d15b2d9a06e259cfde62b65fbbf51fe78e2d73ba 100644 (file)
--- a/include/http_core.h
+++ b/include/http_core.h
@@ -264,20 +264,12 @@ AP_DECLARE(size_t) ap_get_limit_xml_body(const request_rec *r);
 AP_DECLARE(void) ap_custom_response(request_rec *r, int status, const char *string);
 
 /**
- * Check if the current request is beyond the configured max. number of redirects
+ * Check if the current request is beyond the configured max. number of redirects or subrequests
  * @param r The current request
  * @return true (is exceeded) or false
- * @deffunc int ap_is_redirect_limit_exceeded(const request_rec *r)
+ * @deffunc int ap_is_recursion_limit_exceeded(const request_rec *r)
  */
-AP_DECLARE(int) ap_is_redirect_limit_exceeded(const request_rec *r);
-
-/**
- * Check if the current request is beyond the configured subreq nesting level
- * @param r The current request
- * @return true (is exceeded) or false
- * @deffunc int ap_is_subreq_limit_exceeded(const request_rec *r)
- */
-AP_DECLARE(int) ap_is_subreq_limit_exceeded(const request_rec *r);
+AP_DECLARE(int) ap_is_recursion_limit_exceeded(const request_rec *r);
 
 /**
  * Check for a definition from the server command line

diff --git a/modules/http/http_request.c b/modules/http/http_request.c
index 8f8472ce9c11d34011267babdab78d8fafa3de14..ae01c260dea16df0bb98e4429c845f39e643534d 100644 (file)
--- a/modules/http/http_request.c
+++ b/modules/http/http_request.c
@@ -334,7 +334,7 @@ static request_rec *internal_internal_redirect(const char *new_uri,
     int access_status;
     request_rec *new;
 
-    if (ap_is_redirect_limit_exceeded(r)) {
+    if (ap_is_recursion_limit_exceeded(r)) {
         ap_die(HTTP_INTERNAL_SERVER_ERROR, r);
         return NULL;
     }

diff --git a/server/core.c b/server/core.c
index 0c26d129d7dd99d2f2e1cdf4705945992d4e8726..a813babd797fefc3220d1507ef3e1c060144d2b8 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -2661,102 +2661,90 @@ static const char *set_recursion_limit(cmd_parms *cmd, void *dummy,
     return NULL;
 }
 
-static void log_backtrace(const request_rec *top, const request_rec *r)
+static void log_backtrace(const request_rec *r)
 {
-    while (top) {
-        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
-                      "redirected from r->uri = %s",
-                      top->uri ? top->uri : "(unexpectedly NULL)");
+    const request_rec *top = r;
+
+    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+                  "r->uri = %s", r->uri ? r->uri : "(unexpectedly NULL)");
+
+    while (top && (top->prev || top->main)) {
+        if (top->prev) {
+            top = top->prev;
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+                          "redirected from r->uri = %s",
+                          top->uri ? top->uri : "(unexpectedly NULL)");
+        }
 
         if (!top->prev && top->main) {
             top = top->main;
-
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
                           "subrequested from r->uri = %s",
                           top->uri ? top->uri : "(unexpectedly NULL)");
         }
-        else {
-            top = top->prev;
-        }
     }
 }
 
 /*
  * check whether redirect limit is reached
  */
-AP_DECLARE(int) ap_is_redirect_limit_exceeded(const request_rec *r)
+AP_DECLARE(int) ap_is_recursion_limit_exceeded(const request_rec *r)
 {
     core_server_config *conf = ap_get_module_config(r->server->module_config,
                                                     &core_module);
     const request_rec *top = r;
-    int redirects = 0;
-    int limit = conf->redirect_limit
-                ? conf->redirect_limit
-                : AP_DEFAULT_MAX_INTERNAL_REDIRECTS;
-
-    while (top->prev) {
-        if (++redirects >= limit) {
-            /* uuh, too much. */
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                          "Request exceeded the limit of %d internal redirects "
-                          "due to probable configuration error. Use "
-                          "'LimitInternalRecursion' to increase the limit if "
-                          "necessary. Use 'LogLevel debug' to get a "
-                          "backtrace.", limit);
+    int redirects = 0, subreqs = 0;
+    int rlimit = conf->redirect_limit
+                 ? conf->redirect_limit
+                 : AP_DEFAULT_MAX_INTERNAL_REDIRECTS;
+    int slimit = conf->subreq_limit
+                 ? conf->subreq_limit
+                 : AP_DEFAULT_MAX_SUBREQ_DEPTH;
+
+
+    while (top->prev || top->main) {
+        if (top->prev) {
+            if (++redirects >= rlimit) {
+                /* uuh, too much. */
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                              "Request exceeded the limit of %d internal "
+                              "redirects due to probable configuration error. "
+                              "Use 'LimitInternalRecursion' to increase the "
+                              "limit if necessary. Use 'LogLevel debug' to get "
+                              "a backtrace.", rlimit);
 
-            /* post backtrace */
-            log_backtrace(r->prev, r);
+                /* post backtrace */
+                log_backtrace(r);
 
-            /* return failure */
-            return 1;
-        }
+                /* return failure */
+                return 1;
+            }
 
-        if (!top->prev && top->main) {
-            top = top->main;
-        }
-        else {
             top = top->prev;
         }
-    }
 
-    /* number of redirects is ok */
-    return 0;
-}
+        if (!top->prev && top->main) {
+            if (++subreqs >= slimit) {
+                /* uuh, too much. */
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                              "Request exceeded the limit of %d subrequest "
+                              "nesting levels due to probable confguration "
+                              "error. Use 'LimitInternalRecursion' to increase "
+                              "the limit if necessary. Use 'LogLevel debug' to "
+                              "get a backtrace.", slimit);
 
-/*
- * check whether subrequest depth limit is reached
- */
-AP_DECLARE(int) ap_is_subreq_limit_exceeded(const request_rec *r)
-{
-    core_server_config *conf = ap_get_module_config(r->server->module_config,
-                                                    &core_module);
-    const request_rec *top = r;
-    int subreqs = 0;
-    int limit = conf->subreq_limit
-                ? conf->subreq_limit
-                : AP_DEFAULT_MAX_SUBREQ_DEPTH;
-
-    while (top->main) {
-        if (++subreqs >= limit) {
-            /* uuh, too much. */
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                          "Request exceeded the limit of %d subrequest "
-                          "nesting levels due to probable confguration error. "
-                          "Use 'LimitInternalRecursion' to increase the limit "
-                          "if necessary. Use 'LogLevel debug' to get a "
-                          "backtrace.", limit);
+                /* post backtrace */
+                log_backtrace(r);
 
-            /* post backtrace */
-            log_backtrace(r->main, r);
+                /* return failure */
+                return 1;
+            }
 
-            /* return failure */
-            return 1;
+            top = top->main;
         }
-
-        top = top->main;
     }
 
-    /* number of subrequests is ok */
+    /* recursion state: ok */
     return 0;
 }
 

diff --git a/server/request.c b/server/request.c
index 4254c8e446382d5e06d967d5c9a5681b9194b748..5bfe188c7c1113942dc50dd728fc0fcacc04c353 100644 (file)
--- a/server/request.c
+++ b/server/request.c
@@ -1634,7 +1634,7 @@ AP_DECLARE(request_rec *) ap_sub_req_method_uri(const char *method,
 
     /* We cannot return NULL without violating the API. So just turn this
      * subrequest into a 500 to indicate the failure. */
-    if (ap_is_subreq_limit_exceeded(r)) {
+    if (ap_is_recursion_limit_exceeded(r)) {
         rnew->status = HTTP_INTERNAL_SERVER_ERROR;
         return rnew;
     }
@@ -1774,7 +1774,7 @@ AP_DECLARE(request_rec *) ap_sub_req_lookup_dirent(const apr_finfo_t *dirent,
 
     /* We cannot return NULL without violating the API. So just turn this
      * subrequest into a 500. */
-    if (ap_is_subreq_limit_exceeded(r)) {
+    if (ap_is_recursion_limit_exceeded(r)) {
         rnew->status = HTTP_INTERNAL_SERVER_ERROR;
         return rnew;
     }
@@ -1868,7 +1868,7 @@ AP_DECLARE(request_rec *) ap_sub_req_lookup_file(const char *new_file,
 
     /* We cannot return NULL without violating the API. So just turn this
      * subrequest into a 500. */
-    if (ap_is_subreq_limit_exceeded(r)) {
+    if (ap_is_recursion_limit_exceeded(r)) {
         rnew->status = HTTP_INTERNAL_SERVER_ERROR;
         return rnew;
     }