You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Francis Daly <de...@daoine.org> on 2003/01/25 22:23:04 UTC

[patch] IndexResults

Hi there,

updating some old patches to the recent release...

This one is "IndexResults", the real intention of which is to allow
files requiring authentication appear in autoindex'ed directory
listings.

If the docs patch below doesn't properly describe what it does, then I
need to rewrite that too.

Built and tested against the 2.0.44 codebase.

Part 2, with some optional extras, follows.

All the best,

	f
-- 
Francis Daly        deva@daoine.org

--- modules/generators/mod_autoindex.c	2003-01-06 15:24:29.000000000 +0000
+++ modules/generators/mod_autoindex.c	2003-01-25 17:54:28.000000000 +0000
@@ -155,6 +155,12 @@
     int wildcards;
 } ai_desc_t;
 
+/* res_info is used when merging res_list */
+typedef struct res_info {
+    int *range;
+    apr_table_t *tab;
+} res_info;
+
 typedef struct autoindex_config_struct {
 
     char *default_icon;
@@ -176,6 +182,7 @@
     apr_array_header_t *ign_list;
     apr_array_header_t *hdr_list;
     apr_array_header_t *rdme_list;
+    apr_table_t *res_list;
 
 } autoindex_config_rec;
 
@@ -321,6 +328,45 @@
     return NULL;
 }
 
+static const char *set_results(cmd_parms *cmd, void *d, const char *name)
+{
+    char entry[4];
+    int showp = 1;
+
+    if (name[0] == '-') {
+        showp = 0;
+        name++;
+    }
+
+    /* verify that the form is valid */
+    if (name[0] == '\0' || name[1] == '\0' || name[2] == '\0' ||
+            name[3] != '\0') {
+        return "Value (after leading minus) must be three characters long";
+    }
+
+    /* verify that the value is valid */
+    if ((name[0] < '1' || name[0] > '9')
+        || !((isdigit(name[1]) && isdigit(name[2]))
+            || (name[1] == 'x' && name[2] == 'x'))) {
+        return "Value must be [-]### or [-]#xx, where # is a digit";
+    }
+
+    strcpy(entry, name);
+    if (name[1] == 'x') {
+        entry[1] = '\0';
+    }
+
+    /* The "value" here is "a string beginning with 1" (for show) or
+     * "a string not beginning with 1" (for hide), as per show_result() */
+    if (showp) {
+        apr_table_set(((autoindex_config_rec *) d)->res_list, entry, "1");
+    } else {
+        apr_table_set(((autoindex_config_rec *) d)->res_list, entry, "0");
+    }
+
+    return NULL;
+}
+
 static const char *add_ignore(cmd_parms *cmd, void *d, const char *ext)
 {
     push_item(((autoindex_config_rec *) d)->ign_list, 0, ext, cmd->path, NULL);
@@ -582,6 +628,8 @@
                     "one or more file extensions"),
     AP_INIT_ITERATE2("AddDescription", add_desc, BY_PATH, DIR_CMD_PERMS,
                      "Descriptive text followed by one or more filenames"),
+    AP_INIT_ITERATE("IndexResults", set_results, NULL, DIR_CMD_PERMS,
+            "one or more http status codes"),
     AP_INIT_TAKE1("HeaderName", add_header, NULL, DIR_CMD_PERMS,
                   "a filename"),
     AP_INIT_TAKE1("ReadmeName", add_readme, NULL, DIR_CMD_PERMS,
@@ -594,7 +642,7 @@
     {NULL}
 };
 
-static void *create_autoindex_config(apr_pool_t *p, char *dummy)
+static void *create_autoindex_config(apr_pool_t *p, char *dir)
 {
     autoindex_config_rec *new =
     (autoindex_config_rec *) apr_pcalloc(p, sizeof(autoindex_config_rec));
@@ -611,15 +659,47 @@
     new->ign_list = apr_array_make(p, 4, sizeof(struct item));
     new->hdr_list = apr_array_make(p, 4, sizeof(struct item));
     new->rdme_list = apr_array_make(p, 4, sizeof(struct item));
+    new->res_list = apr_table_make(p, 4);
     new->opts = 0;
     new->incremented_opts = 0;
     new->decremented_opts = 0;
     new->default_keyid = '\0';
     new->default_direction = '\0';
 
+    /* include, effectively, a global "IndexResults 3xx" */
+    if (dir == NULL) {
+        apr_table_set(new->res_list, "3", "1");
+    }
+
     return (void *) new;
 }
 
