Dear List!

We are trying to use mod_autoindex on an access-restricted web server
with lots of directories. Our AuthN is costly (we are using
mod_auth_external). With ~ 700 directories, the generation of the index
takes > 1 minute.

With a little profiling we found out that authentication is done for
every subrequest, of which mod_autoindex uses plenty.

There is some logic in ap_process_request_internal() that should
optimize these out; however, it does not work for us. The comparison
(r->main->per_dir_config == r->per_dir_config) never succeeds as
ap_merge_per_dir_configs() always returns a new configuration vector.

Cf. request.c lines 145ff (inside of ap_process_request_internal()) and
466ff (ap_directory_walk()) and config.c lines 230ff
(ap_merge_per_dir_configs()).

Why is it necessary to re-authenticate within a subrequest?
When the per_dir configuration has changed, AuthZ has of course to be
rechecked. AuthN, on the other hand, could be taken from the parent
request, where it already has been verified. What are we missing?

With the attached patch (against 2.2.17), the page generation is down to
one AuthN process (ca. 2 seconds overall) on our machine.
The patch is meant for communication purposes only, to illustrate what we are talking about. It is not intended to be a ready made solution to the problem we found. To make our changes stand out, we also forwent correct indentation.

We would have liked to include a check like ``if the user is the same as
in the parent request and it already has been AuthN'd'', but we don't
know the best way to do that.

A quick test with a .htaccess file showed that Access Restrictions are
still honored within a subrequest.

Thank you for your consideration and time!

PlagiaTUM
diff --git a/server/request.c b/server/request.c
index 1801cf7..4b30db7 100644
--- a/server/request.c
+++ b/server/request.c
@@ -170,7 +170,7 @@ AP_DECLARE(int) ap_process_request_internal(request_rec *r)
      * functions in map_to_storage that use the same merge results given
      * identical input.)  If the config changes, we must re-auth.
      */
-    if (r->main && (r->main->per_dir_config == r->per_dir_config)) {
+    if (r->main) {
         r->user = r->main->user;
         r->ap_auth_type = r->main->ap_auth_type;
     }
@@ -178,7 +178,7 @@ AP_DECLARE(int) ap_process_request_internal(request_rec *r)
         r->user = r->prev->user;
         r->ap_auth_type = r->prev->ap_auth_type;
     }
-    else {
+    {
         switch (ap_satisfies(r)) {
         case SATISFY_ALL:
         case SATISFY_NOSPEC:
@@ -187,8 +187,8 @@ AP_DECLARE(int) ap_process_request_internal(request_rec *r)
             }
 
             if (ap_some_auth_required(r)) {
-                if (((access_status = ap_run_check_user_id(r)) != 0)
-                    || !ap_auth_type(r)) {
+                if (!r->user && (((access_status = ap_run_check_user_id(r)) != 0)
+                    || !ap_auth_type(r))) {
                     return decl_die(access_status, ap_auth_type(r)
                                   ? "check user.  Check your authn provider!"
                                   : "perform authentication. AuthType not set!",
@@ -211,8 +211,8 @@ AP_DECLARE(int) ap_process_request_internal(request_rec *r)
                     return decl_die(access_status, "check access", r);
                 }
 
-                if (((access_status = ap_run_check_user_id(r)) != 0)
-                    || !ap_auth_type(r)) {
+                if (!r->user && (((access_status = ap_run_check_user_id(r)) != 0)
+                    || !ap_auth_type(r))) {
                     return decl_die(access_status, ap_auth_type(r)
                                   ? "check user.  Check your authn provider!"
                                   : "perform authentication. AuthType not set!",


Reply via email to