You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Josh Siegel <jo...@stormbirds.org> on 2005/02/18 16:29:07 UTC
pcre/regexp in mod_authz_svn
After extensive conversations in irc yesterday as well as via private
email, I rewrote the mod_authz_svn changes to use the pcre library out
of apache2. So, there are two questions that need to be resolved.
First, section header parsing (libsvn_subr/config_file.c:parse_section_name)
[repos:/foo/[^/]+/]
Right now if ] is anywhere but at the end, it emits a "Section header
must end with ']'" error. Should we modify it to do a greedy match
till it finds a "]\s*\n"? the perl expression would be: [(.*)]\s*\n?
Second, the hard issue is with pcre itself. M_COPY, M_MOVE, and
M_DELETE all require a resursive access check.
mod_authz_svn calls parse_authz_sections to search for sections where
the supplied repos_path is a substring of the section header...
Problem is you run into problems with things like
[repos:/a/b/.*/d]
* = r
/a/b/c is a partial match against this section header which would mean
we should prevent these operations from happening but only if /a/b/c has
a d some place as a terminating leaf.? or...
Solutions:
1) glob out the expression into all matching pathnames and then do
the check.
Expensive and expensive and slow...
2) pcre 5.0 supports partial matches. Since /a/b/c would be a
partial match for /a/b/.*/d, we could have the rule match. This would
be too restrictive but fast..
Apache 2.0.53 does not have pcre 5.0.. you need apache 2.2 or you
would need to patch your apache source code to add the partial match
support into the pcre that it comes with. Uncool...
3) ignore regular expressions when doing the recursive checks
Would you like me to help tighten that rope around your neck?
thoughts?
As a FYI, my test currently has option #3 in place and I have been doing
things like
[repos:/groups/([^/]+)/]
@$1 = rw
[repos:/private/([^/]+)]
$1 = rw
ie, you create a group folder under /groups and it automatically looks
for a groups entry under that name. Also, creating a folder under
/private for a user automatically only gives them access to that folder.
- josh siegel
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: pcre/regexp in mod_authz_svn
Posted by André Malo <nd...@perlig.de>.
* Sander Striker wrote:
> Branko Čibej wrote:
> > I'm a bit worried about the case-insensitive nature of the section and
> > option names. Mea culpa, I designed that interface and didn't realise
> > at the time that this was a mistake. Maybe we can silently change that
> > to case-sensitive... formally, we couldn't do this before 2.0.
>
> +1 on case-sentitive section names. I assumed they were actually.
>
> > [[[
> > Change the svn_config parser allow '[' and ']' in section names.
>
> Woah. Won't that break compatibility with the python ConfigParser?
Ah, yes (simply tested).
nd
--
Already I've seen people (really!) write web URLs in the form:
http:\\some.site.somewhere
[...] How soon until greengrocers start writing "apples $1\pound"
or something? -- Joona I Palaste in clc
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: pcre/regexp in mod_authz_svn
Posted by Branko Čibej <br...@xbc.nu>.
Sander Striker wrote:
> Branko Čibej wrote:
>
>> I'm a bit worried about the case-insensitive nature of the section
>> and option names. Mea culpa, I designed that interface and didn't
>> realise at the time that this was a mistake. Maybe we can silently
>> change that to case-sensitive... formally, we couldn't do this before
>> 2.0.
>
>
> +1 on case-sentitive section names. I assumed they were actually.
>
>> [[[
>> Change the svn_config parser allow '[' and ']' in section names.
>
>
> Woah. Won't that break compatibility with the python ConfigParser?
Wow, you're probably right. Well then, mod_authz_svn has to escape []
somehow, then. \(\), perhaps? :-)
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: pcre/regexp in mod_authz_svn
Posted by Sander Striker <s....@striker.nl>.
Branko Čibej wrote:
> I'm a bit worried about the case-insensitive nature of the section and
> option names. Mea culpa, I designed that interface and didn't realise at
> the time that this was a mistake. Maybe we can silently change that to
> case-sensitive... formally, we couldn't do this before 2.0.
+1 on case-sentitive section names. I assumed they were actually.
> [[[
> Change the svn_config parser allow '[' and ']' in section names.
Woah. Won't that break compatibility with the python ConfigParser?
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: pcre/regexp in mod_authz_svn
Posted by Branko Čibej <br...@xbc.nu>.
Josh Siegel wrote:
>
> After extensive conversations in irc yesterday as well as via private
> email, I rewrote the mod_authz_svn changes to use the pcre library out
> of apache2. So, there are two questions that need to be resolved.
> First, section header parsing
> (libsvn_subr/config_file.c:parse_section_name)
>
> [repos:/foo/[^/]+/]
>
> Right now if ] is anywhere but at the end, it emits a "Section header
> must end with ']'" error. Should we modify it to do a greedy match
> till it finds a "]\s*\n"? the perl expression would be: [(.*)]\s*\n?
I think this would be a relatively safe thing to do in the config
parser. The attached patch (untested) should do it.
I'm a bit worried about the case-insensitive nature of the section and
option names. Mea culpa, I designed that interface and didn't realise at
the time that this was a mistake. Maybe we can silently change that to
case-sensitive... formally, we couldn't do this before 2.0.
[[[
Change the svn_config parser allow '[' and ']' in section names.
* subversion/libsvn_subr/config_file.c (parse_section_name):
Don't end the section name at the first ']', look for the
last ']' instead but allow only whitespace from there to
the end of the line.
]]]
Index: config_file.c
===================================================================
--- config_file.c (revision 13049)
+++ config_file.c (working copy)
@@ -211,11 +211,11 @@
}
-/* Read chars until enounter ']', then skip everything to the end of
- * the line. Set *PCH to the character that ended the line (either
- * newline or EOF), and set CTX->section to the string of characters
- * seen before ']'.
- *
+/* Read chars until the last ']' in the line, then skip whitespace to
+ * the end of the line. Set *PCH to the character that ended the line
+ * (either newline or EOF), and set CTX->section to the string of
+ * characters seen before ']'.
+ *
* This is meant to be called immediately after reading the '[' that
* starts a section name.
*/
@@ -223,18 +223,32 @@
parse_section_name (int *pch, parse_context_t *ctx)
{
svn_error_t *err = SVN_NO_ERROR;
+ svn_boolean_t bracket = FALSE;
+ svn_boolean_t nonspace = FALSE;
+ apr_size_t bracket_col = 0;
+ apr_size_t column = 0;
int ch;
svn_stringbuf_setempty (ctx->section);
for (ch = getc (ctx->fd);
- ch != EOF && ch != ']' && ch != '\n';
- ch = getc (ctx->fd))
+ ch != EOF && && ch != '\n';
+ ch = getc (ctx->fd), ++column)
{
const char char_from_int = ch;
svn_stringbuf_appendbytes (ctx->section, &char_from_int, 1);
+ if (ch == ']')
+ {
+ /* bracket_col and bracket must be separate, because empty
+ section names are allowed. */
+ bracket_col = column;
+ bracket = TRUE;
+ nonspace = FALSE;
+ }
+ else if (bracket && !apr_isspace (ch))
+ nonspace = TRUE;
}
- if (ch != ']')
+ if (!bracket || nonspace)
{
ch = EOF;
err = svn_error_createf (SVN_ERR_MALFORMED_FILE, NULL,
@@ -245,8 +259,7 @@
}
else
{
- /* Everything from the ']' to the end of the line is ignored. */
- ch = skip_to_eoln (ctx->fd);
+ svn_stringbuf_chop (ctx->section, column - backet_col + 1);
if (ch != EOF)
++ctx->line;
}
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org