You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dan Poirier <po...@pobox.com> on 2010/03/01 14:44:12 UTC
Wildcards internal to Include paths matching files
Re: this proposal:
* core: Support wildcards in both the directory and file components of
the path specified by the Include directive.
Trunk patch: http://svn.apache.org/viewvc?rev=909878&view=rev
2.2.x patch: http://people.apache.org/~minfrin/httpd-wildcard+docs.patch
With the code currently in trunk and proposed for backport, if an
interior wildcard matches something not a directory, it results in a
fatal error. E.g. if I have
Include /foo/conf/*/*.conf
and this file happens to exist
/foo/conf/httpd.conf
then you end up with this error:
httpd: Syntax error on line 472 of /path/to/serverroot/conf/httpd.conf:
Could not open config directory /foo/conf/httpd.conf: Not a directory
I think if the configuration says "Include /foo/conf/*/*.conf", the user
isn't expecting files in /foo/conf to be included, and we should just
skip over them.
If people agree with that behavior, here's a proposed implementation.
Either way, the behavior should be documented. (I can add doc, unless
Graham wants to do it, once the right behavior is agreed on.)
Index: config.c
===================================================================
--- config.c (revision 916378)
+++ config.c (working copy)
@@ -1710,8 +1710,17 @@
&& strcmp(dirent.name, "..")
&& (apr_fnmatch(fname, dirent.name,
APR_FNM_PERIOD) == APR_SUCCESS)) {
+ const char *full_path = ap_make_full_path(ptemp, path, dirent.name);
+ if (rest) {
+ /* If matching internal to path, and we happen to match something other than a directory, skip it */
+ apr_finfo_t finfo;
+ rv = apr_stat(&finfo, full_path, APR_FINFO_TYPE, ptemp);
+ if ((rv == APR_SUCCESS) && (finfo.filetype != APR_DIR)) {
+ continue;
+ }
+ }
fnew = (fnames *) apr_array_push(candidates);
- fnew->fname = ap_make_full_path(ptemp, path, dirent.name);
+ fnew->fname = full_path;
}
}
Re: Wildcards internal to Include paths matching files
Posted by Niklas Edmundsson <ni...@acc.umu.se>.
On Mon, 1 Mar 2010, Graham Leggett wrote:
> On 01 Mar 2010, at 3:44 PM, Dan Poirier wrote:
>
>> I think if the configuration says "Include /foo/conf/*/*.conf", the user
>> isn't expecting files in /foo/conf to be included, and we should just
>> skip over them.
>
> +1, definitely makes sense.
+1, let's avoid confusing our users even more :)
/Nikke
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se | nikke@acc.umu.se
---------------------------------------------------------------------------
I am MODERATOR of BORG. Follow the rules or be assimilated.
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Re: Wildcards internal to Include paths matching files
Posted by Graham Leggett <mi...@sharp.fm>.
On 01 Mar 2010, at 3:44 PM, Dan Poirier wrote:
> I think if the configuration says "Include /foo/conf/*/*.conf", the
> user
> isn't expecting files in /foo/conf to be included, and we should just
> skip over them.
+1, definitely makes sense.
Regards,
Graham
--
Re: Wildcards internal to Include paths matching files
Posted by Graham Leggett <mi...@sharp.fm>.
On 02 Mar 2010, at 12:12 AM, Joe Orton wrote:
> You can get the type by checking dirent.filetype directly, if you add
> APR_FINFO_TYPE to the bitmask passed to apr_dir_read(), this will be
> more efficient on most platforms since it can avoid the stat call.
Doh, forgot the APR_FINFO_TYPE. Fixed in r917759. Thanks for the heads
up.
Regards,
Graham
--
Re: Wildcards internal to Include paths matching files
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Mar 01, 2010 at 08:44:12AM -0500, Dan Poirier wrote:
> --- config.c (revision 916378)
> +++ config.c (working copy)
> @@ -1710,8 +1710,17 @@
> && strcmp(dirent.name, "..")
> && (apr_fnmatch(fname, dirent.name,
> APR_FNM_PERIOD) == APR_SUCCESS)) {
> + const char *full_path = ap_make_full_path(ptemp, path, dirent.name);
> + if (rest) {
> + /* If matching internal to path, and we happen to match something other than a directory, skip it */
> + apr_finfo_t finfo;
> + rv = apr_stat(&finfo, full_path, APR_FINFO_TYPE, ptemp);
> + if ((rv == APR_SUCCESS) && (finfo.filetype != APR_DIR)) {
You can get the type by checking dirent.filetype directly, if you add
APR_FINFO_TYPE to the bitmask passed to apr_dir_read(), this will be
more efficient on most platforms since it can avoid the stat call.
Regards, Joe