You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2022/01/17 16:10:51 UTC

svn commit: r1897156 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number docs/manual/mod/core.xml modules/dav/main/mod_dav.c server/core.c

Author: minfrin
Date: Mon Jan 17 16:10:51 2022
New Revision: 1897156

URL: http://svn.apache.org/viewvc?rev=1897156&view=rev
Log:
core: Allow an optional expression to be specified for an effective
path in the DirectoryMatch and LocationMatch directives. This allows
modules like mod_dav to map URLs to URL spaces or to directories on
the filesystem.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/docs/log-message-tags/next-number
    httpd/httpd/trunk/docs/manual/mod/core.xml
    httpd/httpd/trunk/modules/dav/main/mod_dav.c
    httpd/httpd/trunk/server/core.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1897156&r1=1897155&r2=1897156&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Mon Jan 17 16:10:51 2022
@@ -1,6 +1,11 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.1
 
+  *) core: Allow an optional expression to be specified for an effective
+     path in the DirectoryMatch and LocationMatch directives. This allows
+     modules like mod_dav to map URLs to URL spaces or to directories on
+     the filesystem. [Graham Leggett]
+
   *) http: Enforce that fully qualified uri-paths not to be forward-proxied
      have an http(s) scheme, and that the ones to be forward proxied have a
      hostname, per HTTP specifications.  [Ruediger Pluem, Yann Ylavic]

Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1897156&r1=1897155&r2=1897156&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
+++ httpd/httpd/trunk/docs/log-message-tags/next-number Mon Jan 17 16:10:51 2022
@@ -1 +1 @@
-10367
+10369

Modified: httpd/httpd/trunk/docs/manual/mod/core.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/core.xml?rev=1897156&r1=1897155&r2=1897156&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/core.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/core.xml Mon Jan 17 16:10:51 2022
@@ -926,6 +926,20 @@ named file-system directory, sub-directo
     the corresponding <directive type="section">Directory</directive> will
     be applied.</p>
 
+    <p>Some modules require the directory-path prefix in order to do their work,
+    and when a regular expression is provided the directory-path is no longer
+    available. From 2.5.1 onwards, an expression can be specified in addition
+    to the regular expression that resolves to the directory-path prefix. This
+    can allow complex mappings from the URL space to an effective directory.
+    This funcionality is identical to that provided by the
+    <directive>DirectoryMatch</directive> directive below.</p>
+
+    <highlight language="config">
+&lt;Directory ~ /home/%{env:MATCH_PARTITIONNAME}/dav/ ^/dav/(?&lt;PARTITIONNAME&gt;[^/]+)/&gt;
+    Dav on
+&lt;/Directory&gt;
+    </highlight>
+
    <p><strong>Note that the default access for
     <code>&lt;Directory "/"&gt;</code> is to permit all access.
     This means that Apache httpd will serve any file mapped from an URL. It is
@@ -959,7 +973,7 @@ named file-system directory, sub-directo
 <name>DirectoryMatch</name>
 <description>Enclose directives that apply to
 the contents of file-system directories matching a regular expression.</description>
-<syntax>&lt;DirectoryMatch <var>regex</var>&gt;
+<syntax>&lt;DirectoryMatch [<var>expr</var>] <var>regex</var>&gt;
 ... &lt;/DirectoryMatch&gt;</syntax>
 <contextlist><context>server config</context><context>virtual host</context>
 </contextlist>
@@ -1007,6 +1021,18 @@ the contents of file-system directories
     Require ldap-group cn=%{env:MATCH_SITENAME},ou=combined,o=Example
 &lt;/DirectoryMatch&gt;
     </highlight>
+
+    <p>Some modules require the directory-path prefix in order to do their work,
+    and when a regular expression is provided the directory-path is no longer
+    available. From 2.5.1 onwards, an expression can be specified in addition
+    to the regular expression that resolves to the directory-path prefix. This
+    can allow complex mappings from the URL space to an effective directory.</p>
+
+    <highlight language="config">
+&lt;DirectoryMatch /home/%{env:MATCH_PARTITIONNAME}/dav/ ^/dav/(?&lt;PARTITIONNAME&gt;[^/]+)/&gt;
+    Dav on
+&lt;/DirectoryMatch&gt;
+    </highlight>
 </usage>
 <seealso><directive type="section" module="core">Directory</directive> for
 a description of how regular expressions are mixed in with normal
