[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: PATCH: simple regexp support in mod_authz_svn

From: Branko Čibej <brane_at_xbc.nu>
Date: 2005-02-17 21:06:12 CET

Josh Siegel wrote:

>
> These patches give you both simple regular expression support (unix
> shell like) in mod_authz_svn as well as the ability to make all string
> comparisons case insensitive. You turn on regular expression support
> by setting AuthzSVNRegexp in your apache.conf file. Setting
> AuthzSVNCaseInsensitiveNameMatches to On makes all checks case
> insensitive.
> Once you've done this, you can do things like
>
> [foo:/tags/*/bar]
> @group1 = r
>
> [foo:/branches/*/bar]
> @group1 = rw
>
> Currently, the only shell regexp's it supports are * and ?. It does
> not support {a,b,c}
>
> Regards,
> Josh Siegel
>
>------------------------------------------------------------------------
>
>Index: subversion/mod_authz_svn/mod_authz_svn.c
>===================================================================
>--- subversion/mod_authz_svn/mod_authz_svn.c (revision 13038)
>+++ subversion/mod_authz_svn/mod_authz_svn.c (working copy)
>@@ -27,6 +27,7 @@
> #include <http_log.h>
> #include <ap_config.h>
> #include <apr_uri.h>
>+#include <apr_hash.h>
> #include <mod_dav.h>
>
> #include "mod_dav_svn.h"
>@@ -47,6 +48,8 @@
> typedef struct {
> int authoritative;
> int anonymous;
>+ int regexp;
>
>
The way you describe this, it's not regexp matching but wildcard matching.

>+ int caseInsensitiveNameMatches;
>
>
Follow Subversion naming conventions, please. This name is way too long,
and shouldn't contain uppercase letters. I suggest "case_folding", or
just "fold_case".

>@@ -105,7 +118,7 @@
> * Access checking
> */
>
>-static int group_contains_user_internal(svn_config_t *cfg,
>+static int group_contains_user_internal(svn_config_t *cfg, const int caseInsensitive,
>
>
Don't declare "const int" parameters. This is a call-by-value, so the
caller doesn't care about the const anyway. again, the name of the
parameter doesn't conform; "fold_case" woudl do here, too.

>@@ -204,12 +218,18 @@
>
> baton.pool = pool;
> baton.config = cfg;
>+ baton.aconfig = acfg;
> baton.user = user;
>
> /* First try repos specific */
> qualified_repos_path = apr_pstrcat(pool, repos_name, ":", repos_path,
> NULL);
>- svn_config_enumerate(cfg, qualified_repos_path,
>+
>+ if (acfg->regexp)
>+ svn_config_enumerate_regexp(cfg, qualified_repos_path,
>+ acfg->caseInsensitiveNameMatches ? 0x5 : 0x4, parse_authz_line, &baton);
>
>
Hello? Whata are these magic numbers doing here?

>+ else
>+ svn_config_enumerate(cfg, qualified_repos_path,
> parse_authz_line, &baton);
> *granted_access = !(baton.deny & required_access)
> || (baton.allow & required_access);
>@@ -218,7 +238,11 @@
> || (baton.allow & required_access))
> return TRUE;
>
>- svn_config_enumerate(cfg, repos_path,
>+ if (acfg->regexp)
>+ svn_config_enumerate_regexp(cfg, repos_path,
>+ acfg->caseInsensitiveNameMatches ? 0x5 : 0x4, parse_authz_line, &baton);
>
>
and here?

>Index: subversion/include/svn_config.h
>===================================================================
>--- subversion/include/svn_config.h (revision 13038)
>+++ subversion/include/svn_config.h (working copy)
>@@ -235,6 +235,43 @@
> svn_config_enumerator_t callback, void *baton);
>
>
>+/**
>+ * The regular expression matcher used by svn_config_enumerate_regexp
>+ *
>+ * This uses regular expressions similer to directory paths. It supports
>+ * the '*' and the '?' modifier. It does not support the bracket "{a,b,c}" notation
>+ * yet.
>+ *
>+ * flags:
>+ * 0x1 - case insensitive search
>+ *
>+ * returns:
>+ * 0 - does not match
>+ * 1 - matches entire string
>+ * 2 - left text after matching the pattern
>+ */
>+int svn_config_regexp(const char * str, const char *pattern, const int flags);
>+
>+/** Enumerate the sections, treating each section header as a regular expressio,
>+ * passing @a baton and the current section's name to
>+ * @a callback.
>+ *
>+ * Return the number of times @a callback was called.
>+ *
>+ * ### See kff's comment to @c svn_config_enumerate. It applies to this
>+ * function, too. ###
>+ *
>+ * @a callback's @a name and @a name parameters are only valid for the
>+ * duration of the call.
>+ *
>+ * flags:
>+ * 0x1 - case insensitive search
>+ * 0x2 - stop on first match
>+ * 0x4 - only match entire string
>+ */
>+int svn_config_enumerate_regexp (svn_config_t *cfg, const char *section, const int flags,
>+ svn_config_enumerator_t callback, void *baton);
>+
>
>

Sigh. Second of all, you completely missed the fact that there's an
apr_fnmatch API that does what you're duplicating here, and more.

But first of all, you've committed the deadly sin of Not Defining Named
Constants For Magic Numbers.

And I'm pretty sure we don't want this functionality in svn_config. You
can do the same in an enumerator callback in mod_dav_svn, using Apache's
wildcard (or even regexp) matching functions. This isn't even faster,
because you can filter the section names in the callback from
svn_config_enumerate_sections.

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Feb 17 21:07:25 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.