+static int res_list_ranges(int *range, const char *key, const char *dummy)
+{
+    int ikey;
+
+    ikey = atoi(key);
+    if (ikey < 10) {
+        range[ikey] = 1; 
+        range[0] = 1;
+    }
+    return 1;
+}
+
+static int res_list_del_entries(res_info *info, const char *key, 
+                                const char *dummy)
+{
+    int ikey;
+
+    ikey = atoi(key);
+    if (ikey >= 100) {
+        if ((*info).range[ikey/100] == 1) {
+            apr_table_unset((*info).tab, key);
+        } 
+    }
+    return 1;
+}
+
 static void *merge_autoindex_configs(apr_pool_t *p, void *basev, void *addv)
 {
     autoindex_config_rec *new;
@@ -638,6 +718,37 @@
     new->desc_list = apr_array_append(p, add->desc_list, base->desc_list);
     new->icon_list = apr_array_append(p, add->icon_list, base->icon_list);
     new->rdme_list = apr_array_append(p, add->rdme_list, base->rdme_list);
+    if (apr_is_empty_table(add->res_list)) {
+        /* Nothing new here */
+        new->res_list = base->res_list;
+    } else {
+        /*
+         * For each new range, unset any old keys within the range.
+         * Then copy in all the new keys.
+         */
+        int range[10] = {0};
+        res_info info;
+
+        new->res_list = apr_table_copy(p, base->res_list);
+
+        /* This sets range[R] to 1 if the range Rxx is to be merged 
+         * and range[0] to 1 if any others are set
+         */
+        apr_table_do((int (*) (void *, const char *, const char *))
+                     res_list_ranges, &range, add->res_list, NULL);
+
+        info.range = range;
+        info.tab = new->res_list;
+        /* This deletes each new{Rnn} if the range R is being merged */
+        if (range[0] == 1) {
+            apr_table_do((int (*) (void *, const char *, const char *))
+                         res_list_del_entries, &info, new->res_list, NULL);
+        }
+
+        /* This replaces-or-adds from add-> to new-> */
+        apr_table_overlap(new->res_list, add->res_list, APR_OVERLAP_TABLES_SET);
+    }
+
     if (add->opts & NO_OPTIONS) {
         /*
          * If the current directory says 'no options' then we also
@@ -710,6 +821,7 @@
                                             : base->default_keyid;
     new->default_direction = add->default_direction ? add->default_direction 
                                                     : base->default_direction;
+
     return new;
 }
 
@@ -799,6 +911,24 @@
 #define find_default_icon(d,n) find_default_item(n, d->icon_list)
 #define find_default_alt(d,n) find_default_item(n, d->alt_list)
 
+static int show_result(autoindex_config_rec *d, int status)
+{
+    char code[4];    /* HTTP codes are 3 digits long */
+    const char *res;
+
+    /* Check for exact match */
+    sprintf(code, "%d", status);
+    if ((res = apr_table_get(d->res_list, code)) != NULL) {
+        return res[0] == '1';
+    }
+    /* Check for within-range match */
+    code[1] = '\0'; 
+    if ((res = apr_table_get(d->res_list, code)) != NULL) {
+        return res[0] == '1';
+    }
+    return 0;
+}
+
 /*
  * Look through the list of pattern/description pairs and return the first one
  * if any) that matches the filename in the request.  If multiple patterns
@@ -1320,7 +1450,7 @@
 
     if ((rr->finfo.filetype != APR_DIR && rr->finfo.filetype != APR_REG)
         || !(rr->status == OK || ap_is_HTTP_SUCCESS(rr->status)
-                              || ap_is_HTTP_REDIRECT(rr->status))) {
+                              || show_result(d, rr->status))) {
         ap_destroy_sub_req(rr);
         return (NULL);
     }
--- docs/manual/mod/mod_autoindex.xml	2003-01-06 16:32:15.000000000 +0000
+++ docs/manual/mod/mod_autoindex.xml	2003-01-25 17:45:21.000000000 +0000
@@ -854,6 +854,41 @@
 </directivesynopsis>
 
 <directivesynopsis>
+<name>IndexResults</name>
+<description>Changes the list of files to show in a directory index,
+based on the HTTP status</description>
+<syntax>IndexResults <var>[-]code</var> [<var>[-]code</var>] ...</syntax>
+<default>IndexResults 3xx</default>
+<contextlist><context>server config</context><context>virtual host</context>
+<context>directory</context><context>.htaccess</context>
+</contextlist>
+<override>Indexes</override>
+<compatibility>Available in version 2.0.45 and later</compatibility>
+
+<usage>
+    <p>The <directive>IndexResults</directive> directive changes the
+    list of files to show in a directory index. <var>code</var>
+    is a HTTP status (from 100 to 999)(like <code>401</code>, for
+    "unauthorized"), or a range (from 1xx to 9xx)(like <code>4xx</code>,
+    meaning "all 400-series statuses") for additional files to show.
+    Prefix <var>code</var> with a minus (-) to explicitly hide that
+    status or range. Multiple IndexResults directives are processed
+    by first modifying the ranges (in order), and then the individual
+    statuses (in order). The <code>2xx</code> ("successful") range is
+    always present. By default, the list contains the <code>3xx</code>
+    range ("redirection"), but that can be explicitly overridden by a
+    <code>-3xx</code> if that is really wanted.</p>
+
+    <example><title>Show unauthorized filenames</title>
+      IndexResults 401
+    </example>
+    <example><title>Show unauthorized filenames, but no other 4xx ones</title>
+      IndexResults 401 -4xx
+    </example>
+</usage>
+</directivesynopsis>
+
+<directivesynopsis>
 <name>ReadmeName</name>
 <description>Name of the file that will be inserted at the end
 of the index listing</description>

Re: [patch] IndexResults

Posted by Chris Taylor <ch...@x-bb.org>.
Heh, FWIW, PLEASE put this one in Apache 2 guys ;)

It's one feature I really miss from 1.3, I lose track of the hidden URLs I
have sometimes, without them showing up in the indexes :)

Cheers,

Chris Taylor - chris@x-bb.org - The guy with the PS2 WebServer -
http://www.x-bb.org/chris.asc

----- Original Message -----
From: "Francis Daly" <de...@daoine.org>
To: <de...@httpd.apache.org>
Sent: Saturday, January 25, 2003 9:23 PM
Subject: [patch] IndexResults


> Hi there,
>
> updating some old patches to the recent release...
>
> This one is "IndexResults", the real intention of which is to allow
> files requiring authentication appear in autoindex'ed directory
> listings.
>
> If the docs patch below doesn't properly describe what it does, then I
> need to rewrite that too.
>
> Built and tested against the 2.0.44 codebase.
>
> Part 2, with some optional extras, follows.
>
> All the best,
>
> f
> --
> Francis Daly        deva@daoine.org
>
> --- modules/generators/mod_autoindex.c 2003-01-06 15:24:29.000000000 +0000
> +++ modules/generators/mod_autoindex.c 2003-01-25 17:54:28.000000000 +0000
> @@ -155,6 +155,12 @@
>      int wildcards;
>  } ai_desc_t;
>
> +/* res_info is used when merging res_list */
> +typedef struct res_info {
> +    int *range;
> +    apr_table_t *tab;
> +} res_info;
> +
>  typedef struct autoindex_config_struct {
>
>      char *default_icon;
> @@ -176,6 +182,7 @@
>      apr_array_header_t *ign_list;
>      apr_array_header_t *hdr_list;
>      apr_array_header_t *rdme_list;
> +    apr_table_t *res_list;
>
>  } autoindex_config_rec;
>
> @@ -321,6 +328,45 @@
>      return NULL;
>  }
>
> +static const char *set_results(cmd_parms *cmd, void *d, const char *name)
> +{
> +    char entry[4];
> +    int showp = 1;
> +
> +    if (name[0] == '-') {
> +        showp = 0;
> +        name++;
> +    }
> +
> +    /* verify that the form is valid */
> +    if (name[0] == '\0' || name[1] == '\0' || name[2] == '\0' ||
> +            name[3] != '\0') {
> +        return "Value (after leading minus) must be three characters
long";
> +    }
> +
> +    /* verify that the value is valid */
> +    if ((name[0] < '1' || name[0] > '9')
> +        || !((isdigit(name[1]) && isdigit(name[2]))
> +            || (name[1] == 'x' && name[2] == 'x'))) {
> +        return "Value must be [-]### or [-]#xx, where # is a digit";
> +    }
> +
> +    strcpy(entry, name);
> +    if (name[1] == 'x') {
> +        entry[1] = '\0';
> +    }
> +
> +    /* The "value" here is "a string beginning with 1" (for show) or
> +     * "a string not beginning with 1" (for hide), as per show_result()
*/
> +    if (showp) {
> +        apr_table_set(((autoindex_config_rec *) d)->res_list, entry,
"1");
> +    } else {
> +        apr_table_set(((autoindex_config_rec *) d)->res_list, entry,
"0");
> +    }
> +
> +    return NULL;
> +}
> +
>  static const char *add_ignore(cmd_parms *cmd, void *d, const char *ext)
>  {
>      push_item(((autoindex_config_rec *) d)->ign_list, 0, ext, cmd->path,
NULL);
> @@ -582,6 +628,8 @@
>                      "one or more file extensions"),
>      AP_INIT_ITERATE2("AddDescription", add_desc, BY_PATH, DIR_CMD_PERMS,
>                       "Descriptive text followed by one or more
filenames"),
> +    AP_INIT_ITERATE("IndexResults", set_results, NULL, DIR_CMD_PERMS,
> +            "one or more http status codes"),
>      AP_INIT_TAKE1("HeaderName", add_header, NULL, DIR_CMD_PERMS,
>                    "a filename"),
>      AP_INIT_TAKE1("ReadmeName", add_readme, NULL, DIR_CMD_PERMS,
> @@ -594,7 +642,7 @@
>      {NULL}
>  };
>
> -static void *create_autoindex_config(apr_pool_t *p, char *dummy)
> +static void *create_autoindex_config(apr_pool_t *p, char *dir)
>  {
>      autoindex_config_rec *new =
>      (autoindex_config_rec *) apr_pcalloc(p,
sizeof(autoindex_config_rec));
> @@ -611,15 +659,47 @@
>      new->ign_list = apr_array_make(p, 4, sizeof(struct item));
>      new->hdr_list = apr_array_make(p, 4, sizeof(struct item));
>      new->rdme_list = apr_array_make(p, 4, sizeof(struct item));
> +    new->res_list = apr_table_make(p, 4);
>      new->opts = 0;
>      new->incremented_opts = 0;
>      new->decremented_opts = 0;
>      new->default_keyid = '\0';
>      new->default_direction = '\0';
>
> +    /* include, effectively, a global "IndexResults 3xx" */
> +    if (dir == NULL) {
> +        apr_table_set(new->res_list, "3", "1");
> +    }
> +
>      return (void *) new;
>  }
>
> +static int res_list_ranges(int *range, const char *key, const char
*dummy)
> +{
> +    int ikey;
> +
> +    ikey = atoi(key);
> +    if (ikey < 10) {
> +        range[ikey] = 1;
> +        range[0] = 1;
> +    }
> +    return 1;
> +}
> +
> +static int res_list_del_entries(res_info *info, const char *key,
> +                                const char *dummy)
> +{
> +    int ikey;
> +
> +    ikey = atoi(key);
> +    if (ikey >= 100) {
> +        if ((*info).range[ikey/100] == 1) {
> +            apr_table_unset((*info).tab, key);
> +        }
> +    }
> +    return 1;
> +}
> +
>  static void *merge_autoindex_configs(apr_pool_t *p, void *basev, void
*addv)
>  {
>      autoindex_config_rec *new;
> @@ -638,6 +718,37 @@
>      new->desc_list = apr_array_append(p, add->desc_list,
base->desc_list);
>      new->icon_list = apr_array_append(p, add->icon_list,
base->icon_list);
>      new->rdme_list = apr_array_append(p, add->rdme_list,
base->rdme_list);
> +    if (apr_is_empty_table(add->res_list)) {
> +        /* Nothing new here */
> +        new->res_list = base->res_list;
> +    } else {
> +        /*
> +         * For each new range, unset any old keys within the range.
> +         * Then copy in all the new keys.
> +         */
> +        int range[10] = {0};
> +        res_info info;
> +
> +        new->res_list = apr_table_copy(p, base->res_list);
> +
> +        /* This sets range[R] to 1 if the range Rxx is to be merged
> +         * and range[0] to 1 if any others are set
> +         */
> +        apr_table_do((int (*) (void *, const char *, const char *))
> +                     res_list_ranges, &range, add->res_list, NULL);
> +
> +        info.range = range;
> +        info.tab = new->res_list;
> +        /* This deletes each new{Rnn} if the range R is being merged */
> +        if (range[0] == 1) {
> +            apr_table_do((int (*) (void *, const char *, const char *))
> +                         res_list_del_entries, &info, new->res_list,
NULL);
> +        }
> +
> +        /* This replaces-or-adds from add-> to new-> */
> +        apr_table_overlap(new->res_list, add->res_list,
APR_OVERLAP_TABLES_SET);
> +    }
> +
>      if (add->opts & NO_OPTIONS) {
>          /*
>           * If the current directory says 'no options' then we also
> @@ -710,6 +821,7 @@
>                                              : base->default_keyid;
>      new->default_direction = add->default_direction ?
add->default_direction
>                                                      :
base->default_direction;
> +
>      return new;
>  }
>
> @@ -799,6 +911,24 @@
>  #define find_default_icon(d,n) find_default_item(n, d->icon_list)
>  #define find_default_alt(d,n) find_default_item(n, d->alt_list)
>
> +static int show_result(autoindex_config_rec *d, int status)
> +{
> +    char code[4];    /* HTTP codes are 3 digits long */
> +    const char *res;
> +
> +    /* Check for exact match */
> +    sprintf(code, "%d", status);
> +    if ((res = apr_table_get(d->res_list, code)) != NULL) {
> +        return res[0] == '1';
> +    }
> +    /* Check for within-range match */
> +    code[1] = '\0';
> +    if ((res = apr_table_get(d->res_list, code)) != NULL) {
> +        return res[0] == '1';
> +    }
> +    return 0;
> +}
> +
>  /*
>   * Look through the list of pattern/description pairs and return the
first one
>   * if any) that matches the filename in the request.  If multiple
patterns
> @@ -1320,7 +1450,7 @@
>
>      if ((rr->finfo.filetype != APR_DIR && rr->finfo.filetype != APR_REG)
>          || !(rr->status == OK || ap_is_HTTP_SUCCESS(rr->status)
> -                              || ap_is_HTTP_REDIRECT(rr->status))) {
> +                              || show_result(d, rr->status))) {
>          ap_destroy_sub_req(rr);
>          return (NULL);
>      }
> --- docs/manual/mod/mod_autoindex.xml 2003-01-06 16:32:15.000000000 +0000
> +++ docs/manual/mod/mod_autoindex.xml 2003-01-25 17:45:21.000000000 +0000
> @@ -854,6 +854,41 @@
>  </directivesynopsis>
>
>  <directivesynopsis>
> +<name>IndexResults</name>
> +<description>Changes the list of files to show in a directory index,
> +based on the HTTP status</description>
> +<syntax>IndexResults <var>[-]code</var> [<var>[-]code</var>] ...</syntax>
> +<default>IndexResults 3xx</default>
> +<contextlist><context>server config</context><context>virtual
host</context>
> +<context>directory</context><context>.htaccess</context>
> +</contextlist>
> +<override>Indexes</override>
> +<compatibility>Available in version 2.0.45 and later</compatibility>
> +
> +<usage>
> +    <p>The <directive>IndexResults</directive> directive changes the
> +    list of files to show in a directory index. <var>code</var>
> +    is a HTTP status (from 100 to 999)(like <code>401</code>, for
> +    "unauthorized"), or a range (from 1xx to 9xx)(like <code>4xx</code>,
> +    meaning "all 400-series statuses") for additional files to show.
> +    Prefix <var>code</var> with a minus (-) to explicitly hide that
> +    status or range. Multiple IndexResults directives are processed
> +    by first modifying the ranges (in order), and then the individual
> +    statuses (in order). The <code>2xx</code> ("successful") range is
> +    always present. By default, the list contains the <code>3xx</code>
> +    range ("redirection"), but that can be explicitly overridden by a
> +    <code>-3xx</code> if that is really wanted.</p>
> +
> +    <example><title>Show unauthorized filenames</title>
> +      IndexResults 401
> +    </example>
> +    <example><title>Show unauthorized filenames, but no other 4xx
ones</title>
> +      IndexResults 401 -4xx
> +    </example>
> +</usage>
> +</directivesynopsis>
> +
> +<directivesynopsis>
>  <name>ReadmeName</name>
>  <description>Name of the file that will be inserted at the end
>  of the index listing</description>


Re: [patch] IndexResults

Posted by Francis Daly <de...@daoine.org>.
On Sat, Jan 25, 2003 at 09:23:04PM +0000, Francis Daly wrote:

Hi there,

> Part 2, with some optional extras, follows.

IndexResults: the optional extras.

(a) Allow a separate icon for unauthorized files.

(b) Hide some file details unauthorized clients don't need to see.

(c) document (a)

Built against 2.0.44 + the previous IndexResults patch.

The usual amount of presumption regarding version numbers is in the
docs patch -- that can be changed appropriately if it gets committed.

All the best,

	f
-- 
Francis Daly        deva@daoine.org

--- modules/generators/mod_autoindex.c	2003-01-25 17:54:28.000000000 +0000
+++ modules/generators/mod_autoindex.c	2003-01-25 17:57:47.000000000 +0000
@@ -1481,6 +1481,11 @@
     p->version_sort = !!(autoindex_opts & VERSION_SORT);
     p->ignore_case = !!(autoindex_opts & IGNORE_CASE);
 
+    /* Change to the "UNAUTHORIZED" icon, if appropriate */
+    if (rr->status == HTTP_UNAUTHORIZED) {
+        rr->filename = "^^UNAUTHORIZED^^";
+    }
+
     if (autoindex_opts & (FANCY_INDEXING | TABLE_INDEXING)) {
         p->lm = rr->finfo.mtime;
         if (dirent->filetype == APR_DIR) {
--- modules/generators/mod_autoindex.c	2003-01-25 17:57:47.000000000 +0000
+++ modules/generators/mod_autoindex.c	2003-01-25 18:00:07.000000000 +0000
@@ -1481,9 +1481,12 @@
     p->version_sort = !!(autoindex_opts & VERSION_SORT);
     p->ignore_case = !!(autoindex_opts & IGNORE_CASE);
 
-    /* Change to the "UNAUTHORIZED" icon, if appropriate */
+    /* Change to the "UNAUTHORIZED" icon, if appropriate.
+     * And conceal details which people don't need until they authenticate */
     if (rr->status == HTTP_UNAUTHORIZED) {
         rr->filename = "^^UNAUTHORIZED^^";
+        rr->finfo.mtime = -1;
+        rr->finfo.size = -1;
     }
 
     if (autoindex_opts & (FANCY_INDEXING | TABLE_INDEXING)) {
--- docs/manual/mod/mod_autoindex.xml	2003-01-25 17:45:21.000000000 +0000
+++ docs/manual/mod/mod_autoindex.xml	2003-01-25 18:01:59.000000000 +0000
@@ -309,7 +309,9 @@
 
     <p><var>Name</var> is either <code>^^DIRECTORY^^</code> for directories,
     <code>^^BLANKICON^^</code> for blank lines (to format the list
-    correctly), a file extension, a wildcard expression, a partial
+    correctly), <code>^^UNAUTHORIZED^^</code> for anything for which
+    the user has not provided appropriate credentials (available from
+    2.0.45), a file extension, a wildcard expression, a partial
     filename or a complete filename.</p>
 
     <example><title>Examples</title>

Re: [patch] IndexResults

Posted by Francis Daly <de...@daoine.org>.
On Mon, Jan 27, 2003 at 09:37:23AM +0000, Francis Daly wrote:
> On Sat, Jan 25, 2003 at 02:06:26PM -0800, Justin Erenkrantz wrote:

Newer version of the patch included below. Built and tested against
2.0.44, it applies and compiles (and works) against current HEAD too.

> But now that I actually write it down, it looks like it may not be much
> of a problem after all -- a better data structure makes a big difference

Looks like I was wrong in the first part, but not the second. I ran
into big problems trying to properly merge configurations, so tried a
simpler data structure -- 

typedef struct range_info {
    int code;
    int show;
    struct range_info *next;
} range_info;

range_info *range_allow[10];

where "code" is either the HTTP status, or a magic value meaning "this
range", and "show" is either "show" or "hide". 

> > But, we ought to keep the comparisons as integers.  This would be 
> > reasonably efficient to compare (tables are awful) and is directly 
> > proportional to the number of IndexResults directives you have.  I 
> > think it's a fair tradeoff with a much cleaner implementation.

Yup, that's there now. It turns out to be (worst case) proportional to
the number of IndexReults directives after the most recent range one
which corresponds to the range of this status.

(The code probably explains it much better than that sentence.)

> > The comparison would simply look like (pseudocode):
> > 
> > current = ranges[r->status % 100];
> > while (current) {
> >  if (r->status >= current.start && r->status <= current.end) {
> >    return 1;
> >  }
> >  current = current.next;
> > }
> > 
> > That's way more efficient than doing a sprintf.  Walk it twice for 
> > denials and approvals.

show_result() is now something like that, but only needs to be walked
once. It may have to walk the entire chain, though, since the chain isn't
ordered -- but it would be a strange config where any chain has more
than a handful of links.

The docs patch has changed too, since the order of arguments to the
directive now matters more than in the previous patch.

Any further comments are welcome,

	f
-- 
Francis Daly        deva@daoine.org

--- modules/generators/mod_autoindex.c	2003-01-06 15:24:29.000000000 +0000
+++ modules/generators/mod_autoindex.c	2003-02-06 21:57:48.000000000 +0000
@@ -155,6 +155,17 @@ typedef struct ai_desc_t {
     int wildcards;
 } ai_desc_t;
 
+/* range_info is for IndexResults */
+typedef struct range_info {
+    int code;
+    int show;
+    struct range_info *next;
+} range_info;
+
+#define AI_FULL_RANGE 99
+/* int that is never otherwise valid in range_info.code */
+
+
 typedef struct autoindex_config_struct {
 
     char *default_icon;
@@ -177,6 +188,8 @@ typedef struct autoindex_config_struct {
     apr_array_header_t *hdr_list;
     apr_array_header_t *rdme_list;
 
+    range_info *range_allow[10];
+
 } autoindex_config_rec;
 
 static char c_by_encoding, c_by_type, c_by_path;
@@ -321,6 +334,85 @@ static const char *add_desc(cmd_parms *c
     return NULL;
 }
 
+void range_allow_modify(range_info *given[], apr_pool_t *p, 
+        const int entry, const int show)
+{
+    /* 
+     * given - (pointer to) an array of 10 pointers to range_info. writeable.
+     * p     - pool out of which new range_info's can be allocated
+     * show is - 1 => show 
+     *         - 0 => hide
+     *         entry is- <10 => that full range
+     *                 - else => that single value
+     */
+
+    range_info *current;
+    range_info *prev = NULL;
+
+    if (entry < 10) {
+        /* replace current with the new full range */
+        if (!given[entry]) {
+             given[entry] = apr_palloc(p, sizeof(range_info));
+        }
+        given[entry]->code = AI_FULL_RANGE;
+        given[entry]->show = show;
+        given[entry]->next = NULL;
+        return;
+    }
+
+    current = given[entry/100];
+    /* move to the end of the chain; stop if this entry is already present */
+    while (current) {
+        if (entry == current->code) {
+            current->show = show;
+            return;
+        }
+        prev = current;
+        current = current->next;
+    }
+
+    current = apr_palloc(p, sizeof(range_info));
+    if (prev) {
+        prev->next = current;
+    } else {
+        given[entry/100] = current;
+    }
+
+    current->code = entry;
+    current->show = show;
+    current->next = NULL;
+    return;
+}
+
+static const char *set_results(cmd_parms *cmd, void *d, const char *name)
+{
+    autoindex_config_rec *dr = d;
+    int entry;
+    int show = 1;
+    if (name[0] == '-') {
+        show = 0;
+        name++;
+    }
+
+    /* verify that the form is valid */
+    if (name[0] == '\0' || name[1] == '\0' || name[2] == '\0' ||
+            name[3] != '\0') {
+        return "Value (after leading minus) must be three characters long";
+    }
+
+    /* verify that the value is valid */
+    if ((name[0] < '1' || name[0] > '9')
+            || !((isdigit(name[1]) && isdigit(name[2]))
+                || (name[1] == 'x' && name[2] == 'x'))) {
+        return "Value must be [-]### or [-]#xx, where # is a digit";
+    }
+
+    entry = atoi(name);
+    range_allow_modify(dr->range_allow, cmd->pool, entry, show);
+    return NULL;
+}
+
+
 static const char *add_ignore(cmd_parms *cmd, void *d, const char *ext)
 {
     push_item(((autoindex_config_rec *) d)->ign_list, 0, ext, cmd->path, NULL);
@@ -582,6 +674,8 @@ static const command_rec autoindex_cmds[
                     "one or more file extensions"),
     AP_INIT_ITERATE2("AddDescription", add_desc, BY_PATH, DIR_CMD_PERMS,
                      "Descriptive text followed by one or more filenames"),
+    AP_INIT_ITERATE("IndexResults", set_results, NULL, DIR_CMD_PERMS,
+                    "one or more http status codes"),
     AP_INIT_TAKE1("HeaderName", add_header, NULL, DIR_CMD_PERMS,
                   "a filename"),
     AP_INIT_TAKE1("ReadmeName", add_readme, NULL, DIR_CMD_PERMS,
@@ -594,7 +688,7 @@ static const command_rec autoindex_cmds[
     {NULL}
 };
 
-static void *create_autoindex_config(apr_pool_t *p, char *dummy)
+static void *create_autoindex_config(apr_pool_t *p, char *dir)
 {
     autoindex_config_rec *new =
     (autoindex_config_rec *) apr_pcalloc(p, sizeof(autoindex_config_rec));
@@ -617,6 +711,15 @@ static void *create_autoindex_config(apr
     new->default_keyid = '\0';
     new->default_direction = '\0';
 
+    /* new->range_allow is already initialised as 10 NULL pointers */
+    /* include, effectively, a global "IndexResults 3xx" */
+    if (dir == NULL) {
+        new->range_allow[3] = apr_palloc(p, sizeof(range_info));
+        new->range_allow[3]->code = AI_FULL_RANGE;
+        new->range_allow[3]->show = 1;
+        new->range_allow[3]->next = NULL;
+    }
+
     return (void *) new;
 }
 
@@ -638,6 +741,50 @@ static void *merge_autoindex_configs(apr
     new->desc_list = apr_array_append(p, add->desc_list, base->desc_list);
     new->icon_list = apr_array_append(p, add->icon_list, base->icon_list);
     new->rdme_list = apr_array_append(p, add->rdme_list, base->rdme_list);
+    {
+        int i;
+        range_info *mod;
+        range_info *old;
+        range_info *copy;
+        for (i=0; i<10; i++) {
+            mod = add->range_allow[i];
+
+            /* if there is no change at all to i this time, use base */
+            if (!mod || mod->code == 0) {
+                new->range_allow[i] = base->range_allow[i];
+                continue;
+            }
+
+            /* else if the change involves a range, use add */
+            if (mod->code == AI_FULL_RANGE) {
+                new->range_allow[i] = add->range_allow[i];
+                continue;
+            }   
+
+            /* else deep-copy base, then merge the changes */
+            old = base->range_allow[i];
+            if (old && old->code) {
+                copy = apr_palloc(p, sizeof(range_info));
+                new->range_allow[i] = copy;
+                while (old && old->code) {
+                    copy->code = old->code;
+                    copy->show = old->show;
+                    old = old->next;
+                    if (old) {
+                        copy->next = apr_palloc(p, sizeof(range_info));
+                        copy = copy->next;
+                    } else {
+                        copy->next = NULL;
+                    }
+                }
+            }
+            while (mod && mod->code) {
+                range_allow_modify(new->range_allow, p, mod->code, mod->show);
+                mod = mod->next;
+            }
+        }
+    }
+
     if (add->opts & NO_OPTIONS) {
         /*
          * If the current directory says 'no options' then we also
@@ -799,6 +946,24 @@ static char *find_default_item(char *bog
 #define find_default_icon(d,n) find_default_item(n, d->icon_list)
 #define find_default_alt(d,n) find_default_item(n, d->alt_list)
 
+static int show_result(autoindex_config_rec *d, int status)
+{
+    range_info *current;
+    int show = 0;  /* default don't show */
+
+    current = d->range_allow[status / 100];
+    while (current) {
+        if (current->code == status) {
+            return current->show;
+        }
+        if (current->code == AI_FULL_RANGE) {
+            show = current->show;
+        } 
+        current = current->next;
+    }
+    return show;
+}
+
 /*
  * Look through the list of pattern/description pairs and return the first one
  * if any) that matches the filename in the request.  If multiple patterns
@@ -1320,7 +1484,7 @@ static struct ent *make_autoindex_entry(
 
     if ((rr->finfo.filetype != APR_DIR && rr->finfo.filetype != APR_REG)
         || !(rr->status == OK || ap_is_HTTP_SUCCESS(rr->status)
-                              || ap_is_HTTP_REDIRECT(rr->status))) {
+                              || show_result(d, rr->status))) {
         ap_destroy_sub_req(rr);
         return (NULL);
     }
--- docs/manual/mod/mod_autoindex.xml	2003-01-06 16:32:15.000000000 +0000
+++ docs/manual/mod/mod_autoindex.xml	2003-02-07 00:01:31.000000000 +0000
@@ -854,6 +854,41 @@ Name|Date|Size|Description</syntax>
 </directivesynopsis>
 
 <directivesynopsis>
+<name>IndexResults</name>
+<description>Changes the list of files to show in a directory index,
+based on the HTTP status</description>
+<syntax>IndexResults <var>[-]code</var> [<var>[-]code</var>] ...</syntax>
+<default>IndexResults 3xx</default>
+<contextlist><context>server config</context><context>virtual host</context>
+<context>directory</context><context>.htaccess</context>
+</contextlist>
+<override>Indexes</override>
+<compatibility>Available in version 2.0.45 and later</compatibility>
+
+<usage>
+    <p>The <directive>IndexResults</directive> directive changes the
+    list of files to show in a directory index. <var>code</var>
+    is a HTTP status (from 100 to 999)(like <code>401</code>, for
+    "unauthorized"), or a range (from 1xx to 9xx)(like <code>4xx</code>,
+    meaning "all 400-series statuses") for additional files to list.
+    Prefix <var>code</var> with a minus (-) to explicitly hide that
+    status or range. Multiple IndexResults arguments or directives are 
+    processed in order, which means a range always overrides an
+    earlier status. The <code>2xx</code> ("successful") range is
+    always present. By default, the list contains the <code>3xx</code>
+    range ("redirection"), but that can be explicitly overridden by a
+    <code>-3xx</code> if that is really wanted.</p>
+
+    <example><title>Show unauthorized filenames</title>
+      IndexResults 401
+    </example>
+    <example><title>Show unauthorized filenames, but no other 4xx ones</title>
+      IndexResults -4xx 401
+    </example>
+</usage>
+</directivesynopsis>
+
+<directivesynopsis>
 <name>ReadmeName</name>
 <description>Name of the file that will be inserted at the end
 of the index listing</description>

Re: [patch] IndexResults

Posted by Francis Daly <de...@daoine.org>.
On Sat, Jan 25, 2003 at 02:06:26PM -0800, Justin Erenkrantz wrote:
> --On Saturday, January 25, 2003 9:23 PM +0000 Francis Daly 
> <de...@daoine.org> wrote:

Hi there,

thanks for your input.

> >+/* res_info is used when merging res_list */
> >+typedef struct res_info {
> >+    int *range;
> >+    apr_table_t *tab;
> >+} res_info;
> 
> I think this is the wrong data structure to be using here.  A table 
> doesn't make a whole lot of sense.  It's primarily used to deal with 
> strings not integers.  The 'native' status and true/false conditions 
> are best done with an integers for keys.  

I agree that integers would be much better to use.  With my first few
tries at an implementation based on them, though, I couldn't come up
with a sensible way of processing all IndexResults directives.  Possibly
I was trying to be too clever, allowing too much flexibility in the
configuration.

I'll try to show what I mean using your data structure below -- although
the answer may be "don't allow such freedom in configuring the thing".

But now that I actually write it down, it looks like it may not be much
of a problem after all -- a better data structure makes a big difference
(as always).

> This patch does way too many unsafe things with strings for my liking.

"Unwise" I'll accept, but I thought I had been careful to avoid
"unsafe".  No matter: if it can work with integers, that's the way to go.

> You can key off the first digit (easy to get) and only have it 
> restricted to that region of the number space (create buckets).  For 
> series that cross sequences, you can split them up at 100-intervals. 

At least for the initial config intention, there shouldn't be any of
those.  Either "###" meaning "just that one", or "#xx" meaning "range
from #00 to #99".

> (I'd be content to lose one bucket so that we don't have to subtract 
> the status on the request.)  Allows and denies are separated into two 
> lists making it easy to keep the ordering (where allows are always 
> before denies).

The problem I ran into when trying to implement something along those
lines, was how to manage individual deny rules.  That's how I ended up
with separate "range" and "individual" lists.

Using your outline, merging a deny (or group of denies) will potentially
involve quite a bit of fiddling with the allow list.

Contrived example: one directory has IndexResults 4xx (or whatever the
syntax for "all 400-series" is).  A subdirectory has IndexResults -401
-406.  A subsubdirectory has IndexResults 405 406 407.

Creating the per-directory configs is easy -- the first is a single big
range, the second is two 1-element ranges, and the third is either three
1-element ranges, or possibly one 3-element range.  Merging them is where
I couldn't find a straightforward algorithm (although, with the right
data structure, it appears much easier).

The top directory here has "allow" of 400-499, and "deny" of nothing.
The middle directory has to have "deny" of 401,406, and so will
presumably have to switch "allow" to 400,402-405,407-499.  The bottom
directory will add 405,406,407 to allow (it won't have to touch deny, as
the allow list is tested first).  It may be able to be clever and notice
that it can change "allow" to 400,402-499, but that's not something
needed right now.

Basically, if "allow" is always read first, then the merging of "deny"
must remove things from "allow", adjusting the entire chain as it goes.

Now, I expect that the common case will be IndexResults 401 set
globally, (or not at all) and not modified in directories, so the
"allow" chain will be 300-399,401-401 (or 300-399) everywhere.  Given
that, the performance of a merge compared to a search may not be an
issue at all.

> What I would envision would be something like this:
> 
> /* res_info is used when merging res_list */
> typedef struct range_info {
>   int start;
>   int end;    /* start == end indicates a singleton. */
>   struct range_info *next;
> } range_info;
> 
> typedef struct range_list {
>  range_info allow[10];
>  range_info deny[10];
> } range_list;

Looks good.

The merge algorithm then becomes: for any IndexResults Nxx, we remove
all elements in allow[N], and then add allow[N] = {N00, N99, NULL}.

For any IndexResults -Nxx, we remove all elements in allow[N], and
remove all elements in deny[N], and then add deny[N] = {N00, N99, NULL}.
Actually, with that, do we even need a deny[]?  "If it's not in allow[],
deny it" might be more straightforward -- especially if allow[] will
always be checked first anyway.

Assuming there is no deny[], then for any IndexResults N##, check if
it's in the allow[N] chain, and if not, add it (by incrementing .end
or decrementing .start of the appropriate element, or adding a new
singleton element, and adjusting .next pointers accordingly).  Or, just
add it to the end of the chain, and leave any future [-]Nxx directive to
remove it when the time comes.

And for any IndexResults -N##, check each element of the allow[N] chain.
If the N## case is "add to the end of the chain", then for each element
of allow[N], split any ranges that include N## into two (by creating a
new link, and adjusting a .next pointer), and remove any singletons N##,
also adjusting a .next pointer in the previous link.  Alternatively, if
the N## case was the more-work "keep the chain ordered with no overlap",
then the -N## case becomes "find the link that should include it, and
split or remove that one if it's there".

It may come down to a guess as to whether N## or -N## is more likely,
but I suspect that keeping the chain ordered without overlapping entries
will be tidier.  Especially if pool-magic means that we can just create
new links without having to worry about correctly de-allocating older
ones which are no longer in the chain.

(Of course, because the chain involves following pointers, any "merge"
operation will have to generate an entirely new chain, at least for
each affected allow[N].  Shouldn't be too much trouble, especially as it
shouldn't be the "usual" code path.)

> But, we ought to keep the comparisons as integers.  This would be 
> reasonably efficient to compare (tables are awful) and is directly 
> proportional to the number of IndexResults directives you have.  I 
> think it's a fair tradeoff with a much cleaner implementation.

I agree.  I used strings because, basically, I couldn't come up with a
better data structure.  I guess I should have tried harder, or asked
sooner.

> The comparison would simply look like (pseudocode):
> 
> current = ranges[r->status % 100];
> while (current) {
>  if (r->status >= current.start && r->status <= current.end) {
>    return 1;
>  }
>  current = current.next;
> }
> 
> That's way more efficient than doing a sprintf.  Walk it twice for 
> denials and approvals.
> 
> What do you think?  -- justin

I like, although I think a single ordered allow list should be
sufficient, both in terms of "possible configs" and "probable configs".
(Hopefully I've properly understood your suggestion.)

Can you see any glaring holes in the implementation plan outlined above?

If not, I'll start writing.

Thanks,

	f
-- 
Francis Daly        deva@daoine.org

Re: [patch] IndexResults

Posted by Justin Erenkrantz <je...@apache.org>.
--On Saturday, January 25, 2003 9:23 PM +0000 Francis Daly 
<de...@daoine.org> wrote:

> +/* res_info is used when merging res_list */
> +typedef struct res_info {
> +    int *range;
> +    apr_table_t *tab;
> +} res_info;

I think this is the wrong data structure to be using here.  A table 
doesn't make a whole lot of sense.  It's primarily used to deal with 
strings not integers.  The 'native' status and true/false conditions 
are best done with an integers for keys.  This patch does way too 
many unsafe things with strings for my liking.

You can key off the first digit (easy to get) and only have it 
restricted to that region of the number space (create buckets).  For 
series that cross sequences, you can split them up at 100-intervals. 
(I'd be content to lose one bucket so that we don't have to subtract 
the status on the request.)  Allows and denies are separated into two 
lists making it easy to keep the ordering (where allows are always 
before denies).

What I would envision would be something like this:

/* res_info is used when merging res_list */
typedef struct range_info {
   int start;
   int end;    /* start == end indicates a singleton. */
   struct range_info *next;
} range_info;

typedef struct range_list {
  range_info allow[10];
  range_info deny[10];
} range_list;

But, we ought to keep the comparisons as integers.  This would be 
reasonably efficient to compare (tables are awful) and is directly 
proportional to the number of IndexResults directives you have.  I 
think it's a fair tradeoff with a much cleaner implementation.

The comparison would simply look like (pseudocode):

current = ranges[r->status % 100];
while (current) {
  if (r->status >= current.start && r->status <= current.end) {
    return 1;
  }
  current = current.next;
}

That's way more efficient than doing a sprintf.  Walk it twice for 
denials and approvals.

What do you think?  -- justin