@@ -3172,6 +3198,19 @@ URLs</description>
 &lt;/Location&gt;
     </highlight>
 
+    <p>Some modules require the URL-path prefix in order to do their work, and when
+    a regular expression is provided the URL-path is no longer available. From
+    2.5.1 onwards, an expression can be specified in addition to the regular
+    expression that resolves to the URL-path prefix. This can allow complex
+    mappings from the URL space to an effective path. This funcionality is identical
+    to that provided by the <directive>LocationMatch</directive> directive below.</p>
+
+    <highlight language="config">
+&lt;Location ~ /dav/%{env:MATCH_PARTITIONNAME} ^/dav/(?&lt;PARTITIONNAME&gt;[^/]+)/&gt;
+    Dav on
+&lt;/Location&gt;
+    </highlight>
+
     <note><title>Note about / (slash)</title>
       <p>The slash character has special meaning depending on where in a
       URL it appears. People may be used to its behavior in the filesystem
@@ -3207,7 +3246,7 @@ URLs</description>
 <description>Applies the enclosed directives only to regular-expression
 matching URLs</description>
 <syntax>&lt;LocationMatch
-    <var>regex</var>&gt; ... &lt;/LocationMatch&gt;</syntax>
+    [<var>expr</var>] <var>regex</var>&gt; ... &lt;/LocationMatch&gt;</syntax>
 <contextlist><context>server config</context><context>virtual host</context>
 </contextlist>
 
@@ -3250,6 +3289,18 @@ matching URLs</description>
 &lt;/LocationMatch&gt;
     </highlight>
 
+    <p>Some modules require the URL-path prefix in order to do their work, and when
+    a regular expression is provided the URL-path is no longer available. From
+    2.5.1 onwards, an expression can be specified in addition to the regular
+    expression that resolves to the URL-path prefix. This can allow complex
+    mappings from the URL space to an effective path.</p>
+
+    <highlight language="config">
+&lt;LocationMatch /dav/%{env:MATCH_PARTITIONNAME} ^/dav/(?&lt;PARTITIONNAME&gt;[^/]+)/&gt;
+    Dav on
+&lt;/LocationMatch&gt;
+    </highlight>
+
      <note><title>Note about / (slash)</title>
       <p>The slash character has special meaning depending on where in a
       URL it appears. People may be used to its behavior in the filesystem

Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.c?rev=1897156&r1=1897155&r2=1897156&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/dav/main/mod_dav.c (original)
+++ httpd/httpd/trunk/modules/dav/main/mod_dav.c Mon Jan 17 16:10:51 2022
@@ -83,6 +83,7 @@ typedef struct {
     const char *dir;
     int locktimeout;
     int allow_depthinfinity;
+    const ap_expr_info_t *dir_expr;
 
 } dav_dir_conf;
 
@@ -203,6 +204,7 @@ static void *dav_merge_dir_config(apr_po
     newconf->dir = DAV_INHERIT_VALUE(parent, child, dir);
     newconf->allow_depthinfinity = DAV_INHERIT_VALUE(parent, child,
                                                      allow_depthinfinity);
+    newconf->dir_expr = DAV_INHERIT_VALUE(parent, child, dir_expr);
 
     return newconf;
 }
@@ -282,6 +284,18 @@ static const char *dav_cmd_dav(cmd_parms
         }
     }
 
+    if (!conf->dir_expr) {
+        const char *expr_err = NULL;
+
+        conf->dir_expr = ap_expr_parse_cmd(cmd, conf->dir, AP_EXPR_FLAG_STRING_RESULT,
+                &expr_err, NULL);
+        if (expr_err) {
+            return apr_pstrcat(cmd->temp_pool,
+                    "Cannot parse Directory/Location expression '", conf->dir, "': ",
+                    expr_err, NULL);
+        }
+    }
+
     return NULL;
 }
 
