Autoindex: use temporary pool for directory entries

Directory listing entries (file names, metadata) were allocated
from the request pool and lived until request teardown.  Use a
temporary pool instead, destroying it immediately after the
response buffer is built.

diff --git a/src/http/modules/ngx_http_autoindex_module.c b/src/http/modules/ngx_http_autoindex_module.c
index 082bcb5e622c51b984dc73439d3c93eab62069c1..8f594d170cd845a00c260a0b0eb6f0489606d458 100644 (file)
--- a/src/http/modules/ngx_http_autoindex_module.c
+++ b/src/http/modules/ngx_http_autoindex_module.c
@@ -66,8 +66,6 @@ static ngx_buf_t *ngx_http_autoindex_xml(ngx_http_request_t *r,
 
 static int ngx_libc_cdecl ngx_http_autoindex_cmp_entries(const void *one,
     const void *two);
-static ngx_int_t ngx_http_autoindex_error(ngx_http_request_t *r,
-    ngx_dir_t *dir, ngx_str_t *name);
 
 static ngx_int_t ngx_http_autoindex_init(ngx_conf_t *cf);
 static void *ngx_http_autoindex_create_loc_conf(ngx_conf_t *cf);
@@ -246,15 +244,6 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
 
 #endif
 
-    /* TODO: pool should be temporary pool */
-    pool = r->pool;
-
-    if (ngx_array_init(&entries, pool, 40, sizeof(ngx_http_autoindex_entry_t))
-        != NGX_OK)
-    {
-        return ngx_http_autoindex_error(r, &dir, &path);
-    }
-
     r->headers_out.status = NGX_HTTP_OK;
 
     switch (format) {
@@ -291,6 +280,17 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
         return rc;
     }
 
+    pool = ngx_create_pool(4096, r->connection->log);
+    if (pool == NULL) {
+        goto failed;
+    }
+
+    if (ngx_array_init(&entries, pool, 40, sizeof(ngx_http_autoindex_entry_t))
+        != NGX_OK)
+    {
+        goto failed;
+    }
+
     filename = path.data;
     filename[path.len] = '/';
 
@@ -303,7 +303,7 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
             if (err != NGX_ENOMOREFILES) {
                 ngx_log_error(NGX_LOG_CRIT, r->connection->log, err,
                               ngx_read_dir_n " \"%V\" failed", &path);
-                return ngx_http_autoindex_error(r, &dir, &path);
+                goto failed;
             }
 
             break;
@@ -328,7 +328,7 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
 
                 filename = ngx_pnalloc(pool, allocated);
                 if (filename == NULL) {
-                    return ngx_http_autoindex_error(r, &dir, &path);
+                    goto failed;
                 }
 
                 last = ngx_cpystrn(filename, path.data, path.len + 1);
@@ -348,28 +348,28 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
                         continue;
                     }
 
-                    return ngx_http_autoindex_error(r, &dir, &path);
+                    goto failed;
                 }
 
                 if (ngx_de_link_info(filename, &dir) == NGX_FILE_ERROR) {
                     ngx_log_error(NGX_LOG_CRIT, r->connection->log, ngx_errno,
                                   ngx_de_link_info_n " \"%s\" failed",
                                   filename);
-                    return ngx_http_autoindex_error(r, &dir, &path);
+                    goto failed;
                 }
             }
         }
 
         entry = ngx_array_push(&entries);
         if (entry == NULL) {
-            return ngx_http_autoindex_error(r, &dir, &path);
+            goto failed;
         }
 
         entry->name.len = len;
 
         entry->name.data = ngx_pnalloc(pool, len + 1);
         if (entry->name.data == NULL) {
-            return ngx_http_autoindex_error(r, &dir, &path);
+            goto failed;
         }
 
         ngx_cpystrn(entry->name.data, ngx_de_name(&dir), len + 1);
@@ -410,12 +410,12 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
         break;
     }
 
+    ngx_destroy_pool(pool);
+
     if (b == NULL) {
         return NGX_ERROR;
     }
 
-    /* TODO: free temporary pool */
-
     if (r == r->main) {
         b->last_buf = 1;
     }
@@ -426,6 +426,19 @@ ngx_http_autoindex_handler(ngx_http_request_t *r)
     out.next = NULL;
 
     return ngx_http_output_filter(r, &out);
+
+failed:
+
+    if (pool != NULL) {
+        ngx_destroy_pool(pool);
+    }
+
+    if (ngx_close_dir(&dir) == NGX_ERROR) {
+        ngx_log_error(NGX_LOG_ALERT, r->connection->log, ngx_errno,
+                      ngx_close_dir_n " \"%V\" failed", &path);
+    }
+
+    return NGX_ERROR;
 }
 
 
@@ -1006,17 +1019,6 @@ ngx_http_autoindex_alloc(ngx_http_autoindex_ctx_t *ctx, size_t size)
 #endif
 
 
-static ngx_int_t
-ngx_http_autoindex_error(ngx_http_request_t *r, ngx_dir_t *dir, ngx_str_t *name)
-{
-    if (ngx_close_dir(dir) == NGX_ERROR) {
-        ngx_log_error(NGX_LOG_ALERT, r->connection->log, ngx_errno,
-                      ngx_close_dir_n " \"%V\" failed", name);
-    }
-
-    return r->header_sent ? NGX_ERROR : NGX_HTTP_INTERNAL_SERVER_ERROR;
-}
-
 
 static void *
 ngx_http_autoindex_create_loc_conf(ngx_conf_t *cf)