TL;DR:
https://issues.apache.org/jira/browse/SVN-4204
https://issues.apache.org/jira/browse/SVN-4795
The longer story:
Even though apr_fnmatch(), which we use for matching glob patterns in
authz rules, supports character classes ([A-Z] etc.), we can't use them
in the glob rules because of the way the config parser works. For
example, the following rule:
[:glob:/**/*.[Dd]oc]
* = rw
will be silently parsed to ":glob:/**/*.[Dd" and will match neither
'x.doc' nor 'x.Doc' but will match 'x.[Dd', which the user almost
certainly does not want. The reason for this is that our config parser,
which we still use for the syntactical part of parsing the authz files,
strictly follows the semantics of Python's ConfigParser and treats the
first ']' it finds as the section terminator, ignoring any remaining
characters to the end of the line.
The proposed patch changes this behaviour as follows:
* the /last/ ']' in the line is the section terminator;
* only whitespace is allowed after the terminator to the end of the line.
The proposed change in the parser is only enabled for parsing authz and
global group files, other Subversion configuration files will use the
current semantics.
Comments? Suggestions?
-- Brane
[[[
Change the authz file parser to allow character classes in glob rules.
* subversion/include/private/svn_config_private.h
(svn_config__parse_stream): Add new parameter 'strict_sections'
and update the docstring to describe what it does.
* subversion/include/private/svn_string_private.h
(svn_stringbuf__strip_trailing_whitespace): New.
* subversion/libsvn_repos/authz_parse.c
(svn_authz__parse): Invoke the new behaviour of svn_config__parse_stream.
* subversion/libsvn_subr/config.c
(svn_config_parse): Use the old behaviour of svn_config__parse_stream.
* subversion/libsvn_subr/config_file.c: Inclulde private/svn_string_private.h.
(parse_context_t): New member 'strict_sections'.
(parse_section_name): Select how sections are parsed depending on the
value of the 'strict_sections' flag. Update docstring.
(svn_config__parse_file): Use the old behaviour of svn_config__parse_stream.
(svn_config__parse_stream): Update implementation; store the new
'strict_sections' flag in the parser context.
* subversion/libsvn_subr/string.c
(svn_stringbuf__strip_trailing_whitespace): Implement; extracted from ...
(svn_stringbuf_strip_whitespace): ... here.
Fixes: SVN-4204, SVN-4795
]]]
Index: subversion/include/private/svn_config_private.h
===================================================================
--- subversion/include/private/svn_config_private.h (revision 1847937)
+++ subversion/include/private/svn_config_private.h (working copy)
@@ -98,6 +98,20 @@ svn_config__constructor_create(
* passed to the callback in the same order as they're defined in
* STREAM.
*
+ * If STRICT_SECTIONS is FALSE, the parser treats the first ] on a section
+ * line as the section name terminator and ignores everything else on the
+ * line. Otherwise, it uses the *last* ] as the section name terminator and
+ * allows only whitespace after it. This allows section names to contain
+ * brackets and can be used to define character classes in authz rules. For
+ * example, with STRICT_SECTIONS=FALSE, the following section definition:
+ *
+ * [:glob:/[Dd]ockerfile]
+ *
+ * is parsed as ":glob:/[Dd" and the remaining "ockerfile]" is ignored, but
+ * with STRICT_SECTIONS=TRUE, it will be parsed as ":glob:/[Dd]ockerfile"
+ * and anything other than whitespace after the terminating ] will be
+ * flagged as an error.
+ *
* The lifetome of section names, option names and values passed to
* the constructor does not extend past the invocation of each
* callback; see calback docs, above.
@@ -108,6 +122,7 @@ svn_error_t *
svn_config__parse_stream(svn_stream_t *stream,
svn_config__constructor_t *constructor,
void *constructor_baton,
+ svn_boolean_t strict_sections,
apr_pool_t *scratch_pool);
/*
Index: subversion/include/private/svn_string_private.h
===================================================================
--- subversion/include/private/svn_string_private.h (revision 1847937)
+++ subversion/include/private/svn_string_private.h (working copy)
@@ -123,6 +123,10 @@ svn_membuf__nzero(svn_membuf_t *membuf, apr_size_t
#endif
+/** Strip whitespace from the end of @a str (modified in place). */
+void
+svn_stringbuf__strip_trailing_whitespace(svn_stringbuf_t *str);
+
/** Returns the #svn_string_t information contained in the data and
* len members of @a strbuf. This is effectively a typecast, converting
* @a strbuf into an #svn_string_t. This first will become invalid and must
Index: subversion/libsvn_repos/authz_parse.c
===================================================================
--- subversion/libsvn_repos/authz_parse.c (revision 1847937)
+++ subversion/libsvn_repos/authz_parse.c (working copy)
@@ -1332,7 +1332,7 @@ svn_authz__parse(authz_full_t **authz,
close_section,
rules_add_value,
cb->parser_pool),
- cb, cb->parser_pool));
+ cb, TRUE, cb->parser_pool));
/*
* Pass 1.6487: Parse the global groups file.
@@ -1353,7 +1353,7 @@ svn_authz__parse(authz_full_t **authz,
close_section,
groups_add_value,
cb->parser_pool),
- cb, cb->parser_pool));
+ cb, TRUE, cb->parser_pool));
}
/*
Index: subversion/libsvn_subr/config.c
===================================================================
--- subversion/libsvn_subr/config.c (revision 1847937)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -194,7 +194,7 @@ svn_config_parse(svn_config_t **cfgp, svn_stream_t
NULL, NULL,
svn_config__default_add_value_fn,
scratch_pool),
- cfg, scratch_pool);
+ cfg, FALSE, scratch_pool);
if (err == SVN_NO_ERROR)
*cfgp = cfg;
Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c (revision 1847937)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -40,6 +40,7 @@
#include "svn_ctype.h"
#include "private/svn_config_private.h"
+#include "private/svn_string_private.h"
#include "private/svn_subr_private.h"
#include "svn_private_config.h"
@@ -59,6 +60,9 @@ typedef struct parse_context_t
svn_config__constructor_t *constructor;
void *constructor_baton;
+ /* Defines the behaviour of the section parser. */
+ svn_boolean_t strict_sections;
+
/* The stream struct */
svn_stream_t *stream;
@@ -550,10 +554,9 @@ parse_option(int *pch, parse_context_t *ctx, apr_p
}
-/* 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 ']'.
+/* Set *PCH to the character that ended the line (either newline or EOF),
+ * and set CTX->section to the parsed section name. Note that how the
+ * section name is parsed depends on CTX->strict_sections.
*
* This is meant to be called immediately after reading the '[' that
* starts a section name.
@@ -564,23 +567,60 @@ parse_section_name(int *pch, parse_context_t *ctx,
{
svn_error_t *err = SVN_NO_ERROR;
int ch;
- char *terminal;
SVN_ERR(parser_get_line(ctx, ctx->section, &ch));
- terminal = strchr(ctx->section->data, ']');
- if (!terminal)
+ if (ctx->strict_sections)
{
- ch = EOF;
- err = svn_error_createf(SVN_ERR_MALFORMED_FILE, NULL,
- _("line %d: Section header must end with ']'"),
- ctx->line);
+ const char *terminal;
+
+ /* The last non-whitespace char in the line must be the closing ']'. */
+
+ svn_stringbuf__strip_trailing_whitespace(ctx->section);
+ terminal = (0 >= ctx->section->len ? NULL
+ : &ctx->section->data[ctx->section->len - 1]);
+
+ if (!terminal || *terminal != ']')
+ {
+ ch = EOF;
+ if (strchr(ctx->section->data, ']'))
+ err = svn_error_createf(
+ SVN_ERR_MALFORMED_FILE, NULL,
+ _("line %d: Invalid characters after ']'"),
+ ctx->line);
+ else
+ err = svn_error_createf(
+ SVN_ERR_MALFORMED_FILE, NULL,
+ _("line %d: Section header must end with ']'"),
+ ctx->line);
+ }
+ else
+ {
+ /* Cut away the ']'. */
+ svn_stringbuf_chop(ctx->section, 1);
+ }
}
else
{
- /* Everything from the ']' to the end of the line is ignored. */
- *terminal = 0;
- ctx->section->len = terminal - ctx->section->data;
+ /* The first ']' terminates the section name, then skip everything to
+ the end of the line. */
+
+ char *const terminal = strchr(ctx->section->data, ']');
+
+ if (!terminal)
+ {
+ ch = EOF;
+ err = svn_error_createf(
+ SVN_ERR_MALFORMED_FILE, NULL,
+ _("line %d: Section header must end with ']'"),
+ ctx->line);
+ }
+ else
+ {
+ /* Everything from the ']' to the end of the line is ignored. */
+ *terminal = 0;
+ ctx->section->len = terminal - ctx->section->data;
+ }
}
*pch = ch;
@@ -739,7 +779,7 @@ svn_config__parse_file(svn_config_t *cfg, const ch
NULL, NULL,
svn_config__default_add_value_fn,
scratch_pool),
- cfg, scratch_pool);
+ cfg, FALSE, scratch_pool);
if (err != SVN_NO_ERROR)
{
@@ -759,6 +799,7 @@ svn_error_t *
svn_config__parse_stream(svn_stream_t *stream,
svn_config__constructor_t *constructor,
void *constructor_baton,
+ svn_boolean_t strict_sections,
apr_pool_t *scratch_pool)
{
parse_context_t *ctx;
@@ -769,6 +810,7 @@ svn_config__parse_stream(svn_stream_t *stream,
ctx->constructor = constructor;
ctx->constructor_baton = constructor_baton;
+ ctx->strict_sections = strict_sections;
ctx->stream = stream;
ctx->line = 1;
ctx->ungotten_char = EOF;
Index: subversion/libsvn_subr/string.c
===================================================================
--- subversion/libsvn_subr/string.c (revision 1847937)
+++ subversion/libsvn_subr/string.c (working copy)
@@ -831,6 +831,13 @@ svn_stringbuf_first_non_whitespace(const svn_strin
return string_first_non_whitespace(str->data, str->len);
}
+void
+svn_stringbuf__strip_trailing_whitespace(svn_stringbuf_t *str)
+{
+ while ((str->len > 0) && svn_ctype_isspace(str->data[str->len - 1]))
+ str->len--;
+ str->data[str->len] = '\0';
+}
void
svn_stringbuf_strip_whitespace(svn_stringbuf_t *str)
@@ -849,9 +856,7 @@ svn_stringbuf_strip_whitespace(svn_stringbuf_t *st
}
/* Now that we've trimmed the front, trim the end, wasting more RAM. */
- while ((str->len > 0) && svn_ctype_isspace(str->data[str->len - 1]))
- str->len--;
- str->data[str->len] = '\0';
+ svn_stringbuf__strip_trailing_whitespace(str);
}