@@ -723,6 +737,57 @@ static int dav_get_overwrite(request_rec
     return -1;
 }
 
+static int uripath_is_canonical(const char *uripath)
+{
+    const char *dot_pos, *ptr = uripath;
+    apr_size_t i, len;
+    unsigned pattern = 0;
+
+    /* URIPATH is canonical if it has:
+     *  - no '.' segments
+     *  - no closing '/'
+     *  - no '//'
+     */
+
+    if (ptr[0] == '.'
+            && (ptr[1] == '/' || ptr[1] == '\0'
+                    || (ptr[1] == '.' && (ptr[2] == '/' || ptr[2] == '\0')))) {
+        return 0;
+    }
+
+    /* valid special cases */
+    len = strlen(ptr);
+    if (len < 2) {
+        return 1;
+    }
+
+    /* invalid endings */
+    if (ptr[len - 1] == '/' || (ptr[len - 1] == '.' && ptr[len - 2] == '/')) {
+        return 0;
+    }
+
+    /* '.' are rare. So, search for them globally. There will often be no
+     * more than one hit.  Also note that we already checked for invalid
+     * starts and endings, i.e. we only need to check for "/./"
+     */
+    for (dot_pos = memchr(ptr, '.', len); dot_pos;
+            dot_pos = strchr(dot_pos + 1, '.')) {
+        if (dot_pos > ptr && dot_pos[-1] == '/' && dot_pos[1] == '/') {
+            return 0;
+        }
+    }
+
+    /* Now validate the rest of the path. */
+    for (i = 0; i < len - 1; ++i) {
+        pattern = ((pattern & 0xff) << 8) + (unsigned char) ptr[i];
+        if (pattern == 0x101 * (unsigned char) ('/')) {
+            return 0;
+        }
+    }
+
+    return 1;
+}
+
 /* resolve a request URI to a resource descriptor.
  *
  * If label_allowed != 0, then allow the request target to be altered by
@@ -737,6 +802,7 @@ DAV_DECLARE(dav_error *) dav_get_resourc
 {
     dav_dir_conf *conf;
     const char *label = NULL;
+    const char *dir;
     dav_error *err;
 
     /* if the request target can be overridden, get any target selector */
@@ -753,9 +819,34 @@ DAV_DECLARE(dav_error *) dav_get_resourc
                              ap_escape_html(r->pool, r->uri)));
     }
 
+    if (conf->dir_expr) {
+        const char *err = NULL;
+
+        dir = ap_expr_str_exec(r, conf->dir_expr, &err);
+        if (err) {
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10367)
+                    "Director/Location expression '%s' could not be parsed: %s", conf->dir, err);
+            return dav_new_error(r->pool, HTTP_INTERNAL_SERVER_ERROR, 0, 0,
+                                 apr_psprintf(r->pool,
+                                 "Directory/Location expression could not be parsed: %s", err));
+        }
+
+        /* safety check - is our path canonical? */
+        if (!uripath_is_canonical(dir)) {
+            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(10368)
+                    "Directory/Location is not canonical ('.', '..' and '//' not allowed): %s", dir);
+            return dav_new_error(r->pool, HTTP_BAD_REQUEST, 0, 0,
+                    apr_psprintf(r->pool, "Directory/Location is not canonical for: %s",
+                            ap_escape_html(r->pool, r->uri)));
+        }
+
+    }
+    else {
+        dir = conf->dir;
+    }
+
     /* resolve the resource */
