Author: brane Date: Tue Jan 22 14:34:55 2019 New Revision: 1851823 URL: http://svn.apache.org/viewvc?rev=1851823&view=rev Log: Introduce a warning callback to the authz file parser API.
We need this to warn about the use of empty groups in authz files; this is not an error and doesn't affect the authz file semantics, but it's nice to be able to tell the user about it. See issues #4794, #4802 and #4803. * subversion/include/svn_repos.h (svn_repos_authz_warning_func_t): New callback function type. (svn_repos_authz_read4): New; API revision. (svn_repos_authz_read3): Deprecated. (svn_repos_authz_parse2): New; API revision. (svn_repos_authz_parse): Deprecated. * subversion/libsvn_repos/authz.h (svn_authz__parse): Add warning function and baton parameters. * subversion/libsvn_repos/authz.c (authz_read): Add warning function and baton parameters. Update calls to svn_authz__parse. (svn_repos_authz_read4): Revised from svn_repos_authz_read3. (svn_repos_authz_parse2): Revised from svn_repos_authz_parse. * subversion/libsvn_repos/authz_parse.c (struct ctor_baton_t): Add members warning_func and warning_baton. (create_ctor_baton): Initialise these new members of the constructor baton. (emit_parser_warning): New. (SVN_AUTHZ_PARSE_WARN): New; wrapper macro for the above. (array_insert_ace): Ignore and warn about the use of empty groups. (svn_authz__parse): Update implementation to match prototype. * subversion/libsvn_repos/deprecated.c (svn_repos_authz_read3, svn_repos_authz_parse): Implement deprecated functions. * subversion/mod_authz_svn/mod_authz_svn.c (log_svn_message): New; replaces log_svn_error so that it's useful for logging warnings as well. (log_svn_error): Reimplement, calling log_svn_message. (struct authz_warning_baton_t): New. (log_authz_warning): New. (get_access_conf): Set up an authz warning handler and baton, and call svn_repos_authz_read4 instead of svn_repos_authz_read3. * subversion/svnserve/logger.h (logger__log_error): Make the 'err' parameter a pointer-to-const. Update the docstring to say that the error is not cleared. (logger__log_warning): New. * subversion/svnserve/logger.c (log_message): New; common base for logger__log_error and logger__log_message. Also *do not* allocate 8k on the stack, use the logger pool, which gets cleared at the end of the function. (logger__log_error): Reimplement. (logger__log_warning): Implement. * subversion/svnserve/serve.c (log_error): Make the error parameter const. Fix the docstring. (log_warning): New. (load_authz_config): Add warning function and baton parameters and fix pool handling. Now calls svn_repos_authz_read4 instead of svn_repos_authz_read3. (find_repos): Add warning function and baton parameters for load_authz_config. (handle_authz_warning): New. (construct_server_baton): Pass an authz warning handler and baton to find_repos. * subversion/tests/cmdline/authz_tests.py (group_member_empty_string): Fix docstring. (empty_group): New test case. (test_list): Run it. * subversion/tests/cmdline/svnauthz_tests.py (svnauthz_empty_group_test): Extend the @Issues decorator. Add a check for the expected warning on stderr. Modified: subversion/trunk/subversion/include/svn_repos.h subversion/trunk/subversion/libsvn_repos/authz.c subversion/trunk/subversion/libsvn_repos/authz.h subversion/trunk/subversion/libsvn_repos/authz_parse.c subversion/trunk/subversion/libsvn_repos/deprecated.c subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c subversion/trunk/subversion/svnserve/logger.c subversion/trunk/subversion/svnserve/logger.h subversion/trunk/subversion/svnserve/serve.c subversion/trunk/subversion/tests/cmdline/authz_tests.py subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py subversion/trunk/subversion/tests/libsvn_repos/authz-test.c subversion/trunk/tools/server-side/svnauthz.c Modified: subversion/trunk/subversion/include/svn_repos.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_repos.h?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/include/svn_repos.h (original) +++ subversion/trunk/subversion/include/svn_repos.h Tue Jan 22 14:34:55 2019 @@ -4147,6 +4147,19 @@ svn_error_t * svn_repos_authz_initialize(apr_pool_t *pool); /** + * Callback for reporting authz file parsing warnings. + * + * The implementation may use @a scratch_pool for temporary + * allocations but should not assume that the lifetime of that pool + * persists past the callback invocation. + * + * The implementation @e must @e not clear @a error. + */ +typedef void (*svn_repos_authz_warning_func_t)(void *baton, + const svn_error_t *error, + apr_pool_t *scratch_pool); + +/** * Read authz configuration data from @a path (a dirent, an absolute file url * or a registry path) into @a *authz_p, allocated in @a pool. * @@ -4164,8 +4177,31 @@ svn_repos_authz_initialize(apr_pool_t *p * repository instance. Otherwise, set it to NULL and the repositories will * be opened as needed. * + * If the @a warning_func callback is not @c NULL, it is called + * (with @a warning_baton) to report non-fatal warnings emitted by + * the parser. + * + * @since New in 1.12. + */ +svn_error_t * +svn_repos_authz_read4(svn_authz_t **authz_p, + const char *path, + const char *groups_path, + svn_boolean_t must_exist, + svn_repos_t *repos_hint, + svn_repos_authz_warning_func_t warning_func, + void *warning_baton, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool); + +/** + * Similar to svn_repos_authz_read3(), but with @a warning_func and + * @a warning_baton set to @c NULL. + * * @since New in 1.10. + * @deprecated Provided for backward compatibility with the 1.11 API. */ +SVN_DEPRECATED svn_error_t * svn_repos_authz_read3(svn_authz_t **authz_p, const char *path, @@ -4206,12 +4242,35 @@ svn_repos_authz_read(svn_authz_t **authz /** * Read authz configuration data from @a stream into @a *authz_p, - * allocated in @a pool. + * allocated in @a result_pool. * * If @a groups_stream is set, use the global groups parsed from it. * + * If the @a warning_func callback is not @c NULL, it is called + * (with @a warning_baton) to report non-fatal warnings emitted by + * the parser. + * + * Uses @a scratch_pool for temporary aloocations. + * + * @since New in 1.12. + */ +svn_error_t * +svn_repos_authz_parse2(svn_authz_t **authz_p, + svn_stream_t *stream, + svn_stream_t *groups_stream, + svn_repos_authz_warning_func_t warning_func, + void *warning_baton, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool); + +/** + * Similar to svn_repos_authz_parse2(), but with @a warning_func and + * @a warning_baton set to @c NULL. + * * @since New in 1.8. + * @deprecated Provided for backward compatibility with the 1.11 API. */ +SVN_DEPRECATED svn_error_t * svn_repos_authz_parse(svn_authz_t **authz_p, svn_stream_t *stream, Modified: subversion/trunk/subversion/libsvn_repos/authz.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/authz.c?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_repos/authz.c (original) +++ subversion/trunk/subversion/libsvn_repos/authz.c Tue Jan 22 14:34:55 2019 @@ -1548,6 +1548,8 @@ authz_read(authz_full_t **authz_p, const char *groups_path, svn_boolean_t must_exist, svn_repos_t *repos_hint, + svn_repos_authz_warning_func_t warning_func, + void *warning_baton, apr_pool_t *result_pool, apr_pool_t *scratch_pool) { @@ -1587,7 +1589,8 @@ authz_read(authz_full_t **authz_p, /* Parse the configuration(s) and construct the full authz model * from it. */ err = svn_authz__parse(authz_p, rules_stream, groups_stream, - item_pool, scratch_pool); + warning_func, warning_baton, + item_pool, scratch_pool); if (err != SVN_NO_ERROR) { /* That pool would otherwise never get destroyed. */ @@ -1611,11 +1614,11 @@ authz_read(authz_full_t **authz_p, { /* Parse the configuration(s) and construct the full authz model from * it. */ - err = svn_error_quick_wrapf(svn_authz__parse(authz_p, rules_stream, - groups_stream, - result_pool, scratch_pool), - "Error while parsing authz file: '%s':", - path); + err = svn_error_quick_wrapf( + svn_authz__parse(authz_p, rules_stream, groups_stream, + warning_func, warning_baton, + result_pool, scratch_pool), + "Error while parsing authz file: '%s':", path); } svn_repos__destroy_config_access(config_access); @@ -1628,11 +1631,13 @@ authz_read(authz_full_t **authz_p, /*** Public functions. ***/ svn_error_t * -svn_repos_authz_read3(svn_authz_t **authz_p, +svn_repos_authz_read4(svn_authz_t **authz_p, const char *path, const char *groups_path, svn_boolean_t must_exist, svn_repos_t *repos_hint, + svn_repos_authz_warning_func_t warning_func, + void *warning_baton, apr_pool_t *result_pool, apr_pool_t *scratch_pool) { @@ -1640,7 +1645,8 @@ svn_repos_authz_read3(svn_authz_t **auth authz->pool = result_pool; SVN_ERR(authz_read(&authz->full, &authz->authz_id, path, groups_path, - must_exist, repos_hint, result_pool, scratch_pool)); + must_exist, repos_hint, warning_func, warning_baton, + result_pool, scratch_pool)); *authz_p = authz; return SVN_NO_ERROR; @@ -1648,18 +1654,21 @@ svn_repos_authz_read3(svn_authz_t **auth svn_error_t * -svn_repos_authz_parse(svn_authz_t **authz_p, svn_stream_t *stream, - svn_stream_t *groups_stream, apr_pool_t *pool) +svn_repos_authz_parse2(svn_authz_t **authz_p, + svn_stream_t *stream, + svn_stream_t *groups_stream, + svn_repos_authz_warning_func_t warning_func, + void *warning_baton, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) { - apr_pool_t *scratch_pool = svn_pool_create(pool); - svn_authz_t *authz = apr_pcalloc(pool, sizeof(*authz)); - authz->pool = pool; + svn_authz_t *authz = apr_pcalloc(result_pool, sizeof(*authz)); + authz->pool = result_pool; /* Parse the configuration and construct the full authz model from it. */ - SVN_ERR(svn_authz__parse(&authz->full, stream, groups_stream, pool, - scratch_pool)); - - svn_pool_destroy(scratch_pool); + SVN_ERR(svn_authz__parse(&authz->full, stream, groups_stream, + warning_func, warning_baton, + result_pool, scratch_pool)); *authz_p = authz; return SVN_NO_ERROR; Modified: subversion/trunk/subversion/libsvn_repos/authz.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/authz.h?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_repos/authz.h (original) +++ subversion/trunk/subversion/libsvn_repos/authz.h Tue Jan 22 14:34:55 2019 @@ -312,6 +312,8 @@ svn_error_t * svn_authz__parse(authz_full_t **authz, svn_stream_t *rules, svn_stream_t *groups, + svn_repos_authz_warning_func_t warning_func, + void *warning_baton, apr_pool_t *result_pool, apr_pool_t *scratch_pool); Modified: subversion/trunk/subversion/libsvn_repos/authz_parse.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/authz_parse.c?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_repos/authz_parse.c (original) +++ subversion/trunk/subversion/libsvn_repos/authz_parse.c Tue Jan 22 14:34:55 2019 @@ -127,6 +127,10 @@ typedef struct ctor_baton_t svn_membuf_t rule_path_buffer; svn_stringbuf_t *rule_string_buffer; + /* The warning callback and its baton. */ + svn_repos_authz_warning_func_t warning_func; + void *warning_baton; + /* The parser's scratch pool. This may not be the same pool as passed to the constructor callbacks, that is supposed to be an iteration pool maintained by the generic parser. @@ -203,7 +207,9 @@ insert_default_acl(ctor_baton_t *cb) /* Initialize a constuctor baton. */ static ctor_baton_t * -create_ctor_baton(apr_pool_t *result_pool, +create_ctor_baton(svn_repos_authz_warning_func_t warning_func, + void *warning_baton, + apr_pool_t *result_pool, apr_pool_t *scratch_pool) { apr_pool_t *const parser_pool = svn_pool_create(scratch_pool); @@ -234,6 +240,9 @@ create_ctor_baton(apr_pool_t *result_poo svn_membuf__create(&cb->rule_path_buffer, 0, parser_pool); cb->rule_string_buffer = svn_stringbuf_create_empty(parser_pool); + cb->warning_func = warning_func; + cb->warning_baton = warning_baton; + cb->parser_pool = parser_pool; insert_default_acl(cb); @@ -242,6 +251,25 @@ create_ctor_baton(apr_pool_t *result_poo } +/* Emit a warning. Clears ERROR */ +static void +emit_parser_warning(const ctor_baton_t *cb, + svn_error_t *error, + apr_pool_t *scratch_pool) +{ + if (cb->warning_func) + cb->warning_func(cb->warning_baton, error, scratch_pool); + svn_error_clear(error); +} + +/* Avoid creating an error struct if there is no warning function. */ +#define SVN_AUTHZ_PARSE_WARN(cb, err, pool) \ + do { \ + if ((cb) && (cb)->warning_func) \ + emit_parser_warning((cb), (err), (pool)); \ + } while(0) + + /* Create and store per-user global rights. The USER string must be interned or statically initialized. */ static void @@ -1186,8 +1214,14 @@ array_insert_ace(void *baton, } else if (0 == apr_hash_count(ace->members)) { - /* TODO: Somehow emit a warning about the use of an empty group. */ /* An ACE for an empty group has no effect, so ignore it. */ + SVN_AUTHZ_PARSE_WARN( + iab->cb, + svn_error_createf( + SVN_ERR_AUTHZ_INVALID_CONFIG, NULL, + _("Ignoring access entry for empty group '%s'"), + ace->name), + scratch_pool); return SVN_NO_ERROR; } } @@ -1335,10 +1369,13 @@ svn_error_t * svn_authz__parse(authz_full_t **authz, svn_stream_t *rules, svn_stream_t *groups, + svn_repos_authz_warning_func_t warning_func, + void *warning_baton, apr_pool_t *result_pool, apr_pool_t *scratch_pool) { - ctor_baton_t *const cb = create_ctor_baton(result_pool, scratch_pool); + ctor_baton_t *const cb = create_ctor_baton(warning_func, warning_baton, + result_pool, scratch_pool); /* * Pass 1: Parse the authz file. Modified: subversion/trunk/subversion/libsvn_repos/deprecated.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/deprecated.c?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_repos/deprecated.c (original) +++ subversion/trunk/subversion/libsvn_repos/deprecated.c Tue Jan 22 14:34:55 2019 @@ -1273,6 +1273,21 @@ svn_repos_fs_begin_txn_for_update(svn_fs /*** From authz.c ***/ svn_error_t * +svn_repos_authz_read3(svn_authz_t **authz_p, + const char *path, + const char *groups_path, + svn_boolean_t must_exist, + svn_repos_t *repos_hint, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) +{ + return svn_error_trace(svn_repos_authz_read4(authz_p, path, groups_path, + must_exist, repos_hint, + NULL, NULL, result_pool, + scratch_pool)); +} + +svn_error_t * svn_repos_authz_read2(svn_authz_t **authz_p, const char *path, const char *groups_path, @@ -1300,3 +1315,17 @@ svn_repos_authz_read(svn_authz_t **authz return svn_error_trace(svn_repos_authz_read2(authz_p, file, NULL, must_exist, pool)); } + +svn_error_t * +svn_repos_authz_parse(svn_authz_t **authz_p, + svn_stream_t *stream, + svn_stream_t *groups_stream, + apr_pool_t *pool) +{ + apr_pool_t *scratch_pool = svn_pool_create(pool); + svn_error_t *err = svn_repos_authz_parse2(authz_p, stream, groups_stream, + NULL, NULL, pool, scratch_pool); + svn_pool_destroy(scratch_pool); + + return svn_error_trace(err); +} Modified: subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c (original) +++ subversion/trunk/subversion/mod_authz_svn/mod_authz_svn.c Tue Jan 22 14:34:55 2019 @@ -327,16 +327,17 @@ log_access_verdict(LOG_ARGS_SIGNATURE, } } -/* Log a message indiciating the ERR encountered during the request R. +/* Log a message at LOG_LEVEL indiciating the ERR encountered during + * the request R. * LOG_ARGS_SIGNATURE expands as in log_access_verdict() above. * PREFIX is inserted at the start of the message. The rest of the * message is generated by combining the message for each error in the * chain of ERR, excluding for trace errors. ERR will be cleared * when finished. */ static void -log_svn_error(LOG_ARGS_SIGNATURE, - request_rec *r, const char *prefix, - svn_error_t *err, apr_pool_t *scratch_pool) +log_svn_message(LOG_ARGS_SIGNATURE, int log_level, + request_rec *r, const char *prefix, + svn_error_t *err, apr_pool_t *scratch_pool) { svn_error_t *err_pos = svn_error_purge_tracing(err); svn_stringbuf_t *buff = svn_stringbuf_create(prefix, scratch_pool); @@ -360,7 +361,7 @@ log_svn_error(LOG_ARGS_SIGNATURE, err_pos = err_pos->child; } - ap_log_rerror(LOG_ARGS_CASCADE, APLOG_ERR, + ap_log_rerror(LOG_ARGS_CASCADE, log_level, /* If it is an error code that APR can make sense of, then show it, otherwise, pass zero to avoid putting "APR does not understand this error code" in the error log. */ @@ -372,6 +373,40 @@ log_svn_error(LOG_ARGS_SIGNATURE, svn_error_clear(err); } +/* Log the error error ERR encountered during the request R. + * LOG_ARGS_SIGNATURE expands as in log_access_verdict() above. + * PREFIX is inserted at the start of the message. The rest of the + * message is generated by combining the message for each error in the + * chain of ERR, excluding for trace errors. ERR will be cleared + * when finished. */ +static APR_INLINE void +log_svn_error(LOG_ARGS_SIGNATURE, + request_rec *r, const char *prefix, + svn_error_t *err, apr_pool_t *scratch_pool) +{ + log_svn_message(LOG_ARGS_CASCADE, APLOG_ERR, + r, prefix, err, scratch_pool); +} + +/* Baton for log_authz_warning. */ +typedef struct authz_warning_baton_t +{ + request_rec *r; + const char *prefix; +} authz_warning_baton_t; + +/* Handle an authz parser warning. ERR will *not* be cleared.*/ +static APR_INLINE void +log_authz_warning(void *baton, + const svn_error_t *err, + apr_pool_t *scratch_pool) +{ + const authz_warning_baton_t *const warning_baton = baton; + log_svn_message(APLOG_MARK, APLOG_WARNING, + warning_baton->r, warning_baton->prefix, + svn_error_dup(err), scratch_pool); +} + /* Resolve *PATH into an absolute canonical URL iff *PATH is a repos-relative * URL. If *REPOS_URL is NULL convert REPOS_PATH into a file URL stored * in *REPOS_URL, if *REPOS_URL is not null REPOS_PATH is ignored. The @@ -472,8 +507,13 @@ get_access_conf(request_rec *r, authz_sv access_conf = user_data; if (access_conf == NULL) { - svn_err = svn_repos_authz_read3(&access_conf, access_file, + authz_warning_baton_t warning_baton; + warning_baton.r = r; + warning_baton.prefix = "mod_authz_svn: warning:"; + + svn_err = svn_repos_authz_read4(&access_conf, access_file, groups_file, TRUE, NULL, + log_authz_warning, &warning_baton, r->connection->pool, scratch_pool); Modified: subversion/trunk/subversion/svnserve/logger.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/logger.c?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/svnserve/logger.c (original) +++ subversion/trunk/subversion/svnserve/logger.c Tue Jan 22 14:34:55 2019 @@ -88,19 +88,21 @@ logger__create(logger_t **logger, return SVN_NO_ERROR; } -void -logger__log_error(logger_t *logger, - svn_error_t *err, - repository_t *repository, - client_info_t *client_info) +static void +log_message(logger_t *logger, + const svn_error_t *err, + const char *prefix, + repository_t *repository, + client_info_t *client_info) { if (logger && err) { const char *timestr, *continuation; const char *user, *repos, *remote_host; - char errbuf[256]; + /* 8192 from MAX_STRING_LEN in from httpd-2.2.4/include/httpd.h */ - char errstr[8192]; + const apr_size_t errstr_size = 8192; + char *errstr = apr_palloc(logger->pool, errstr_size); svn_error_clear(svn_mutex__lock(logger->mutex)); @@ -118,21 +120,22 @@ logger__log_error(logger_t *logger, continuation = ""; while (err) { + char errbuf[256]; const char *message = svn_err_best_message(err, errbuf, sizeof(errbuf)); /* based on httpd-2.2.4/server/log.c:log_error_core */ - apr_size_t len = apr_snprintf(errstr, sizeof(errstr), + apr_size_t len = apr_snprintf(errstr, errstr_size, "%" APR_PID_T_FMT - " %s %s %s %s ERR%s %s %ld %d ", + " %s %s %s %s %s%s %s %ld %d ", getpid(), timestr, remote_host, user, - repos, continuation, + repos, prefix, continuation, err->file ? err->file : "-", err->line, err->apr_err); len += escape_errorlog_item(errstr + len, message, - sizeof(errstr) - len); + errstr_size - len); /* Truncate for the terminator (as apr_snprintf does) */ - if (len > sizeof(errstr) - sizeof(APR_EOL_STR)) { - len = sizeof(errstr) - sizeof(APR_EOL_STR); + if (len > errstr_size - sizeof(APR_EOL_STR)) { + len = errstr_size - sizeof(APR_EOL_STR); } memcpy(errstr + len, APR_EOL_STR, sizeof(APR_EOL_STR)); @@ -150,6 +153,24 @@ logger__log_error(logger_t *logger, } } +void +logger__log_error(logger_t *logger, + const svn_error_t *err, + repository_t *repository, + client_info_t *client_info) +{ + log_message(logger, err, "ERR", repository, client_info); +} + +void +logger__log_warning(logger_t *logger, + const svn_error_t *err, + repository_t *repository, + client_info_t *client_info) +{ + log_message(logger, err, "WARN", repository, client_info); +} + svn_error_t * logger__write(logger_t *logger, const char *errstr, Modified: subversion/trunk/subversion/svnserve/logger.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/logger.h?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/svnserve/logger.h (original) +++ subversion/trunk/subversion/svnserve/logger.h Tue Jan 22 14:34:55 2019 @@ -64,14 +64,21 @@ logger__write(logger_t *logger, /* Write a description of ERR with additional information from REPOSITORY * and CLIENT_INFO to the log file managed by LOGGER. REPOSITORY as well * as CLIENT_INFO may be NULL. If either ERR or LOGGER are NULL, this - * becomes a no-op. + * becomes a no-op. Does not clear ERR. */ void logger__log_error(logger_t *logger, - svn_error_t *err, + const svn_error_t *err, repository_t *repository, client_info_t *client_info); +/* Like logger__log_error() but for warnings. */ +void +logger__log_warning(logger_t *logger, + const svn_error_t *err, + repository_t *repository, + client_info_t *client_info); + #ifdef __cplusplus } #endif /* __cplusplus */ Modified: subversion/trunk/subversion/svnserve/serve.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/serve.c?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/svnserve/serve.c (original) +++ subversion/trunk/subversion/svnserve/serve.c Tue Jan 22 14:34:55 2019 @@ -107,15 +107,22 @@ typedef struct authz_baton_t { svn_ra_svn_conn_t *conn; } authz_baton_t; -/* svn_error_create() a new error, log_server_error() it, and - return it. */ +/* Log an error. */ static void -log_error(svn_error_t *err, server_baton_t *server) +log_error(const svn_error_t *err, server_baton_t *server) { logger__log_error(server->logger, err, server->repository, server->client_info); } +/* Log a warning. */ +static void +log_warning(const svn_error_t *err, server_baton_t *server) +{ + logger__log_warning(server->logger, err, server->repository, + server->client_info); +} + /* svn_error_create() a new error, log_server_error() it, and return it. */ static svn_error_t * @@ -294,7 +301,10 @@ static svn_error_t * load_authz_config(repository_t *repository, const char *repos_root, svn_config_t *cfg, - apr_pool_t *pool) + svn_repos_authz_warning_func_t warning_func, + void *warning_baton, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) { const char *authzdb_path; const char *groupsdb_path; @@ -313,17 +323,18 @@ load_authz_config(repository_t *reposito /* Canonicalize and add the base onto the authzdb_path (if needed). */ err = canonicalize_access_file(&authzdb_path, repository, - repos_root, pool); + repos_root, scratch_pool); /* Same for the groupsdb_path if it is present. */ if (groupsdb_path && !err) err = canonicalize_access_file(&groupsdb_path, repository, - repos_root, pool); + repos_root, scratch_pool); if (!err) - err = svn_repos_authz_read3(&repository->authzdb, authzdb_path, + err = svn_repos_authz_read4(&repository->authzdb, authzdb_path, groupsdb_path, TRUE, repository->repos, - pool, pool); + warning_func, warning_baton, + result_pool, scratch_pool); if (err) return svn_error_create(SVN_ERR_AUTHZ_INVALID_CONFIG, err, NULL); @@ -3793,6 +3804,8 @@ find_repos(const char *url, repository_t *repository, svn_repos__config_pool_t *config_pool, apr_hash_t *fs_config, + svn_repos_authz_warning_func_t authz_warning_func, + void *authz_warning_baton, apr_pool_t *result_pool, apr_pool_t *scratch_pool) { @@ -3870,7 +3883,8 @@ find_repos(const char *url, SVN_ERR(load_pwdb_config(repository, cfg, config_pool, result_pool)); SVN_ERR(load_authz_config(repository, repository->repos_root, cfg, - result_pool)); + authz_warning_func, authz_warning_baton, + result_pool, scratch_pool)); /* Should we use Cyrus SASL? */ SVN_ERR(svn_config_get_bool(cfg, &sasl_requested, @@ -4092,6 +4106,16 @@ get_client_info(svn_ra_svn_conn_t *conn, return client_info; } +static void +handle_authz_warning(void *baton, + const svn_error_t *err, + apr_pool_t *scratch_pool) +{ + server_baton_t *const server_baton = baton; + log_warning(err, server_baton); + SVN_UNUSED(scratch_pool); +} + /* Construct the server baton for CONN using PARAMS and return it in *BATON. * It's lifetime is the same as that of CONN. SCRATCH_POOL */ @@ -4214,10 +4238,14 @@ construct_server_baton(server_baton_t ** } } + /* (*b) has the logger, repository and client_info set, so it can + be used as the authz_warning_baton that eventyally gets passed + to log_warning(). */ err = handle_config_error(find_repos(client_url, params->root, b->vhost, b->read_only, params->cfg, b->repository, params->config_pool, params->fs_config, + handle_authz_warning, b, conn_pool, scratch_pool), b); if (!err) Modified: subversion/trunk/subversion/tests/cmdline/authz_tests.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/authz_tests.py?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/tests/cmdline/authz_tests.py (original) +++ subversion/trunk/subversion/tests/cmdline/authz_tests.py Tue Jan 22 14:34:55 2019 @@ -1694,7 +1694,7 @@ def inverted_group_membership(sbox): @Skip(svntest.main.is_ra_type_file) def group_member_empty_string(sbox): - "group definition ignores with empty member" + "group definition ignores empty member" sbox.build(create_wc = False) @@ -1710,6 +1710,27 @@ def group_member_empty_string(sbox): '--username', svntest.main.wc_author, sbox.repo_url) +@Issue(4802) +@Skip(svntest.main.is_ra_type_file) +def empty_group(sbox): + "empty group is ignored" + + sbox.build(create_wc = False) + + write_restrictive_svnserve_conf(sbox.repo_dir) + write_authz_file(sbox, + {"/" : ("$anonymous =\n" + "@empty = rw\n" + "@readonly = r\n")}, + {"groups": ("empty = \n" + "readonly = %s\n" % svntest.main.wc_author)}) + + expected_output = svntest.verify.UnorderedOutput(['A/\n', 'iota\n']) + svntest.actions.run_and_verify_svn(expected_output, [], + 'list', + '--username', svntest.main.wc_author, + sbox.repo_url) + ######################################################################## # Run the tests @@ -1749,6 +1770,7 @@ test_list = [ None, remove_access_after_commit, inverted_group_membership, group_member_empty_string, + empty_group, ] serial_only = True Modified: subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py (original) +++ subversion/trunk/subversion/tests/cmdline/svnauthz_tests.py Tue Jan 22 14:34:55 2019 @@ -965,7 +965,7 @@ def svnauthz_inverted_selector_test(sbox os.remove(authz_path) -@Issue(4802) +@Issues(4802, 4803) def svnauthz_empty_group_test(sbox): "test empty group definition" @@ -984,12 +984,15 @@ def svnauthz_empty_group_test(sbox): (authz_fd, authz_path) = tempfile.mkstemp() svntest.main.file_write(authz_path, authz_content) + expected_stderr = svntest.verify.RegexOutput( + r".*warning: W220003:.*", match_all=False) + svntest.actions.run_and_verify_svnauthz( - [], None, 0, False, 'validate', authz_path) + [], expected_stderr, 0, False, 'validate', authz_path) svntest.actions.run_and_verify_svnauthz( - 'r', None, 0, False, 'accessof', - '--repository', 'A', '--username', 'user', authz_path) + 'r', expected_stderr, 0, False, 'accessof', + '--repository', 'A', '--username', 'user', authz_path) ######################################################################## Modified: subversion/trunk/subversion/tests/libsvn_repos/authz-test.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_repos/authz-test.c?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/subversion/tests/libsvn_repos/authz-test.c (original) +++ subversion/trunk/subversion/tests/libsvn_repos/authz-test.c Tue Jan 22 14:34:55 2019 @@ -212,7 +212,7 @@ test_authz_parse(const svn_test_opts_t * APR_READ, APR_OS_DEFAULT, pool)); groups = svn_stream_from_aprfile2(groups_file, FALSE, pool); - SVN_ERR(svn_authz__parse(&authz, rules, groups, pool, pool)); + SVN_ERR(svn_authz__parse(&authz, rules, groups, NULL, NULL, pool, pool)); printf("Access check for ('%s', '%s')\n", check_user, check_repo); @@ -304,7 +304,7 @@ run_global_rights_tests(const char *cont svn_stringbuf_t *buffer = svn_stringbuf_create(contents, pool); svn_stream_t *stream = svn_stream_from_stringbuf(buffer, pool); - SVN_ERR(svn_repos_authz_parse(&authz, stream, NULL, pool)); + SVN_ERR(svn_repos_authz_parse2(&authz, stream, NULL, NULL, NULL, pool, pool)); for (; test_cases->repos; ++test_cases) { @@ -463,7 +463,7 @@ issue_4741_groups(apr_pool_t *pool) svn_authz_t *authz; svn_boolean_t access_granted; - SVN_ERR(svn_repos_authz_parse(&authz, stream, NULL, pool)); + SVN_ERR(svn_repos_authz_parse2(&authz, stream, NULL, NULL, NULL, pool, pool)); SVN_ERR(svn_repos_authz_check_access(authz, "repo", "/", "userA", svn_authz_write, &access_granted, @@ -500,7 +500,7 @@ reposful_reposless_stanzas_inherit(apr_p svn_authz_t *authz; svn_boolean_t access_granted; - SVN_ERR(svn_repos_authz_parse(&authz, stream, NULL, pool)); + SVN_ERR(svn_repos_authz_parse2(&authz, stream, NULL, NULL, NULL, pool, pool)); SVN_ERR(svn_repos_authz_check_access(authz, "project1", "/foo", "user1", svn_authz_write | svn_authz_recursive, Modified: subversion/trunk/tools/server-side/svnauthz.c URL: http://svn.apache.org/viewvc/subversion/trunk/tools/server-side/svnauthz.c?rev=1851823&r1=1851822&r2=1851823&view=diff ============================================================================== --- subversion/trunk/tools/server-side/svnauthz.c (original) +++ subversion/trunk/tools/server-side/svnauthz.c Tue Jan 22 14:34:55 2019 @@ -31,6 +31,7 @@ #include "private/svn_fspath.h" #include "private/svn_cmdline_private.h" +#include "svn_private_config.h" /*** Option Processing. ***/ @@ -99,6 +100,9 @@ struct svnauthz_opt_state /* Libtool command prefix */ #define SVNAUTHZ_LT_PREFIX "lt-" +/* The prefix for handling errors and warnings. */ +#define SVNAUTHZ_ERR_PREFIX "svnauthz: " + /*** Subcommands. */ @@ -224,6 +228,18 @@ read_file_contents(svn_stream_t **conten return SVN_NO_ERROR; } +/* Handles warning emitted by the authz parser. */ +static void +handle_parser_warning(void *baton, + const svn_error_t *err, + apr_pool_t *scratch_pool) +{ + svn_handle_warning2(stderr, err, SVNAUTHZ_ERR_PREFIX); + SVN_UNUSED(baton); + SVN_UNUSED(scratch_pool); +} + + /* Loads the authz config into *AUTHZ from the file at AUTHZ_FILE in repository at REPOS_PATH from the transaction TXN_NAME. If GROUPS_FILE is set, the resulting *AUTHZ will be constructed from AUTHZ_FILE with @@ -256,7 +272,8 @@ get_authz_from_txn(svn_authz_t **authz, else groups_contents = NULL; - err = svn_repos_authz_parse(authz, authz_contents, groups_contents, pool); + err = svn_repos_authz_parse2(authz, authz_contents, groups_contents, + handle_parser_warning, NULL, pool, pool); /* Add the filename to the error stack since the parser doesn't have it. */ if (err != SVN_NO_ERROR) @@ -283,9 +300,11 @@ get_authz(svn_authz_t **authz, struct sv opt_state->txn, pool); /* Else */ - return svn_repos_authz_read3(authz, opt_state->authz_file, + return svn_repos_authz_read4(authz, opt_state->authz_file, opt_state->groups_file, - TRUE, NULL, pool, pool); + TRUE, NULL, + handle_parser_warning, NULL, + pool, pool); } static svn_error_t * @@ -755,7 +774,7 @@ main(int argc, const char *argv[]) { if (exit_code == 0) exit_code = EXIT_FAILURE; - svn_cmdline_handle_exit_error(err, NULL, "svnauthz: "); + svn_cmdline_handle_exit_error(err, NULL, SVNAUTHZ_ERR_PREFIX); } svn_pool_destroy(pool);