Re: [patch] IndexResults
On Mon, Jan 27, 2003 at 09:37:23AM +, 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[EMAIL PROTECTED]
--- modules/generators/mod_autoindex.c 2003-01-06 15:24:29.0 +
+++ modules/generators/mod_autoindex.c 2003-02-06 21:57:48.0 +
@@ -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' || na
Re: [patch] IndexResults
On Sat, Jan 25, 2003 at 02:06:26PM -0800, Justin Erenkrantz wrote:
> --On Saturday, January 25, 2003 9:23 PM +0000 Francis Daly
> <[EMAIL PROTECTED]> 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 IndexResul
[patch] MultiViewsAllowUnauthorized
Hi there,
another updated old patch, although this one was previously under a
different name (RevealSecretUrl, or something similar).
In an otherwise-accessible directory, I require authentication for the
file "thing.cgi". Currently, I can't advertise that under the the url
"thing". With this patch (and a config directive set) I can. So when I
replace it with "thing.html", links don't break.
More details, as well as warnings of where it might act unexpectedly, in
the docs patch at the end.
Against 2.0.44.
All the best,
f
--
Francis Daly[EMAIL PROTECTED]
--- modules/mappers/mod_negotiation.c 2002-11-25 19:03:26.0 +
+++ modules/mappers/mod_negotiation.c 2003-01-25 19:41:52.0 +
@@ -89,6 +89,7 @@
typedef struct {
int forcelangpriority;
+int allow_unauth;
apr_array_header_t *language_priority;
} neg_dir_config;
@@ -101,6 +102,12 @@
#define FLP_DEFAULT FLP_PREFER
+/* allow_unauth flags
+ */
+#define AUA_UNDEF0/* "no explicit config" */
+#define AUA_ON 1
+#define AUA_OFF 2
+
module AP_MODULE_DECLARE_DATA negotiation_module;
static void *create_neg_dir_config(apr_pool_t *p, char *dummy)
@@ -108,6 +115,7 @@
neg_dir_config *new = (neg_dir_config *) apr_palloc(p, sizeof(neg_dir_config));
new->forcelangpriority = FLP_UNDEF;
+new->allow_unauth = AUA_UNDEF;
new->language_priority = NULL;
return new;
}
@@ -122,6 +130,9 @@
new->forcelangpriority = (add->forcelangpriority != FLP_UNDEF)
? add->forcelangpriority
: base->forcelangpriority;
+new->allow_unauth = (add->allow_unauth != AUA_UNDEF)
+ ? add->allow_unauth
+ : base->allow_unauth;
new->language_priority = add->language_priority
? add->language_priority
: base->language_priority;
@@ -142,6 +153,18 @@
return NULL;
}
+static const char *allow_unauth(cmd_parms *cmd, void *n_, int arg)
+{
+neg_dir_config *n = n_;
+const char *err = ap_check_cmd_context(cmd, NOT_IN_FILES);
+
+if (err != NULL) {
+return err;
+}
+n->allow_unauth = arg ? AUA_ON : AUA_OFF;
+return NULL;
+}
+
static const char *set_force_priority(cmd_parms *cmd, void *n_, const char *w)
{
neg_dir_config *n = n_;
@@ -188,6 +211,8 @@
{
AP_INIT_FLAG("CacheNegotiatedDocs", cache_negotiated_docs, NULL, RSRC_CONF,
"Either 'on' or 'off' (default)"),
+AP_INIT_FLAG("MultiviewsAllowUnauthorized", allow_unauth, NULL,
+RSRC_CONF|OR_AUTHCFG,
+ "Either 'on' or 'off' (default)"),
AP_INIT_ITERATE("LanguagePriority", set_language_priority, NULL, OR_FILEINFO,
"space-delimited list of MIME language abbreviations"),
AP_INIT_ITERATE("ForceLanguagePriority", set_force_priority, NULL, OR_FILEINFO,
@@ -1049,6 +1074,7 @@
struct accept_rec accept_info;
void *new_var;
int anymatch = 0;
+int found_unauth = 0;
clean_var_rec(&mime_info);
@@ -1114,6 +1140,13 @@
if (sub_req->finfo.filetype != APR_REG)
continue;
+/* Note if it failed UNAUTHORIZED. We may want to return this
+ * status, eventually
+ */
+if (sub_req->status == HTTP_UNAUTHORIZED) {
+found_unauth = 1;
+}
+
/* If it has a handler, we'll pretend it's a CGI script,
* since that's a good indication of the sort of thing it
* might be doing.
@@ -1233,9 +1266,13 @@
/* We found some file names that matched. None could be served.
* Rather than fall out to autoindex or some other mapper, this
- * request must die.
+ * request must die, unless we can offer UNAUTHORIZED.
*/
if (anymatch && !neg->avail_vars->nelts) {
+if (found_unauth && neg->conf->allow_unauth == AUA_ON) {
+ap_note_basic_auth_failure(r);
+return HTTP_UNAUTHORIZED;
+}
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"Negotiation: discovered file(s) matching request: %s"
" (None could be negotiated).",
--- docs/manual/mod/mod_negotiation.xml 2002-12-17 18:31:37.0 +
+++ docs/manual/mod/mod_negotiation.xml 2003-01-25 20:39:47.0 +
@@ -258,4 +258,36 @@
AddLanguage
+
+MultiViewsAllowUnauthorized
+Allows some authentication-requiring files be
+MultiViews-matched
+MultiViewsAllowUnauthorized On|Off
+MultiViewsAllowUnauthorized Off
+server configvirtual host
+directory.htaccess
+AuthConfig
+Available in version 2.0.45 and later
+
+
+T
Re: [patch] IndexResults
On Sat, Jan 25, 2003 at 09:23:04PM +, 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[EMAIL PROTECTED]
--- modules/generators/mod_autoindex.c 2003-01-25 17:54:28.0 +
+++ modules/generators/mod_autoindex.c 2003-01-25 17:57:47.0 +
@@ -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.0 +
+++ modules/generators/mod_autoindex.c 2003-01-25 18:00:07.0 +
@@ -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.0 +
+++ docs/manual/mod/mod_autoindex.xml 2003-01-25 18:01:59.0 +
@@ -309,7 +309,9 @@
Name is either ^^DIRECTORY^^ for directories,
^^BLANKICON^^ for blank lines (to format the list
-correctly), a file extension, a wildcard expression, a partial
+correctly), ^^UNAUTHORIZED^^ 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.
Examples
[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[EMAIL PROTECTED]
--- modules/generators/mod_autoindex.c 2003-01-06 15:24:29.0 +
+++ modules/generators/mod_autoindex.c 2003-01-25 17:54:28.0 +
@@ -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, b
Re: cvs commit: httpd-2.0/server request.c
On Thu, Dec 12, 2002 at 11:31:44AM -0500, Paul J. Reder wrote: > [EMAIL PROTECTED] wrote: > > >wrowe 2002/12/11 23:05:54 > > > > Modified:server request.c > > Log: > >Make the code simpler to follow, and perhaps clear up the > >follow-symlink > >bug reports we have seen on bugzilla. e.g. 14206 etc. > > Sorry to be the bearer of bad news but the problem reported in 14206 still > occurs with this new code. [snip] > Now bring up your browser and request: > > http://your.machine.name:port/index.html > > You'll get a 403:forbidden error. > > http://your.machine.name:port/ > > You'll get the page foo.html. > > I can spend more time tracking this if you want, but it won't be > till this afternoon. Can I point out the mail archived at http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=103938644204488&w=2 sent on December 8th, which has some discussion of this issue. It might give some pointers as to where to start looking. The patch there probably won't apply cleanly any more, of course. All the best, f -- Francis Daly[EMAIL PROTECTED]
symlink-following bug in ap_directory_walk?
work too. However,
it wants to handle symlinks itself between lines 2071 and 2088 (lines
2063 and 2080) and so does a stat(). Bad idea. Rip out that code, and
make_autoindex_entry will pass the APR_LNK (if that is what it is) on to
ap_sub_req_lookup_dirent, which will now do the right thing with it.
With that change, mod_autoindex now does what it should, and does not
list files it will not be able to serve. (GET #4 works.)
I offer two patches, the first against the current server/request.c
(cvs 1.121) and the second against the current mod_autoindex.c (cvs
1.112). The second is probably correct -- I don't see anything
done in the deleted section that is not now handled correctly. If
it is significantly slower this new way, then perhaps considering
OPT_SYM_LINKS before the stat() will be an adequate optimization. The
first should be considered a draft patch, for a few reasons.
Mainly, it messes with ap_directory_walk, which is an important part of
the system, and so shouldn't be messed with unless the committer is sure
it's correct.
Also, I've only tested this one one system, and this code change impacts
all systems the server runs on. Validation from someone with a different
system would be good. Particularly in relation to full_name_len.
Proper performance testing would also be a good idea.
And finally, the patch is pretty much the minimal code change that I
can find to get everything working. Any patch to apr_directory_walk has
potential security implications (in fact, this one tries to close some
potential holes), and I didn't want to clutter the main point of it with
extra changes.
Examples of some possible extra changes:
strlen(r->filename) is called twice; they could now be replaced by the
new full_name_len.
The line
if (!r->finfo.filetype || r->finfo.filetype == APR_LNK) {
could be replaced by
if (!r->finfo.filetype) {
since we're now doing an apr_lstat().
Unless I'm misreading it, the line
&& ((opts.opts & (OPT_SYM_OWNER | OPT_SYM_LINKS)) == OPT_SYM_LINKS))
is exactly equivalent to
&& (opts.opts & OPT_SYM_LINKS))
and could be replaced by it.
apr_lstat() is a deprecated function, so can be multi-replaced by a
suitable apr_stat(), saving a function call each time.
And ap_sub_req_lookup_file() plays the same games with
ap_allow_options(rnew) as ap_sub_req_lookup_dirent() did; I haven't
checked, but wouldn't be surprised if it suffers the same flaw of not
being set correctly at that point.
There are probably some more bits I've missed, but I'll get these
patches out there so that people more familiar with the base code can
point out (and fix) the problems with this.
All the best,
f
--
Francis Daly[EMAIL PROTECTED]
--- server/request.cSun Nov 3 23:12:22 2002
+++ server/request.cTue Dec 3 00:39:22 2002
@@ -528,6 +528,7 @@
walk_cache_t *cache;
char *entry_dir;
apr_status_t rv;
+apr_size_t full_name_len;
/* XXX: Better (faster) tests needed!!!
*
@@ -563,6 +564,7 @@
* have significant security holes.
*/
r->filename = entry_dir;
+full_name_len = strlen(r->filename);
cache = prep_walk_cache(AP_NOTE_DIRECTORY_WALK, r);
@@ -577,7 +579,7 @@
* with APR_ENOENT, knowing that the path is good.
*/
if (!r->finfo.filetype || r->finfo.filetype == APR_LNK) {
-apr_stat(&r->finfo, r->filename, APR_FINFO_MIN, r->pool);
+apr_lstat(&r->finfo, r->filename, APR_FINFO_MIN, r->pool);
/* some OSs will return APR_SUCCESS/APR_REG if we stat
* a regular file but we have '/' at the end of the name;
@@ -979,6 +981,7 @@
#ifdef CASE_BLIND_FILESYSTEM
&& (filename_len <= canonical_len)
#endif
+&& (filename_len < full_name_len)
&& ((opts.opts & (OPT_SYM_OWNER | OPT_SYM_LINKS)) == OPT_SYM_LINKS))
{
@@ -1728,35 +1731,14 @@
* this should be safe.
*/
apr_status_t rv;
-if (ap_allow_options(rnew) & OPT_SYM_LINKS) {
-if (((rv = apr_stat(&rnew->finfo, rnew->filename,
-APR_FINFO_MIN, rnew->pool)) != APR_SUCCESS)
-&& (rv != APR_INCOMPLETE)) {
-rnew->finfo.filetype = 0;
-}
-}
-else {
-if (((rv = apr_lstat(&rnew->finfo, rnew->filename,
- APR_FINFO_MIN, rnew->pool)) != APR_SUCCESS)
-&& (rv != APR_INCOMPLETE)) {
-rnew->finfo.filetype = 0;
-}
+if (((rv = apr_lstat(&rnew->finfo, rnew->filename,
+ APR_FINFO_MIN, rnew->pool)) != APR_SUCCESS)
+&& (rv != APR_INCOMPLETE)) {
+rnew->finfo
Re: [PATCH] Case-insensitive listing in mod_autoindex
On Wed, Nov 27, 2002 at 11:29:36AM -0600, William A. Rowe, Jr. wrote:
> Francis, to the extent you don't want to reinvent the wheel;
>
> http://cvs.apache.org/viewcvs/apache-1.3/src/modules/standard/mod_autoindex.c
>
> should help you find that patch. Because 2.0 split before the feature
> was added, we 'missed' the new feature.
Cheers, the 1.3 code was the "heavy inspiration" for this patch.
Attached is a patch to introduce "IgnoreCase" as an allowable
"IndexOption", restoring a function from the late 1.3 series.
It's a fairly straightforward patch, but with one gotcha that
might be the reason why it hasn't been included prior to now:
this addition forces the 17th bit of the options to be used.
It works fine on my 32-bit-int system, but may have an impact
on a 16-bit-int system. The value was apr_int32_t already, but
was referred to as an int in some places. I think I've switched
them all (both) to apr_int32_t, although it'd be worth someone
with access to a 16-bit-int system letting their compiler verify
that.
The suggested docs patch is "strongly reminiscent" of the 1.3.27
docs. This is not a coincidence.
Built and tested against the released 2.0.43 code, the patches
apply (with a line offset) to the current CVS versions.
As usual, criticisms and suggested improvements welcome.
f
--
Francis Daly[EMAIL PROTECTED]
--- modules/generators/mod_autoindex.c Wed Nov 27 20:44:33 2002
+++ modules/generators/mod_autoindex.c Wed Nov 27 21:59:51 2002
@@ -110,6 +110,7 @@
#define FANCY_INDEXING 0x2000
#define TABLE_INDEXING 0x4000
#define IGNORE_CLIENT 0x8000
+#define SORT_NOCASE 0x1
#define K_NOADJUST 0
#define K_ADJUST 1
@@ -353,7 +354,7 @@
opts_add = d_cfg->incremented_opts;
opts_remove = d_cfg->decremented_opts;
while (optstr[0]) {
-int option = 0;
+apr_int32_t option = 0;
w = ap_getword_conf(cmd->pool, &optstr);
if ((*w == '+') || (*w == '-')) {
@@ -407,6 +408,9 @@
else if (!strcasecmp(w, "VersionSort")) {
option = VERSION_SORT;
}
+else if (!strcasecmp(w, "IgnoreCase")) {
+option = SORT_NOCASE;
+}
else if (!strcasecmp(w, "None")) {
if (action != '\0') {
return "Cannot combine '+' or '-' with 'None' keyword";
@@ -727,6 +731,7 @@
int ascending, version_sort;
char key;
int isdir;
+int ignore_case;
};
static char *find_item(request_rec *r, apr_array_header_t *list, int path_only)
@@ -1274,7 +1279,7 @@
}
static struct ent *make_autoindex_entry(const apr_finfo_t *dirent,
-int autoindex_opts,
+apr_int32_t autoindex_opts,
autoindex_config_rec *d,
request_rec *r, char keyid,
char direction,
@@ -1337,6 +1342,7 @@
p->key = apr_toupper(keyid);
p->ascending = (apr_toupper(direction) == D_ASCENDING);
p->version_sort = !!(autoindex_opts & VERSION_SORT);
+p->ignore_case = !!(autoindex_opts & SORT_NOCASE);
if (autoindex_opts & (FANCY_INDEXING | TABLE_INDEXING)) {
p->lm = rr->finfo.mtime;
@@ -1888,6 +1894,17 @@
return result;
}
break;
+}
+if (c1->ignore_case) {
+if (c1->version_sort) {
+result = apr_strnatcasecmp(c1->name, c2->name);
+}
+else {
+result = strcasecmp(c1->name, c2->name);
+}
+if (result) {
+return result;
+}
}
if (c1->version_sort) {
return apr_strnatcmp(c1->name, c2->name);
--- manual/mod/mod_autoindex.xmlThu Nov 21 01:39:04 2002
+++ manual/mod/mod_autoindex.xmlWed Nov 27 23:57:13 2002
@@ -73,6 +73,10 @@
option below may be added to any request for the directory
resource.
+After Apache 2.0.44, the sorting by file name
+can be case-sensitive or not, according to IndexOptions IgnoreCase
+
C=N sorts the directory by file name
@@ -592,6 +596,19 @@
loaded. If no value is given for the option, it defaults to
the standard width of the icons supplied with the Apache
software.
+
+ IgnoreCase (Apache
+ 2.0.44 and later)
+
+ If this option is enabled, names are sorted in
+ case-insensitive manner. For instance, if the sort order is
+ ascending by name, and IgnoreCase is enabled, file
+ Zeta will be listed after file alpha
+ (Note: file GAMMA will always be listed before file
+ gamma). This option only has an effect if FancyIndexing
+ is also enabled.
IgnoreClient
Re: [PATCH] Case-insensitive listing in mod_autoindex
On Wed, Nov 27, 2002 at 11:33:08AM -0500, Joshua Slive wrote: > On Wed, 27 Nov 2002, Francis Daly wrote: > > Note that this is not equivalent to the old IgnoreCase; if identical > > functionality is wanted, then a different patch will be needed. > > Unless there is a good reason to change, it is better to keep the > configuration the same as in 1.3. There is no need to throw unnecessary > obstacles in the way of upgraders. Cool -- I'll see about putting together a straightforward forward-port of the IgnoreCase IndexOption as an alternative to this patch, then. (Unless someone beats me to it, of course.) All the best, f -- Francis Daly[EMAIL PROTECTED]
[PATCH] Case-insensitive listing in mod_autoindex
Hi there,
Apache/1.3.24+ had the option to sort the file list generated by
mod_autoindex in a case-insensitive fashion, by using "IndexOptions
+IgnoreCase".
This functionality has not been carried over to Apache/2.
There is an open bug report on the issue --
http://nagoya.apache.org/bugzilla/show_bug.cgi?14276
I'm not sure if there was a deliberate decision taken to remove this
functionality from Apache/2, or if it's just that no-one has yet gotten
around to putting it back in.
On the off-chance that it is the latter, I offer the following patch
which provides a similar function, but uses a different directive. It
adds "NameNoCase" as a possible second argument of IndexOrderDefault.
It also adds "(case)" to the output list heading as a link to the
case-insensitively-sorted list, if the client is able to choose that.
Note that this is not equivalent to the old IgnoreCase; if identical
functionality is wanted, then a different patch will be needed.
Built and tested against 2.0.43, it was adjusted to apply cleanly to
the current cvs version of mod_autoindex.c, 1.112.
The second patch should apply cleanly to version 1.12 of
mod_autoindex.xml. It contains the usual amount of presumption; a true
compatibility note can be added depending on when and if the patch
is accepted.
Any comments welcome,
f
--
Francis Daly[EMAIL PROTECTED]
--- mod_autoindex.c-1.112 Wed Nov 27 12:53:44 2002
+++ mod_autoindex.c Wed Nov 27 13:25:04 2002
@@ -119,10 +119,11 @@
* Define keys for sorting.
*/
#define K_NAME 'N' /* Sort by file name (default) */
+#define K_NO_CASE 'C' /* Case-insensitive name */
#define K_LAST_MOD 'M' /* Last modification date */
#define K_SIZE 'S' /* Size (absolute, not as displayed) */
#define K_DESC 'D' /* Description */
-#define K_VALID "NMSD" /* String containing _all_ valid K_ opts */
+#define K_VALID "NCMSD" /* String containing _all_ valid K_ opts */
#define D_ASCENDING 'A'
#define D_DESCENDING 'D'
@@ -537,6 +538,9 @@
if (!strcasecmp(key, "Name")) {
d_cfg->default_keyid = K_NAME;
}
+else if (!strcasecmp(key, "NameNoCase")) {
+d_cfg->default_keyid = K_NO_CASE;
+}
else if (!strcasecmp(key, "Date")) {
d_cfg->default_keyid = K_LAST_MOD;
}
@@ -547,8 +551,8 @@
d_cfg->default_keyid = K_DESC;
}
else {
-return "Second keyword must be 'Name', 'Date', 'Size', or "
- "'Description'";
+return "Second keyword must be 'Name', 'NameNoCase', 'Date', "
+ "'Size', or 'Description'";
}
return NULL;
@@ -573,7 +577,7 @@
AP_INIT_RAW_ARGS("IndexOptions", add_opts, NULL, DIR_CMD_PERMS,
"one or more index options [+|-][]"),
AP_INIT_TAKE2("IndexOrderDefault", set_default_order, NULL, DIR_CMD_PERMS,
- "{Ascending,Descending} {Name,Size,Description,Date}"),
+ "{Ascending,Descending} {Name,NameNoCase,Size,Description,Date}"),
AP_INIT_ITERATE("IndexIgnore", add_ignore, NULL, DIR_CMD_PERMS,
"one or more file extensions"),
AP_INIT_ITERATE2("AddDescription", add_desc, BY_PATH, DIR_CMD_PERMS,
@@ -1552,6 +1556,10 @@
ap_rputs("", r);
emit_link(r, "Name", K_NAME, keyid, direction,
colargs, static_columns);
+if (!static_columns) {
+emit_link(r, " (case)", K_NO_CASE, keyid, direction,
+ colargs, static_columns);
+}
if (!(autoindex_opts & SUPPRESS_LAST_MOD)) {
ap_rputs("", r);
emit_link(r, "Last modified", K_LAST_MOD, keyid, direction,
@@ -1597,7 +1605,13 @@
}
emit_link(r, "Name", K_NAME, keyid, direction,
colargs, static_columns);
-ap_rputs(pad_scratch + 4, r);
+if (!static_columns) {
+emit_link(r, " (case)", K_NO_CASE, keyid, direction,
+ colargs, static_columns);
+ap_rputs(pad_scratch + 4 + 7, r);
+} else {
+ap_rputs(pad_scratch + 4, r);
+}
/*
* Emit the guaranteed-at-least-one-space-between-columns byte.
*/
@@ -1891,6 +1905,17 @@
else {
result = strcmp(c1->desc ? c1->desc : "",
c2->desc ? c2->desc : "");
+}
+if (result) {
+return result;
+}
+br
Re: [PATCH] IndexResults for mod_autoindex
On Fri, Nov 22, 2002 at 11:52:10PM -0600, William A. Rowe, Jr. wrote:
> Francis, very cool patch! I'll look at it if noone beats me to it,
> you aren't the only one who wants this feature :-)
Good to hear, thanks.
One caveat, though, that I should have noticed before posting:
A commit about a year ago (v 1.90) removed identifiers called "stat".
I very cleverly then went and added...
> >+static int show_result(autoindex_config_rec *d, int status)
> >+{
> >+char stat[4];/* HTTP codes are 3 digits long */
..an identifier called "stat".
You may want to modify that before any eventual commit, for
consistency.
All the best,
f
--
Francis Daly[EMAIL PROTECTED]
[PATCH] mod_autoindex missing some filenames
Hi there,
Some files with % in the name aren't listed by mod_autoindex.
(See http://nagoya.apache.org/bugzilla/show_bug.cgi?id=13598 for a
report)
The behaviour seems to have become broken this way some time between
2.0.25 and 2.0.28.
As far as I can tell, if a file name is like x%33x, then ap_unescape_url
allows it, and the file is listed. If it is like x%ggx, then it fails
it, and it is not listed. If the file is listed, it is listed correctly,
with the correct html and uri escaping in both the href and the content
part.
In the cases above, "33" is a valid %-encoding, and "gg" isn't.
The breakage seems to be in ap_process_request_internal(), when
ap_unescape_url(r->parsed_uri.path) is called. I don't believe it
needs to be called at all, since r->uri has just come straight from
the filesystem. The obvious place to change things seems to be in
ap_sub_req_lookup_dirent(). Near the end of that function is the comment
"fill in parsed_uri values". Is it actually necessary to do that?
In the 2.0.43 code, ap_sub_req_lookup_dirent is called in two places:
mod_autoindex.c: ap_sub_req_lookup_dirent(dirent, r,
AP_SUBREQ_NO_ARGS, NULL)
mod_negotiation.c: ap_sub_req_lookup_dirent(&dirent, r,
AP_SUBREQ_MERGE_ARGS, NULL)
mod_autoindex doesn't appear to directly care about ->unparsed_uri,
or ->parsed_uri.*, which seem to be what ap_parse_uri modifies.
mod_negotiation also doesn't appear to directly care about them.
Possibly they are needed by some later potential module/filter?
As a test of both mod_autoindex and mod_negotiation, create some
files:
cd DOCROOT
mkdir scratch
echo t%31.html > scratch/t%31.html
echo t%g1.html > scratch/t%g1.html
Options +Indexes +MultiViews
Vanilla:
$ GET http://localhost:2043/scratch/
..
t%31.html
26-Nov-2002 17:34 10
..
$ GET http://localhost:2043/scratch/t%2531
t%31.html
$ GET http://localhost:2043/scratch/t%25g1
..
404 Not Found
..
Patch server/request.c by commenting out lines 1757 to 1763 (cvs
version 1.114)
$ GET http://localhost:2043/scratch/
..
t%31.html
26-Nov-2002 17:34 10
t%g1.html
26-Nov-2002 17:34 10
..
$ GET http://localhost:2043/scratch/t%2531
t%31.html
$ GET http://localhost:2043/scratch/t%25g1
t%g1.html
The output looks right to me, although I'm not sure what nasty side
effects might be brought in by removing the call to ap_parse_uri().
Those lines are 1774 to 1780 in the current cvs version, 1.121, and
that is what this patch is against.
(Of course, if this is the right fix, then the right implementation of
the fix is just to delete those 7 lines.)
There may be some better way of not calling ap_unescape_url() on a
plain filename which is more appropriate.
All the best,
f
--
Francis Daly[EMAIL PROTECTED]
--- request.c-1.121 Tue Nov 26 17:42:35 2002
+++ request.c Tue Nov 26 17:58:07 2002
@@ -1771,6 +1771,7 @@
/* fill in parsed_uri values
*/
+/*
if (r->args && *r->args && (subtype == AP_SUBREQ_MERGE_ARGS)) {
ap_parse_uri(rnew, apr_pstrcat(r->pool, rnew->uri, "?",
r->args, NULL));
@@ -1778,6 +1779,7 @@
else {
ap_parse_uri(rnew, rnew->uri);
}
+*/
if ((res = ap_process_request_internal(rnew))) {
rnew->status = res;
http://httpd.apache.org/dev/ problem
I'm not sure if this should go to dev or docs, but I'll try here as I suspect that it is due to a recent change by someone here. Sorry if I've got this bit wrong. The web page returned from http://httpd.apache.org/dev/ looks strangely blank in my browser right now, almost certainly because there are 10 "
Re: [PATCH] IndexResults for mod_autoindex
On Thu, Nov 21, 2002 at 09:09:35AM +, Francis Daly wrote:
> There are two further optional code patches to follow, which are only
> useful if something similar to these are accepted.
Here come the patches...
The first changes the icon presented in the mod_autoindex listing (if
FancyIndexing is on) to the whatever is set for ^^UNAUTHORIZED^^ or,
failing that, the default. (The final patch is a docs patch for that magic
string.)
The second additionally hides details like file size and modification
time from unauthorized requesters. If they want that info, they can
HEAD the resource with credentials, or just request the directory
index with credentials.
The first patch is probably useful. The second is possibly
unnecessary. I like them both. Neither are required.
All are against the as-patched-in-previous-mail versions, although
they should be trivial to modify if those patches are changed before
being accepted.
All the best,
f
--
Francis Daly[EMAIL PROTECTED]
--- modules/generators/mod_autoindex.c 21 Nov 2002 02:35:54 -
+++ modules/generators/mod_autoindex.c 21 Nov 2002 02:40:23 -
@@ -1468,6 +1468,11 @@
p->ascending = (apr_toupper(direction) == D_ASCENDING);
p->version_sort = !!(autoindex_opts & VERSION_SORT);
+/* 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 21 Nov 2002 02:40:23 -
+++ modules/generators//mod_autoindex.c 21 Nov 2002 02:41:24 -
@@ -1468,9 +1468,12 @@
p->ascending = (apr_toupper(direction) == D_ASCENDING);
p->version_sort = !!(autoindex_opts & VERSION_SORT);
-/* 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)) {
--- mod_autoindex.xml.new Thu Nov 21 09:27:14 2002
+++ mod_autoindex.xml Thu Nov 21 09:28:10 2002
@@ -309,7 +309,9 @@
Name is either ^^DIRECTORY^^ for directories,
^^BLANKICON^^ for blank lines (to format the list
-correctly), a file extension, a wildcard expression, a partial
+correctly), ^^UNAUTHORIZED^^ for anything for which
+the user has not provided appropriate credentials (available from
+2.0.44), a file extension, a wildcard expression, a partial
filename or a complete filename.
Examples
[PATCH] IndexResults for mod_autoindex
Hi there,
This is a late follow-on from some posts in May.
mod_autoindex in Apache/2 does not show filenames for which the requester is
not authenticated, which is a change from the 1.3 behaviour.
The first patch below introduces a per-directory directive which
allows the administrator list which http statuses (or status groups)
should be shown or hidden.
When left unset, I found no clear performance difference in listing a
directory where no authentication was required (as expected); and also
found no clear performance difference in listing a directory where each
file individually required authentication (which was a bit of a
surprise to me).
Built and tested against vanilla 2.0.43, it applies cleanly to the
current CVS version, 1.111.
The second patch below is a docs patch against mod_autoindex.xml 1.12.
There are two further optional code patches to follow, which are only
useful if something similar to these are accepted.
Any comments (and further performance testing) welcome.
f
--
Francis Daly[EMAIL PROTECTED]
--- modules/generators/mod_autoindex.c 17 Oct 2002 18:09:52 -
+++ modules/generators/mod_autoindex.c 21 Nov 2002 02:33:31 -
@@ -154,6 +154,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;
@@ -175,6 +181,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;
@@ -320,6 +327,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);
@@ -578,6 +624,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,
@@ -590,7 +638,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));
@@ -607,15 +655,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, con
Re: [PATCH] ServerSignature privacy - option 1
On Sat, Nov 02, 2002 at 11:29:29AM -0800, Justin Erenkrantz wrote:
>
> >The disadvantage of it is that the current behaviour
> >cannot be replicated -- if ServerTokens is ProductOnly, for
> >example, the signature cannot be the current "Apache/2.0.43". For
> >me, this isn't a problem. For others, it might be --
>
> Nah, I'm not terribly concerned about that edge case.
That suits me fine.
> >Anyway, below is patch alternative 1: change current behaviour to
> >only allow what I want.
>
> I like this alternative much more than the other one. I'm a believer
> that ServerTokens is that 'authoritative' version that should always
> be represented to the world.
>
> However, wouldn't it be better to just have it return
> ap_server_version() rather than trying to be cute and cut off at the
> first space?
The reason I cut it off there was to (as near as possible) mimic
current behaviour. ap_server_version() can return quite a long string,
especially if there are lots of third party modules loaded. The
Server: header from some well-known Apache/1.3 sites exceeds 80
characters.
If it's considered appropriate, then it makes the patch much smaller,
and (presumably) the code that bit faster.
I don't believe there's a danger of any client-side data appearing
there, but even so it may be worth wrapping the output of
ap_server_version() with ap_escape_html() -- although if a webmaster
chooses to load mod_, perhaps they shouldn't be helped. If it
is wanted, the change is obvious.
> If ServerTokens is 'full' anyway, you're already
> exposing it, so I don't see a large concern. It might be a bit more
> than we had before, but I don't think that's going to scare anyone
> away. Perhaps it'll teach people to use 'minimal' more often.
That sounds reasonable to me, and no-one has yet contradicted it that
I have seen.
> And, if you could submit a patch for the documentation, that'd be
> appreciated. =)
I was hoping to be lazy and just provide the words, and let someone
who knows more about the current doc setup do the real work. Oh
well...
Two patches below: one is for httpd-2.0/server/core.c, which just adds
(unescaped) ap_get_server_version() to ap_psignature. Against the
current CVS version; not compiled, not tested, but it looks right to
me.
The other is for httpd-docs-2.0/manual/mod/core.xml, which adds an
extra comment to the two directives. Also against the most recent CVS
version; hopefully I've got the style correct. Obviously, if the
actually-committed patch includes the "stop after Minimal" code, then
the words here are wrong.
All the best,
f
--
Francis Daly[EMAIL PROTECTED]
--- core.c.1.216Tue Nov 5 13:22:10 2002
+++ core.c Tue Nov 5 13:24:12 2002
@@ -2265,7 +2265,8 @@
apr_snprintf(sport, sizeof sport, "%u", (unsigned) ap_get_server_port(r));
if (conf->server_signature == srv_sig_withmail) {
-return apr_pstrcat(r->pool, prefix, "" AP_SERVER_BASEVERSION
+return apr_pstrcat(r->pool, prefix, "",
+ ap_get_server_version(),
" Server at mailto:";,
r->server->server_admin, "\">",
ap_escape_html(r->pool, ap_get_server_name(r)),
@@ -2273,7 +2274,7 @@
"\n", NULL);
}
-return apr_pstrcat(r->pool, prefix, "" AP_SERVER_BASEVERSION
+return apr_pstrcat(r->pool, prefix, "", ap_get_server_version(),
" Server at ",
ap_escape_html(r->pool, ap_get_server_name(r)),
" Port ", sport,
--- core.xml.1.40 Tue Nov 5 13:07:40 2002
+++ core.xmlTue Nov 5 13:10:45 2002
@@ -2509,6 +2509,8 @@
"mailto:"; reference to the ServerAdmin of the referenced
document.
+After version 2.0.44, the details of the server version number
+presented are controlled by the ServerTokens directive.
ServerTokens
@@ -2560,6 +2562,9 @@
This setting applies to the entire server, and cannot be
enabled or disabled on a virtualhost-by-virtualhost basis.
+
+After version 2.0.44, this directive also controls the
+information presented by the ServerSignature directive.
ServerSignature
[PATCH] ServerSignature privacy - option 2
Hi there,
As promised, the "ServerSignature should track ServerTokens" patch
alternative 2: add directive to change current behaviour to allow what I
want, while retaining current behaviour for everyone else.
Built against the released 2.0.43 code, my (limited) testing doesn't
show a significant throughput difference compared with the current code
when ServerSigStyle isn't set. It applies to the current CVS versions,
1.215 and 1.70, with a few line offsets.
The patch adds a field to core_dir_config associated with the directive,
so a full rebuild is probably necessary.
Documentation: ServerSigStyle is a directive to modify the signature
added to server-generated pages. It only has an effect when
ServerSignature is set (to on or email), and can be set down to
the .htaccess level, the same as ServerSignature. It can be set to
"traditional" or "header" (defaulting to acting like "traditional")
-- meaning "replicating the current behaviour" or "giving no more
information than the Server: header, as set by ServerTokens".
In "traditional" mode, the output is like "Apache/2.0.43" irrespective
of how ServerTokens is set. In "header" mode, if "ServerTokens" is set
to, for example, "Major", then the signature would be like "Apache/2",
just like the Server: header. Setting "ServerTokens" beyond "Minimal" --
to "OS" or "Full" -- does not increase the signature output, so it will
not generate more output that the "traditional" setting.
There should probably be a docs patch for ServerSignature saying
something like "the content generated, if any, can be controlled by the
setting of ServerSigStyle".
There could possibly also be a docs patch for ServerTokens indicating
that setting it may influence ServerSignature, depending on how
ServerSigStyle is set, but I think that that would be unnecessary.
Unless someone goes out of their way to set ServerSigStyle, they
should see no change. And if they do that, they should read the docs
for it.
Anyway, any and all comments on the patch are welcome.
f
--
Francis Daly[EMAIL PROTECTED]
--- include-virgin/http_core.h Sun May 12 00:24:29 2002
+++ include/http_core.h Sun Oct 27 20:51:22 2002
@@ -406,6 +406,12 @@
srv_sig_withmail
} server_signature_e;
+typedef enum {
+srv_sig_sty_unset,
+srv_sig_sty_trad,
+srv_sig_sty_head,
+} server_sig_style_e;
+
typedef struct {
/* path of the directory/regex/etc. see also d_is_fnmatch/absolute below */
char *d;
@@ -494,6 +500,7 @@
/* logging options */
server_signature_e server_signature;
+server_sig_style_e server_sig_style;
int loglevel;
--- server-virgin/core.cWed Oct 2 22:35:57 2002
+++ server/core.c Sun Oct 27 21:58:19 2002
@@ -162,6 +162,7 @@
conf->sec_file = apr_array_make(a, 2, sizeof(ap_conf_vector_t *));
conf->server_signature = srv_sig_unset;
+conf->server_sig_style = srv_sig_sty_unset;
conf->add_default_charset = ADD_DEFAULT_CHARSET_UNSET;
conf->add_default_charset_name = DEFAULT_ADD_DEFAULT_CHARSET_NAME;
@@ -384,6 +385,10 @@
conf->server_signature = new->server_signature;
}
+if (new->server_sig_style != srv_sig_sty_unset) {
+conf->server_sig_style = new->server_sig_style;
+}
+
if (new->add_default_charset != ADD_DEFAULT_CHARSET_UNSET) {
conf->add_default_charset = new->add_default_charset;
conf->add_default_charset_name = new->add_default_charset_name;
@@ -2015,6 +2020,29 @@
return NULL;
}
+static const char *set_sig_style(cmd_parms *cmd, void *d_,
+ const char *arg)
+{
+core_dir_config *d = d_;
+const char *err = ap_check_cmd_context(cmd, NOT_IN_LIMIT);
+
+if (err != NULL) {
+return err;
+}
+
+if (strcasecmp(arg, "traditional") == 0) {
+d->server_sig_style = srv_sig_sty_trad;
+}
+else if (strcasecmp(arg, "header") == 0) {
+d->server_sig_style = srv_sig_sty_head;
+}
+else {
+return "ServerSigStyle: use one of: traditional | header";
+}
+
+return NULL;
+}
+
static const char *set_server_root(cmd_parms *cmd, void *dummy,
const char *arg)
{
@@ -2226,6 +2254,9 @@
{
char sport[20];
core_dir_config *conf;
+const char *version_s;
+char *version;
+char *end;
conf = (core_dir_config *)ap_get_module_config(r->per_dir_config,
&core_module);
@@ -2236,8 +2267,18 @@
apr_snprintf(sport, sizeof sport, "%u", (unsigned) ap_get_server_port(r));
+if (conf->server_sig_style == srv_sig_sty_head) {
+version = (char *)version
[PATCH] ServerSignature privacy - option 1
Hi there,
About a year ago, there was a discussion about the fact that
"ServerTokens" could be used to limit the detailed information sent
about the server on every request, while "ServerSignature" only showed
the full product version (or nothing at all).
(See, for example, http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=100323367832594&w=2>
as a starting point -- the threads "Better privacy with
SERVER_SIGNATURE" and "[PATCH] for ServerSignatures / ServerTokens" are
the relevant ones)
[ ServerSignature is the option that sets how the bottom of a
mod_autoindex-generated page looks. The information it generates (in the
function ap_psignature in server/core.c) can also appear in the output
mod_dav, mod_info, and mod_status, amongst other places ]
I prefer to have ServerSignature reveal no more information that the
Server: header (controlled by ServerTokens), so I provide two suggested
patches to add this behaviour. There were patches provided in the
above-referenced threads, but the facility doesn't seem to exists in the
current code.
The first patch, below, only modifies server/core.c so that the output
of ap_psignature tracks the value of ServerTokens (up to the level
of ServerTokens Minimal, which is the current sole possibility). The
disadvantage of it is that the current behaviour cannot be replicated
-- if ServerTokens is ProductOnly, for example, the signature cannot be
the current "Apache/2.0.43". For me, this isn't a problem. For others,
it might be -- especially if, for example, the information is used in
mod_status to find the running version (where, for some reason, httpd -v
isn't practical).
For that reason, there is also an alternative patch, in a following
mail, which modifies both server/core.c and include/http_core.h
to add an option, ServerSigStyle, which defaults to "traditional"
(meaning "replicates the current behaviour") but can be set to "header"
(meaning "track the Server: header" as described above) wherever
ServerSignature can be set. The disadvantage of that patch is that it
modifies core_dir_config to add a new directive. Does that count as a
disadvantage?
Anyway, below is patch alternative 1: change current behaviour to only
allow what I want. Built against the released 2.0.43 code, my (limited)
testing doesn't show a significant throughput difference compared with
the current code. It applies to the current CVS version, 1.215, with a
28-line offset.
A documentation patch for "ServerTokens" should say something like "this
also affects the ServerSignature output, if that directive is not off";
while the "ServerSignature" docs should be modified to say something
like "the signature generated depends on the setting of ServerTokens"
Any comments are welcome,
f
--
Francis Daly[EMAIL PROTECTED]
--- server-virgin/core.cWed Oct 2 22:35:57 2002
+++ server/core.c Sun Oct 27 19:54:50 2002
@@ -2226,6 +2226,9 @@
{
char sport[20];
core_dir_config *conf;
+const char *version_s;
+char *version;
+char *end;
conf = (core_dir_config *)ap_get_module_config(r->per_dir_config,
&core_module);
@@ -2235,9 +2238,15 @@
}
apr_snprintf(sport, sizeof sport, "%u", (unsigned) ap_get_server_port(r));
+version = (char *)version_s = ap_get_server_version();
+
+if ((end = strchr(version_s + strlen(AP_SERVER_BASEPRODUCT), ' '))
+!= NULL) {
+version = apr_pstrndup(r->pool, version_s, end - version_s);
+}
if (conf->server_signature == srv_sig_withmail) {
-return apr_pstrcat(r->pool, prefix, "" AP_SERVER_BASEVERSION
+return apr_pstrcat(r->pool, prefix, "", version,
" Server at mailto:";,
r->server->server_admin, "\">",
ap_escape_html(r->pool, ap_get_server_name(r)),
@@ -2245,7 +2254,7 @@
"\n", NULL);
}
-return apr_pstrcat(r->pool, prefix, "" AP_SERVER_BASEVERSION
+return apr_pstrcat(r->pool, prefix, "", version,
" Server at ",
ap_escape_html(r->pool, ap_get_server_name(r)),
" Port ", sport,
[PATCH] Re: Information disclosure on mod_auth ( apache 1.3.26 ) ?
On Tue, Aug 20, 2002 at 02:54:52AM -0400, Cliff Woolley wrote:
> On Fri, 16 Aug 2002, Hector A. Paterno wrote:
>
> > Hi, I have found a discrepancy between mod_auth and ServerTokens Prod.
> > HEAD / HTTP/1.0\r\n\r\n
> > Server: Apache
> > 401 Authorization Required
> > [bleh bleh info]
> > Apache/1.3.26 Server at x Port 80
> > Giving me the version of the apache server.
> This is a misconfiguration. See also the ServerSignature directive.
Is it configurable enough, though? (Note: it's not specifically
mod_auth doing this, it's every server-generated page.)
Only considering apache 2, the current "generate the signature" code,
ap_psignature in server/core.c, seems to allow the following output,
based purely on the value of ServerSignature:
- no signature at all
- Apache/2.0.40 Server at web.example.com Port 80
- Apache/2.0.40 Server at web.example.com Port 80, with "web.example.com"
being a link to the admin email address.
What I think the original poster wants is the option of
- Apache Server at web.example.com Port 80
with or without the email address. Whatever about the original poster,
that's something that I would like to have.
One way to achieve this could be to base it on "ServerTokens ProductOnly",
and change the signature-generating lines lines like
return apr_pstrcat(r->pool, prefix, "" AP_SERVER_BASEVERSION
" Server at ", ap_get_server_name(r), " Port ", sport,
"\n", NULL);
to
return apr_pstrcat(r->pool, prefix, "",
ap_server_tokens == SrvTk_PRODUCT_ONLY ?
AP_SERVER_BASEPRODUCT : AP_SERVER_BASEVERSION,
" Server at ", ap_get_server_name(r), " Port ", sport,
"\n", NULL);
Included is a patch which does just that for core.c version 1.199,
the current one in CVS. It makes the above three-line change in two
places (one with email address, one without), and then moves the
whole ap_psignature function down the file a bit, to below where
ap_server_tokens is declared -- most of the patch is just moving
otherwise-unchanged lines.
Built and tested on 1.184 (2.0.39), it applies cleanly to 1.199.
This *does* change the content of server-generated pages where someone
has explicitly configured "ServerTokens ProductOnly" and has not also
explicitly configured "ServerSignature off"; and it doesn't include a
configurable means of reverting to current behaviour.
For this reason the patch may be inappropriate, and it might be worth
instead introducing a new ServerSignature option to achieve this
output; I don't think it's worth it, myself. Certainly, the few times
I've configured "ServerTokens ProductOnly" with "ServerSignature on",
I've taken the time to modify core.c so the HTTP headers and the HTML
generated matched.
f
--
Francis Daly[EMAIL PROTECTED]
--- server/core.c.2039 Sat Jun 15 06:49:06 2002
+++ server/core.c Wed Aug 21 09:52:10 2002
@@ -,33 +,6 @@
return NULL;
}
-AP_DECLARE(const char *) ap_psignature(const char *prefix, request_rec *r)
-{
-char sport[20];
-core_dir_config *conf;
-
-conf = (core_dir_config *)ap_get_module_config(r->per_dir_config,
- &core_module);
-if ((conf->server_signature == srv_sig_off)
-|| (conf->server_signature == srv_sig_unset)) {
-return "";
-}
-
-apr_snprintf(sport, sizeof sport, "%u", (unsigned) ap_get_server_port(r));
-
-if (conf->server_signature == srv_sig_withmail) {
-return apr_pstrcat(r->pool, prefix, "" AP_SERVER_BASEVERSION
- " Server at mailto:";,
- r->server->server_admin, "\">",
- ap_get_server_name(r), " Port ", sport,
- "\n", NULL);
-}
-
-return apr_pstrcat(r->pool, prefix, "" AP_SERVER_BASEVERSION
- " Server at ", ap_get_server_name(r), " Port ", sport,
- "\n", NULL);
-}
-
/*
* Load an authorisation realm into our location configuration, applying the
* usual rules that apply to realms.
@@ -2292,6 +2265,37 @@
SrvTk_PRODUCT_ONLY /* eg: Apache */
};
static enum server_token_type ap_server_tokens = SrvTk_FULL;
+
+AP_DECLARE(const char *) ap_psignature(const char *prefix, request_rec *r)
+{
+char sport[20];
+core_dir_config *conf;
+
+conf = (core_dir_config *)ap_get_module_config(r->per_dir_config,
+ &core_
Re: [patch] mod_negotiation and authorization
On Tue, Jul 02, 2002 at 10:31:53AM -0400, Rodent of Unusual Size wrote:
> Did anyone check this one out? (I haven't) It sounds
> as though it would scratch some itches..
Not many other itches, by the looks of things :-(
On the assumption that it would be convenient to make available a
version of the patch that actually applies to the current CVS HEAD,
I'm including an updated version below.
Background and docs are available at, for example,
http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=102190232502173&q=raw
-- if reposting that would be useful, let me know and I'll get on to
it.
Possibly this should be adapted to include some of the other
400-series status codes -- 402, 407, or perhaps 411 or 412 might be
useful (longer term) to be handled similarly. I'll wait to copy from
the mod_autoindex code on that count, though. For now, it's 401 only.
Build and tested against 2.0.39, which appears to still be the current
version (1.102) of mod_negotiation.c
Comments and criticisms welcome.
f
--
Francis Daly[EMAIL PROTECTED]
--- modules/mappers/mod_negotiation.c.2039 Fri May 17 12:24:16 2002
+++ modules/mappers/mod_negotiation.c Mon Jul 8 22:27:45 2002
@@ -88,10 +88,17 @@
*/
typedef struct {
+int reveal_secret_url;
int forcelangpriority;
apr_array_header_t *language_priority;
} neg_dir_config;
+/* reveal_secret_url flags
+ */
+#define RSU_UNDEF2/* this means "no explicit config" */
+#define RSU_ON 1/* "config on" */
+#define RSU_OFF 0/* "config off" */
+
/* forcelangpriority flags
*/
#define FLP_UNDEF0/* Same as FLP_DEFAULT, but base overrides */
@@ -107,6 +114,7 @@
{
neg_dir_config *new = (neg_dir_config *) apr_palloc(p, sizeof(neg_dir_config));
+new->reveal_secret_url = RSU_UNDEF;
new->forcelangpriority = FLP_UNDEF;
new->language_priority = NULL;
return new;
@@ -119,6 +127,9 @@
neg_dir_config *new = (neg_dir_config *) apr_palloc(p, sizeof(neg_dir_config));
/* give priority to the config in the subdirectory */
+new->reveal_secret_url = (add->reveal_secret_url != RSU_UNDEF)
+ ? add->reveal_secret_url
+: base->reveal_secret_url;
new->forcelangpriority = (add->forcelangpriority != FLP_UNDEF)
? add->forcelangpriority
: base->forcelangpriority;
@@ -128,6 +139,22 @@
return new;
}
+static const char *reveal_secret_url(cmd_parms *cmd, void *n_, int arg)
+{
+neg_dir_config *n = n_;
+const char *err = ap_check_cmd_context(cmd, NOT_IN_FILES);
+
+if (err != NULL) {
+return err;
+}
+n->reveal_secret_url = arg == RSU_OFF ? RSU_OFF : RSU_ON;
+/* that is functionally equivalent to
+n->reveal_secret_url = arg != 0;
+ for the RSU_* values #defined'd above. Clarity vs efficiency?
+*/
+return NULL;
+}
+
static const char *set_language_priority(cmd_parms *cmd, void *n_,
const char *lang)
{
@@ -188,6 +215,8 @@
{
AP_INIT_FLAG("CacheNegotiatedDocs", cache_negotiated_docs, NULL, RSRC_CONF,
"Either 'on' or 'off' (default)"),
+AP_INIT_FLAG("MultiviewsRevealSecretURL", reveal_secret_url, NULL,
+RSRC_CONF|OR_AUTHCFG,
+ "Either 'on' or 'off' (default)"),
AP_INIT_ITERATE("LanguagePriority", set_language_priority, NULL, OR_FILEINFO,
"space-delimited list of MIME language abbreviations"),
AP_INIT_ITERATE("ForceLanguagePriority", set_force_priority, NULL, OR_FILEINFO,
@@ -1045,6 +1074,7 @@
struct accept_rec accept_info;
void *new_var;
int anymatch = 0;
+int secretmatch = 0;
clean_var_rec(&mime_info);
@@ -1110,6 +1140,13 @@
if (sub_req->finfo.filetype != APR_REG)
continue;
+/* Note if it failed UNAUTHORIZED. We may want to return this
+ * status, eventually
+ */
+if (sub_req->status == HTTP_UNAUTHORIZED) {
+secretmatch = 1;
+}
+
/* If it has a handler, we'll pretend it's a CGI script,
* since that's a good indication of the sort of thing it
* might be doing.
@@ -1232,6 +1269,9 @@
* request must die.
*/
if (anymatch && !neg->avail_vars->nelts) {
+if (secretmatch && neg->conf->reveal_secret_url == RSU_ON) {
+return HTTP_UNAUTHORIZED;
+}
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"Negotiation: discovered file(s) matching request: %s"
" (None could be negotiated).",
Re: [PATCH] mod_autoindex and authorization [repost]
On Tue, May 28, 2002 at 09:19:33AM -0500, William A. Rowe, Jr. wrote: > At 08:25 AM 5/28/2002, Francis Daly wrote: > > > >Between 1.3 and 2.0, the behaviour of mod_autoindex changed such that > >URLs for which the requester was not (yet) authorized did not appear > >in the generated listings. This patch allows the administrator > >configure, on a per-directory basis, whether or not to show the names > >of the authorization-requiring resources in that directory. > > And the list generally agreed that the right fix is to configure a list > of HTTP result codes that the administrator will allow to be listed, > rather than the toggle you proposed. Ah right, I'd missed that bit of the discussion. I saw the "IndexResults" suggestion, but hadn't noticed that it might be useful to allow, for example, statuses 402 or 41[1-4] too. No harm done. > >It introduces a fake filename "^^UNAUTHORIZED^^" which can be used by > >AddIcon and AddAlt to enhance the display if IndexOptions FancyIndexing > >is also set, mirroring ^^DIRECTORY^^ and ^^BLANKICON^^. An UNAUTHORIZED > >DIRECTORY will appear UNAUTHORIZED, falling back to DefaultIcon. That > >could be changed to appear DIRECTORY by adding a filetype check just > >before setting the string ^^UNAUTHORIZED^^. > > Very slick... I see lock icons popping up on my own sites really soon :-) All the real work was done by whoever coded for ^^DIRECTORY^^ and ^^BLANKICON^^ -- once I was fiddling rr->filename, that bit came as a freebie. But it's a nice one. > >It explicitly hides the file size and modification time of unauthorized > >resources. This differs from the behaviour of 1.3. Code already in > >find_title() ensures that IndexOptions ScanHTMLTitles won't reveal any > >content. > > I'm asking myself what it matters? If they want to include these resources > in the file list, why do we care that they show up without size/time stamps? > I suspect that working around this is overkill. My take would be that advertising the name of the resource will allow someone with the right credentials to follow links to get the full information; someone without the credentials doesn't need to know anything extra. If they really want to know, they can HEAD with the right username:password and be happy. It's a slightly more open interpretation of the "reveal nothing" philosophy that removed 400-series statuses from the listing in the first place, without quite being "reveal lots and lots" The final call is up to the person doing the committing, of course. All the best, f -- Francis Daly[EMAIL PROTECTED]
[PATCH] mod_autoindex and authorization [repost]
This is a repost of a patch sent in the thread "An unusual request"
about a week ago.
Between 1.3 and 2.0, the behaviour of mod_autoindex changed such that
URLs for which the requester was not (yet) authorized did not appear
in the generated listings. This patch allows the administrator
configure, on a per-directory basis, whether or not to show the names
of the authorization-requiring resources in that directory.
This patch introduces a config option which changes the
behaviour of Options +Indexes. It potentially exposes names of
authentication-requiring URLs to unauthenticated users. I've called
the option "IndexOptions RevealSecretURL" to make sure that it isn't
unintentionally enabled. It defaults to not set, which leaves behaviour
as it currently is.
It introduces a fake filename "^^UNAUTHORIZED^^" which can be used by
AddIcon and AddAlt to enhance the display if IndexOptions FancyIndexing
is also set, mirroring ^^DIRECTORY^^ and ^^BLANKICON^^. An UNAUTHORIZED
DIRECTORY will appear UNAUTHORIZED, falling back to DefaultIcon. That
could be changed to appear DIRECTORY by adding a filetype check just
before setting the string ^^UNAUTHORIZED^^.
It explicitly hides the file size and modification time of unauthorized
resources. This differs from the behaviour of 1.3. Code already in
find_title() ensures that IndexOptions ScanHTMLTitles won't reveal any
content.
Arguably, it should require AllowOverride AuthConfig too for use in
.htaccess, although that may need a new directive rather than a new
option to an existing directive.
===
Docs for the IndexOptions RevealSecretURL option:
set or unset on a per-directory basis, just like the rest of
IndexOptions. Default unset overall.
If set, URLs for which valid authentication credentials have not
been presented will appear in autoindex-generated lists of directory
contents.
"^^UNAUTHORIZED^^" can be used as a filename for AddIcon or AddAlt,
if the default choices are inappropriate.
It's only useful in directories where only some files require
authentication; it will reveal to unauthenticated clients the names
of urls that require authentication. However, it also allows Options
+Indexes to work more like it used to in 1.3.
I'm sure someone with more imagination can come up with a better option
name.
Built and tested against the version of mod_autoindex released with
httpd-2.0.35, it applies cleanly to the version released with 2.0.36,
and also to the current version in CVS.
f
--
Francis Daly[EMAIL PROTECTED]
--- modules/generators/mod_autoindex.c Fri Apr 5 18:50:37 2002
+++ modules/generators/mod_autoindex.c.new Thu May 16 22:36:38 2002
@@ -110,6 +110,7 @@
#define FANCY_INDEXING 0x2000
#define TABLE_INDEXING 0x4000
#define IGNORE_CLIENT 0x8000
+#define REVEAL_401 0x1
#define K_NOADJUST 0
#define K_ADJUST 1
@@ -407,6 +408,9 @@
else if (!strcasecmp(w, "VersionSort")) {
option = VERSION_SORT;
}
+else if (!strcasecmp(w, "RevealSecretURL")) {
+option = REVEAL_401;
+}
else if (!strcasecmp(w, "None")) {
if (action != '\0') {
return "Cannot combine '+' or '-' with 'None' keyword";
@@ -1316,7 +1320,9 @@
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))) {
+ || ap_is_HTTP_REDIRECT(rr->status)
+ || ( rr->status == HTTP_UNAUTHORIZED
+ && (autoindex_opts & REVEAL_401) ))) {
ap_destroy_sub_req(rr);
return (NULL);
}
@@ -1337,6 +1343,13 @@
p->key = apr_toupper(keyid);
p->ascending = (apr_toupper(direction) == D_ASCENDING);
p->version_sort = !!(autoindex_opts & VERSION_SORT);
+
+/* Now hide bits that don't need to be revealed */
+if (rr->status == HTTP_UNAUTHORIZED) {
+rr->finfo.mtime = -1;
+rr->finfo.size = -1;
+rr->filename = "^^UNAUTHORIZED^^";
+}
if (autoindex_opts & (FANCY_INDEXING | TABLE_INDEXING)) {
p->lm = rr->finfo.mtime;
[repost] [PATCH] mod_negotiation and authorization
Hi there,
This is a repost of an earlier mail. The patch has been edited to
rename the directive (from "expose" to "reveal") and to rename the
#define'd macros to avoid namespace invasion.
This patch introduces a config directive which changes the behaviour of
MultiViews. It potentially exposes names of authentication-requiring
URLs to unauthenticated users. I've called the directive
"MultiviewsRevealSecretURL" to make sure that it isn't unintentionally
enabled.
My specific intention was to be able to require authentication for
the file "thing.cgi" while advertising the url "thing", without
requiring authentication for the directory in which "thing.cgi"
resided. MultiViews doesn't work the way I want it to in directories
in which only some files require authentication. I know that there are
sound security reasons for that behaviour. I was looking for a way to
selectively override the behaviour.
Incidentally, the non-code-changing workaround I have been using is
renaming "thing.cgi" to "thing" and using "SetHandler cgi-script"
in a "" block. I know I'm abusing MultiViews for
"extension-hiding" rather than "content negotiation", but I suspect I'm
not the only person doing that.
===
Docs for the MultiviewsRevealSecretURL directive:
set On or Off on a per-directory basis. Default Off overall. If unset,
inherit the value from the parent directory. AllowOverride AuthConfig to
use in .htaccess.
A MultiViews request which would otherwise fail 404 not found can
instead fail 401 authorization required, if there is a filename which
might suit the request when the correct credentials are provided. The
subsequent authenticated request can succeed 200 or 300, or can fail
406 or 404, or could persistently fail 401. The last two failures might
appear surprising.
It's only useful in directories where only some files require
authentication; it will reveal to unauthenticated clients that the url
they requested may match one requiring authentication. However, it also
allows MultiViews to work more like I expect it to in such directories.
One thing I'm not certain about is that I've got the best
option-handling code. If the patch is considered worthwhile, I'd suggest
that someone with more experience there take a look to see if there
isn't a better way to achieve the aims in the first paragraph of the
docs.
The directive name itself wouldn't say no to a change, either.
Built against the version of mod_negotiation released with httpd-2.0.36.
f
--
Francis Daly[EMAIL PROTECTED]
--- modules/mappers/mod_negotiation.c Fri Mar 29 08:17:23 2002
+++ modules/mappers/mod_negotiation.c.new Thu May 9 09:05:11 2002
@@ -88,10 +88,17 @@
*/
typedef struct {
+int reveal_secret_url;
int forcelangpriority;
apr_array_header_t *language_priority;
} neg_dir_config;
+/* reveal_secret_url flags
+ */
+#define RSU_UNDEF2/* this means "no explicit config" */
+#define RSU_ON 1/* "config on" */
+#define RSU_OFF 0/* "config off" */
+
/* forcelangpriority flags
*/
#define FLP_UNDEF0/* Same as FLP_DEFAULT, but base overrides */
@@ -107,6 +114,7 @@
{
neg_dir_config *new = (neg_dir_config *) apr_palloc(p, sizeof(neg_dir_config));
+new->reveal_secret_url = RSU_UNDEF;
new->forcelangpriority = FLP_UNDEF;
new->language_priority = NULL;
return new;
@@ -119,6 +127,9 @@
neg_dir_config *new = (neg_dir_config *) apr_palloc(p, sizeof(neg_dir_config));
/* give priority to the config in the subdirectory */
+new->reveal_secret_url = (add->reveal_secret_url != RSU_UNDEF)
+ ? add->reveal_secret_url
+: base->reveal_secret_url;
new->forcelangpriority = (add->forcelangpriority != FLP_UNDEF)
? add->forcelangpriority
: base->forcelangpriority;
@@ -128,6 +139,22 @@
return new;
}
+static const char *reveal_secret_url(cmd_parms *cmd, void *n_, int arg)
+{
+neg_dir_config *n = n_;
+const char *err = ap_check_cmd_context(cmd, NOT_IN_FILES);
+
+if (err != NULL) {
+return err;
+}
+n->reveal_secret_url = arg == RSU_OFF ? RSU_OFF : RSU_ON;
+/* that is functionally equivalent to
+n->reveal_secret_url = arg != 0;
+ for the RSU_* values #defined'd above. Clarity vs efficiency?
+*/
+return NULL;
+}
+
static const char *set_language_priority(cmd_parms *cmd, void *n_,
const char *lang)
{
@@ -188,6 +215,8 @@
{
AP_INIT_FLAG("CacheNegotiatedDocs", cache_negotiated_docs, NULL, RSRC_CONF,
&q
Re: An unusual request [PATCH] mod_autoindex
On Wed, May 15, 2002 at 09:40:56PM -0400, Cliff Woolley wrote:
> On Wed, 15 May 2002, William A. Rowe, Jr. wrote:
>
> >I think this is a good direction. What about IndexResults [with a
> > default of 2xx/3xx]? Shorthand would be #xx for a group [so you could
> > include 2xx 3xx 4xx], or explicit response codes, say 200 only.
>
> +1. I never liked that we arbitrarily took away the 4xx's, especially
> 401. It should be up to the admin.
Vaguely related to this, the appended patch lets the admin allow 401s
through. I don't think may other 400-series errors want to passed
through at all?
Anyway, for your consideration:
This patch introduces a config option which changes the
behaviour of Options +Indexes. It potentially exposes names of
authentication-requiring URLs to unauthenticated users. I've called
the option "IndexOptions RevealSecretURL" to make sure that it isn't
unintentionally enabled. It defaults to not set, which leaves behaviour
as it currently is.
This patch does not address the concern raised earlier about the many
stat()s and subrequests made in an autoindex'ed directory containing
directories. I think changing that would require a different design
entirely.
It introduces a fake filename "^^UNAUTHORIZED^^" which can be used by
AddIcon and AddAlt to enhance the display if IndexOptions FancyIndexing
is also set, mirroring ^^DIRECTORY^^ and ^^BLANKICON^^. An UNAUTHORIZED
DIRECTORY will appear UNAUTHORIZED, falling back to DefaultIcon. That
could be changed to appear DIRECTORY by adding a filetype check just
before setting the string ^^UNAUTHORIZED^^.
It explicitly hides the file size and modification time of unauthorized
resources. This differs from the behaviour of 1.3. Code already in
find_title() ensures that IndexOptions ScanHTMLTitles won't reveal any
content.
Arguably, it should require AllowOverride AuthConfig too for use in
.htaccess, although that may need a new directive rather than a new
option to an existing directive.
===
Docs for the IndexOptions RevealSecretURL option:
set or unset on a per-directory basis, just like the rest of
IndexOptions. Default unset overall.
If set, URLs for which valid authentication credentials have not
been presented will appear in autoindex-generated lists of directory
contents.
"^^UNAUTHORIZED^^" can be used as a filename for AddIcon or AddAlt,
if the default choices are inappropriate.
It's only useful in directories where only some files require
authentication; it will reveal to unauthenticated clients the names
of urls that require authentication. However, it also allows Options
+Indexes to work more like it used to in 1.3.
I'm sure someone with more imagination can come up with a better option
name.
Built and tested against the version of mod_autoindex released with
httpd-2.0.35, it applies cleanly to the version released with 2.0.36,
which appears to be the current version in CVS.
f
--
Francis Daly[EMAIL PROTECTED]
--- modules/generators/mod_autoindex.c Fri Apr 5 18:50:37 2002
+++ modules/generators/mod_autoindex.c.new Thu May 16 22:36:38 2002
@@ -110,6 +110,7 @@
#define FANCY_INDEXING 0x2000
#define TABLE_INDEXING 0x4000
#define IGNORE_CLIENT 0x8000
+#define REVEAL_401 0x1
#define K_NOADJUST 0
#define K_ADJUST 1
@@ -407,6 +408,9 @@
else if (!strcasecmp(w, "VersionSort")) {
option = VERSION_SORT;
}
+else if (!strcasecmp(w, "RevealSecretURL")) {
+option = REVEAL_401;
+}
else if (!strcasecmp(w, "None")) {
if (action != '\0') {
return "Cannot combine '+' or '-' with 'None' keyword";
@@ -1316,7 +1320,9 @@
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))) {
+ || ap_is_HTTP_REDIRECT(rr->status)
+ || ( rr->status == HTTP_UNAUTHORIZED
+ && (autoindex_opts & REVEAL_401) ))) {
ap_destroy_sub_req(rr);
return (NULL);
}
@@ -1337,6 +1343,13 @@
p->key = apr_toupper(keyid);
p->ascending = (apr_toupper(direction) == D_ASCENDING);
p->version_sort = !!(autoindex_opts & VERSION_SORT);
+
+/* Now hide bits that don't need to be revealed */
+if (rr->status == HTTP_UNAUTHORIZED) {
+rr->finfo.mtime = -1;
+rr->finfo.size = -1;
+rr->filename = "^^UNAUTHORIZED^^";
+}
if (autoindex_opts & (FANCY_INDEXING | TABLE_INDEXING)) {
p->lm = rr->finfo.mtime;
Re: [PATCH] distinguishing 64-bit vs. 32-bit in httpd -v
On Tue, Dec 04, 2001 at 01:36:25PM -0500, Jeff Trawick wrote:
> Francis Daly <[EMAIL PROTECTED]> writes:
>
> > 8 is a magic number. How about
> >
> >printf("Pointer size: %d bits\n", CHAR_BIT*sizeof(void*)); ?
> >
> > to allow for any machine which has some other number of bits in a
> > (C-)byte.
>
> So what is more likely?
>
> a) 8 * sizeof(void *) breaks because there aren't 8 bits in a byte
Possible in C. I don't know how likely it is in current reality.
This presumes that the point of the question is "how many bits is a
void pointer", of course.
> b) CHAR_BIT * sizeof(void *) breaks because while there may be 8 bits
>in a byte a char is two bytes
If I understand it correctly, that isn't possible in C. In C, a char
is one byte, which comprises CHAR_BIT bits (and CHAR_BIT is at least
8).
In common usage in the real world, "byte" is the same as "octet",
which is "8 bits"; but in C, "byte" is defined in terms of "sizeof
(char)", which is 1.
> c) CHAR_BIT * sizeof(void *) breaks because CHAR_BIT isn't defined or
> doesn't exist
If doesn't exist, or CHAR_BIT isn't defined in it, then
we're not in C any more either.
ISO/ANSI, that is. Are there other variants of compilers that
currently compile the code?
> :)
>
> (I don't mean to say anything bad about your suggestion.)
The only plus points I can come up with, are that it's a compile-time
constant, so has no performance impact. And it's right ;-)
f
--
Francis Daly[EMAIL PROTECTED]
Re: [PATCH] distinguishing 64-bit vs. 32-bit in httpd -v
On Tue, Dec 04, 2001 at 09:46:21AM -0500, Jeff Trawick wrote:
> Brian Pane <[EMAIL PROTECTED]> writes:
>
> > >+printf("Pointer size: %d\n", sizeof(void *));
> >
> > How about
> >printf("Pointer size: %d bits\n", 8*sizeof(void*)); ?
> > to make it a bit less subtle?
>
> Yessir, that is a big improvement. If nobody comes up with a better
> scheme or expresses a preference for an APR symbol then I'll go with
> that.
8 is a magic number. How about
printf("Pointer size: %d bits\n", CHAR_BIT*sizeof(void*)); ?
to allow for any machine which has some other number of bits in a
(C-)byte.
#include
would be needed somewhere too, if it's not already there.
f
--
Francis Daly[EMAIL PROTECTED]
Re: [PATCH] mod_negotiation, suffix order
On Tue, Oct 02, 2001 at 08:33:21PM -0500, William A. Rowe, Jr. wrote:
> I am very impressed by this idea for Apache 2.0. But I don't like the
> many to many mapping. If we change your underlying rule here to require
> that each filename extension is passed in sequence, I would be _very_
> happy to commit this patch :) E.g. index.en _could_ match index.html.en.
> But index.en.html would _not_ match index.html.en.
By that, I take it you mean something like a requirement (5), or
perhaps (3b), to be added to the description below, along the lines of
(3b)"each .suffix in r->filename must exist in the real filename, in
the same sequence as they were in r->filename"? (r->filename here
means "the bit of it after the final /")
>> The requirements are (1)r->filename up to the first dot must match the
>> real filename up to the first dot; (2)r->filename may not be longer
>> than the real filename; (3)each .suffix in r->filename must exist
>> (string match) in the real filename; (4)the real filename must
>> correspond to a known mime-type, encoding, etc -- which I think means
>> that the final suffix must be known, and only suffixes followed by
>> known suffixes are considered.
[ I note that others feel that (1) above should be replaced with
something more like "all of r->filename must match the start of the
real filename", which would make the remainder of this mail
irrelevant. I'll continue anyway, but feel free to bin it if this is
the Wrong Thing ]
In case my interpretation of (3b) is unclear: as a (hopefully)
complete example, given the file "name.a.b.x.cd.e.f", and presuming
that "x" is the only suffix which is _not_ a recognised mime extension
(type, language, encoding, whatever) then which of the following
requests should be accepted, and which not?
name
name.a
name.a.b
name.a.c
name.a.cd
name.b.c.e
name.b.x.e.f
name.e
name.x.c
name.x.f
name.a.b...f
name.b.a
name.a.b.f.c
name.x.b
name.c.x
name.f.e
name.e.
name.e.e
name.f.
(my understanding is that the first group should all be passed down as
possibilities, the second group shouldn't, and the third group could
be anything. I'd plump for "yes" for all three, probably.)
And for extra fun, which would be different if the file were called
"name.a.b.x.cd.e.f.e".
(my understanding is that the third group definitely becomes "yes",
name.f.e from the second group becomes "yes", and the rest stay as
they were.)
As mentioned in the earlier mail, this patch just decides whether or
not to allow the file as a possibility -- later code gets a shot at
deciding how to handle the suffixes, so if any of the trailing
not-explicitly-listed-in-r->filename suffixes aren't actually
recognised, the only way to get the file would be to request it
by the full name, and therefore bypass mod_negotiation.
For the specific example above, this means that the only requests that
would actually return the file would be name.b.x.e.f, name.x.c, and
name.x.f
The change to the patch to limit the matches as described above is
mostly straightforward -- instead of starting each strstr() at the
start of "name", start it at the point of the previous match (either
the start or the end -- it'd presumably make a difference if someone
requests "file.html.html").
A new const char * which points into dirent.name is the only addition
over the previous patch. However, unless someone has a
case-insensitive strstr() lying around, the CASE_BLIND_FILESYSTEM
cases won't work sensibly -- the "name" part would match
insensitively, but each suffix won't.
I'm including the reworked patch below, in case it's considered
useful. Written and somewhat tested against mod_negotiation.c from
httpd-2.0.25; it applies cleanly to CVS version 1.84.
f
--
Francis Daly[EMAIL PROTECTED]
=
--- mod_negotiation.c.orig Tue Aug 28 04:08:31 2001
+++ mod_negotiation.c Wed Oct 3 21:44:12 2001
@@ -1019,6 +1019,11 @@
struct var_rec mime_info;
struct accept_rec accept_info;
void *new_var;
+char *pos;
+int pos_len;
+int not_this_dirent;/* actually, boolean. */
+int dots_in_request = 0;/* 1 == one dot, 2 == some dots */
+const char *dpos; /* points into the dirent.name */
int anymatch = 0;
clean_var_rec(&mime_info);
@@ -1041,20 +1046,92 @@
return HTTP_FORBIDDEN;
}
+if ((pos = strchr(filp, '.'))) {
+dots_in_request = 1;
+if (strchr(++pos, '.')) {
+dots_in_request = 2;
+}
+}
+
while (apr_dir_read(&dirent, APR_FINFO_DIRENT, dirp) == APR_SUCCESS) {
apr_array_header_t *exception_list;
request_rec *sub_req;
-/* Do we hav