-    err = (*conf->provider->repos->get_resource)(r, conf->dir,
-                                                 label, use_checked_in,
+    err = (*conf->provider->repos->get_resource)(r, dir, label, use_checked_in,
                                                  res_p);
     if (err != NULL) {
         err = dav_push_error(r->pool, err->status, 0,

Modified: httpd/httpd/trunk/server/core.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=1897156&r1=1897155&r2=1897156&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core.c (original)
+++ httpd/httpd/trunk/server/core.c Mon Jan 17 16:10:51 2022
@@ -2506,6 +2506,7 @@ static const char *dirsection(cmd_parms
     char *old_path = cmd->path;
     core_dir_config *conf;
     ap_conf_vector_t *new_dir_conf = ap_create_per_dir_config(cmd->pool);
+    const char *regex;
     ap_regex_t *r = NULL;
     const command_rec *thiscmd = cmd->cmd;
 
@@ -2529,15 +2530,20 @@ static const char *dirsection(cmd_parms
 
     if (!strcmp(cmd->path, "~")) {
         cmd->path = ap_getword_conf(cmd->pool, &arg);
-        if (!cmd->path)
-            return "<Directory ~ > block must specify a path";
-        r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
+        if (!*cmd->path) {
+            return "<Directory ~ > block must specify a regex";
+        }
+        regex = ap_getword_conf(cmd->pool, &arg);
+        r = ap_pregcomp(cmd->pool, *regex ? regex : cmd->path,
+                AP_REG_EXTENDED | USE_ICASE);
         if (!r) {
             return "Regex could not be compiled";
         }
     }
     else if (thiscmd->cmd_data) { /* <DirectoryMatch> */
-        r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
+        regex = ap_getword_conf(cmd->pool, &arg);
+        r = ap_pregcomp(cmd->pool, *regex ? regex : cmd->path,
+                AP_REG_EXTENDED | USE_ICASE);
         if (!r) {
             return "Regex could not be compiled";
         }
@@ -2599,8 +2605,8 @@ static const char *dirsection(cmd_parms
     ap_add_per_dir_conf(cmd->server, new_dir_conf);
 
     if (*arg != '\0') {
-        return apr_pstrcat(cmd->pool, "Multiple ", thiscmd->name,
-                           "> arguments not (yet) supported.", NULL);
+        return apr_pstrcat(cmd->pool, "Additional ", thiscmd->name,
+                           "> arguments not (yet) supported: ", arg, NULL);
     }
 
     cmd->path = old_path;
@@ -2616,6 +2622,7 @@ static const char *urlsection(cmd_parms
     int old_overrides = cmd->override;
     char *old_path = cmd->path;
     core_dir_config *conf;
+    const char *regex;
     ap_regex_t *r = NULL;
     const command_rec *thiscmd = cmd->cmd;
     ap_conf_vector_t *new_url_conf = ap_create_per_dir_config(cmd->pool);
@@ -2638,14 +2645,21 @@ static const char *urlsection(cmd_parms
     cmd->override = OR_ALL|ACCESS_CONF;
 
     if (thiscmd->cmd_data) { /* <LocationMatch> */
-        r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED);
+        regex = ap_getword_conf(cmd->pool, &arg);
+        r = ap_pregcomp(cmd->pool, *regex ? regex : cmd->path,
+                AP_REG_EXTENDED);
         if (!r) {
             return "Regex could not be compiled";
         }
     }
     else if (!strcmp(cmd->path, "~")) {
         cmd->path = ap_getword_conf(cmd->pool, &arg);
-        r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED);
+        if (!*cmd->path) {
+            return "<Location ~ > block must specify a regex";
+        }
+        regex = ap_getword_conf(cmd->pool, &arg);
+        r = ap_pregcomp(cmd->pool, *regex ? regex : cmd->path,
+                AP_REG_EXTENDED);
         if (!r) {
             return "Regex could not be compiled";
         }
@@ -2671,8 +2685,8 @@ static const char *urlsection(cmd_parms
     ap_add_per_url_conf(cmd->server, new_url_conf);
 
     if (*arg != '\0') {
-        return apr_pstrcat(cmd->pool, "Multiple ", thiscmd->name,
-                           "> arguments not (yet) supported.", NULL);
+        return apr_pstrcat(cmd->pool, "Additional ", thiscmd->name,
+                           "> arguments not (yet) supported: ", arg, NULL);
     }
 
     cmd->path = old_path;
@@ -2688,6 +2702,7 @@ static const char *filesection(cmd_parms
     int old_overrides = cmd->override;
     char *old_path = cmd->path;
     core_dir_config *conf;
+    const char *regex;
     ap_regex_t *r = NULL;
     const command_rec *thiscmd = cmd->cmd;
     ap_conf_vector_t *new_file_conf = ap_create_per_dir_config(cmd->pool);
@@ -2715,14 +2730,18 @@ static const char *filesection(cmd_parms
     }
 
     if (thiscmd->cmd_data) { /* <FilesMatch> */
-        r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
+        regex = ap_getword_conf(cmd->pool, &arg);
+        r = ap_pregcomp(cmd->pool, *regex ? regex : cmd->path,
+                AP_REG_EXTENDED | USE_ICASE);
         if (!r) {
             return "Regex could not be compiled";
         }
     }
     else if (!strcmp(cmd->path, "~")) {
         cmd->path = ap_getword_conf(cmd->pool, &arg);
-        r = ap_pregcomp(cmd->pool, cmd->path, AP_REG_EXTENDED|USE_ICASE);
+        regex = ap_getword_conf(cmd->pool, &arg);
+        r = ap_pregcomp(cmd->pool, *regex ? regex : cmd->path,
+                AP_REG_EXTENDED | USE_ICASE);
         if (!r) {
             return "Regex could not be compiled";
         }
@@ -2758,8 +2777,8 @@ static const char *filesection(cmd_parms
     ap_add_file_conf(cmd->pool, (core_dir_config *)mconfig, new_file_conf);
 
     if (*arg != '\0') {
-        return apr_pstrcat(cmd->pool, "Multiple ", thiscmd->name,
-                           "> arguments not (yet) supported.", NULL);
+        return apr_pstrcat(cmd->pool, "Additional ", thiscmd->name,
+                           "> arguments not (yet) supported: ", arg, NULL);
     }
 
     cmd->path = old_path;
@@ -2845,8 +2864,8 @@ static const char *ifsection(cmd_parms *
         return errmsg;
 
     if (*arg != '\0') {
-        return apr_pstrcat(cmd->pool, "Multiple ", thiscmd->name,
-                           "> arguments not supported.", NULL);
+        return apr_pstrcat(cmd->pool, "Additional ", thiscmd->name,
+                           "> arguments not supported: ", arg, NULL);
     }
 
     cmd->path = old_path;



Re: svn commit: r1897156 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number docs/manual/mod/core.xml modules/dav/main/mod_dav.c server/core.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 18 Jan 2022, at 12:31, Ruediger Pluem <rp...@apache.org> wrote:

> Not sure I get the intention correct, but this looks like a change to core for a mod_dav specific short coming.

It's not specific to mod_dav, no.

I created a patch ages ago for mod_dav_svn that depended on this patch and this got buried under other things, and I have other modules that need the same thing: http://svn.apache.org/viewvc/subversion/branches/mod-dav-svn-expressions/

At the core of the problem is for many modules to work, they need to know the “root” of the URL space, so that a PATH_INFO can be accurately calculated. As soon as you’re in a LocationMatch or DirectoryMatch the root of the URL space is replaced with the regex, and you’re lost.

> e.g. in mod_proxy similar problems are solved for ProxyPass that can be used in and outside Directory / Location elements.

The ProxyPass was originally outside of Directory / Location only, I added the option of the one parameter version of ProxyPass that inherited from the Directory / Location, but this (as I recall) doesn't work for regexes.

It would be great if the two parameter ProxyPass can go away in future to simplify.

> I would be in favor for a similar approach here, by either allowing a second optional parameter to the 'DAV' directive that sets
> this or for a separate directive that allows to set this. In case nothing is set the cmd->path could be used as currently.
> This also allows to use this feature in 'if' sections as well.
> Another option would be to provide this information via an environment variable that can set via SetEnvIfExpr or a RewriteRule.

Hmmmm…

Adding an extra parameter to DAV, and then to something generic like SetHandler may do the trick. This gives most modules access to a sane path when used inside a Match.

Let me think about this for a bit.

Regards,
Graham
—


Re: svn commit: r1897156 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number docs/manual/mod/core.xml modules/dav/main/mod_dav.c server/core.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 1/17/22 5:10 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Mon Jan 17 16:10:51 2022
> New Revision: 1897156
> 
> URL: http://svn.apache.org/viewvc?rev=1897156&view=rev
> Log:
> core: Allow an optional expression to be specified for an effective
> path in the DirectoryMatch and LocationMatch directives. This allows
> modules like mod_dav to map URLs to URL spaces or to directories on
> the filesystem.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/docs/log-message-tags/next-number
>     httpd/httpd/trunk/docs/manual/mod/core.xml
>     httpd/httpd/trunk/modules/dav/main/mod_dav.c
>     httpd/httpd/trunk/server/core.c
> 

Not sure I get the intention correct, but this looks like a change to core for a mod_dav specific short coming.
e.g. in mod_proxy similar problems are solved for ProxyPass that can be used in and outside Directory / Location elements.
I would be in favor for a similar approach here, by either allowing a second optional parameter to the 'DAV' directive that sets
this or for a separate directive that allows to set this. In case nothing is set the cmd->path could be used as currently.
This also allows to use this feature in 'if' sections as well.
Another option would be to provide this information via an environment variable that can set via SetEnvIfExpr or a RewriteRule.

Regards

Rüdiger


Re: svn commit: r1897156 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number docs/manual/mod/core.xml modules/dav/main/mod_dav.c server/core.c

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Jan 17, 2022 at 04:10:51PM -0000, minfrin@apache.org wrote:
> Author: minfrin
> Date: Mon Jan 17 16:10:51 2022
> New Revision: 1897156
> 
> URL: http://svn.apache.org/viewvc?rev=1897156&view=rev
> Log:
> core: Allow an optional expression to be specified for an effective
> path in the DirectoryMatch and LocationMatch directives. This allows
> modules like mod_dav to map URLs to URL spaces or to directories on
> the filesystem.

https://app.travis-ci.com/github/apache/httpd/jobs/555883817#L2039

This has introduced new warnings:

In file included from mod_dav.c:51:
mod_dav.c: In function ‘uripath_is_canonical’:
mod_dav.c:774:38: error: passing argument 1 of ‘ap_strchr’ discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
  774 |             dot_pos = strchr(dot_pos + 1, '.')) {
      |                              ~~~~~~~~^~~
/home/travis/build/apache/httpd/include/httpd.h:2469:34: note: in definition of macro ‘strchr’
 2469 | # define strchr(s, c)  ap_strchr(s,c)
      |                                  ^
/home/travis/build/apache/httpd/include/httpd.h:2457:36: note: expected ‘char *’ but argument is of type ‘const char *’
 2457 | AP_DECLARE(char *) ap_strchr(char *s, int c);
      |                              ~~~~~~^


Re: svn commit: r1897156 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number docs/manual/mod/core.xml modules/dav/main/mod_dav.c server/core.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Jan 17, 2022 at 5:10 PM <mi...@apache.org> wrote:
>
> Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.c?rev=1897156&r1=1897155&r2=1897156&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/dav/main/mod_dav.c (original)
> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.c Mon Jan 17 16:10:51 2022
> @@ -723,6 +737,57 @@ static int dav_get_overwrite(request_rec
>      return -1;
>  }
>
> +static int uripath_is_canonical(const char *uripath)

Maybe reuse ap_normalize_path() for the implementation, like:

static int uripath_is_canonical(apr_pool_t *p, const char *uripath)
{
    char *path = apr_pstrdup(uripath);
    return (ap_normalize_path(path, (AP_NORMALIZE_MERGE_SLASHES |
                                     AP_NORMALIZE_NOT_ABOVE_ROOT))
            && strcmp(uri_path, path) == 0);
}

?


Regards;
Yann.

Re: svn commit: r1897156 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number docs/manual/mod/core.xml modules/dav/main/mod_dav.c server/core.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 19 Jan 2022, at 09:40, Ruediger Pluem <rp...@apache.org> wrote:

>> @@ -723,6 +737,57 @@ static int dav_get_overwrite(request_rec
>>     return -1;
>> }
>> 
>> +static int uripath_is_canonical(const char *uripath)
> 
> Isn't this a filesystem path we are talking about in the <Directory > case?
> How does this function work on Windows when people might use '\' instead of '/'?

This comes from svn at https://github.com/apache/subversion/blob/0f4de5d963ce3b43084f66efab0901be15d2eec2/subversion/libsvn_subr/dirent_uri.c#L1856

If you’ve gone through the expression parser you need a sanity check to verify you’re still canonical. You don;t need to canonicalise, as you;ve done this already.

You would need to do the same step for directories, but canonicalise to a file path.

I am going to look at this again, as you extra-parameter-to-dav is a lot cleaner.

Regards,
Graham
—


Re: svn commit: r1897156 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number docs/manual/mod/core.xml modules/dav/main/mod_dav.c server/core.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 1/17/22 5:10 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Mon Jan 17 16:10:51 2022
> New Revision: 1897156
> 
> URL: http://svn.apache.org/viewvc?rev=1897156&view=rev
> Log:
> core: Allow an optional expression to be specified for an effective
> path in the DirectoryMatch and LocationMatch directives. This allows
> modules like mod_dav to map URLs to URL spaces or to directories on
> the filesystem.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/docs/log-message-tags/next-number
>     httpd/httpd/trunk/docs/manual/mod/core.xml
>     httpd/httpd/trunk/modules/dav/main/mod_dav.c
>     httpd/httpd/trunk/server/core.c
> 
avior in the filesystem
> 
> Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.c?rev=1897156&r1=1897155&r2=1897156&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/dav/main/mod_dav.c (original)
> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.c Mon Jan 17 16:10:51 2022

>  
> @@ -723,6 +737,57 @@ static int dav_get_overwrite(request_rec
>      return -1;
>  }
>  
> +static int uripath_is_canonical(const char *uripath)

Isn't this a filesystem path we are talking about in the <Directory > case?
How does this function work on Windows when people might use '\' instead of '/'?

> +{
> +    const char *dot_pos, *ptr = uripath;
> +    apr_size_t i, len;
> +    unsigned pattern = 0;
> +
> +    /* URIPATH is canonical if it has:
> +     *  - no '.' segments
> +     *  - no closing '/'
> +     *  - no '//'
> +     */
> +
> +    if (ptr[0] == '.'
> +            && (ptr[1] == '/' || ptr[1] == '\0'
> +                    || (ptr[1] == '.' && (ptr[2] == '/' || ptr[2] == '\0')))) {
> +        return 0;
> +    }
> +
> +    /* valid special cases */
> +    len = strlen(ptr);
> +    if (len < 2) {

Empty pathes ("") are ok?

> +        return 1;
> +    }
> +
> +    /* invalid endings */
> +    if (ptr[len - 1] == '/' || (ptr[len - 1] == '.' && ptr[len - 2] == '/')) {
> +        return 0;
> +    }
> +
> +    /* '.' are rare. So, search for them globally. There will often be no
> +     * more than one hit.  Also note that we already checked for invalid
> +     * starts and endings, i.e. we only need to check for "/./"

Why don't we need to look for /../ segments?

> +     */
> +    for (dot_pos = memchr(ptr, '.', len); dot_pos;
> +            dot_pos = strchr(dot_pos + 1, '.')) {
> +        if (dot_pos > ptr && dot_pos[-1] == '/' && dot_pos[1] == '/') {
> +            return 0;
> +        }
> +    }
> +
> +    /* Now validate the rest of the path. */
> +    for (i = 0; i < len - 1; ++i) {
> +        pattern = ((pattern & 0xff) << 8) + (unsigned char) ptr[i];
> +        if (pattern == 0x101 * (unsigned char) ('/')) {
> +            return 0;
> +        }
> +    }
> +
> +    return 1;
> +}
> +
>  /* resolve a request URI to a resource descriptor.
>   *
>   * If label_allowed != 0, then allow the request target to be altered by