Index: subversion/libsvn_repos/authz.c =================================================================== --- subversion/libsvn_repos/authz.c (revision 1511246) +++ subversion/libsvn_repos/authz.c (working copy) @@ -45,7 +45,7 @@ lookup. */ struct authz_lookup_baton { /* The authz configuration. */ - svn_config_t *config; + svn_authz_t *authz; /* The user to authorize. */ const char *user; @@ -77,14 +77,26 @@ enumerator, if any. */ }; -/* Currently this structure is just a wrapper around a - svn_config_t. */ +/* Information needed for authz lookups */ struct svn_authz_t { - svn_config_t *cfg; + svn_config_t *cfg; /* The configuration file where to look up + authorizations. */ + apr_hash_t *cache; /* Cache of previous lookup results (w.r.t. user + CACHED_USER). A hash table whose keys are paths + and whose values are access_cache_t *. */ + const char *cached_user; /* User for which lookups are cached. */ + apr_pool_t *pool; /* The pool this struct was allocated in. */ }; +/* Cached authz information */ +struct access_cache_t +{ + svn_repos_authz_access_t allow; + svn_repos_authz_access_t deny; +}; + /*** Checking access. ***/ @@ -242,10 +254,10 @@ */ if (rule_match_string[0] == '@') return authz_group_contains_user( - b->config, &rule_match_string[1], b->user, pool); + b->authz->cfg, &rule_match_string[1], b->user, pool); else if (rule_match_string[0] == '&') return authz_alias_is_user( - b->config, &rule_match_string[1], b->user, pool); + b->authz->cfg, &rule_match_string[1], b->user, pool); else return (strcmp(b->user, rule_match_string) == 0); } @@ -296,6 +308,62 @@ } +/* Wrapper for svn_config_enumerate2(..., authz_parse_line, ...) + * that manages the allow/deny per-section cache. + */ +static void +authz_lookup_cached(const char *path, + struct authz_lookup_baton *baton, + apr_pool_t *pool) +{ + struct svn_config_t *cfg = baton->authz->cfg; + struct apr_hash_t *cache = baton->authz->cache; + struct access_cache_t *access; + svn_repos_authz_access_t saved_allow, saved_deny; + + if (!cache) + { + svn_config_enumerate2(cfg, path, + authz_parse_line, baton, pool); + return; + } + + /* If section [path] doesn't exist, then we're done. */ + if (!svn_config_has_section(cfg, path)) + return; + + /* If we already know allow/deny for section [path] ... */ + access = svn_hash_gets(cache, path); + if (access) + { + /* ... combine baton with the cached allow/deny and we're done. */ + baton->allow |= access->allow; + baton->deny |= access->deny; + return; + } + + /* Reuse baton: save baton's allow/deny and reset it. */ + saved_allow = baton->allow; + saved_deny = baton->deny; + baton->allow = baton->deny = svn_authz_none; + + svn_config_enumerate2(cfg, path, authz_parse_line, baton, pool); + + /* Cache the result. + * Note: Use baton->authz->pool instead of pool as the cache must have + * the same lifetime as baton->authz. + */ + access = apr_palloc(baton->authz->pool, sizeof(*access)); + access->allow = baton->allow; + access->deny = baton->deny; + svn_hash_sets(cache, apr_pstrdup(baton->authz->pool, path), access); + + /* Combine the result with the saved allow/deny. */ + baton->allow |= saved_allow; + baton->deny |= saved_deny; +} + + /* Callback to parse a section and update the authz_baton if the * section denies access to the subtree the baton describes. */ @@ -311,9 +379,8 @@ return TRUE; /* Work out what this section grants. */ - b->allow = b->deny = 0; - svn_config_enumerate2(b->config, section_name, - authz_parse_line, b, pool); + b->allow = b->deny = svn_authz_none; + authz_lookup_cached(section_name, b, pool); /* Has the section explicitly determined an access? */ conclusive = authz_access_is_determined(b->allow, b->deny, @@ -339,7 +406,7 @@ * successfully determined. */ static svn_boolean_t -authz_get_path_access(svn_config_t *cfg, const char *repos_name, +authz_get_path_access(svn_authz_t *authz, const char *repos_name, const char *path, const char *user, svn_repos_authz_access_t required_access, svn_boolean_t *access_granted, @@ -347,14 +414,13 @@ { const char *qualified_path; struct authz_lookup_baton baton = { 0 }; - - baton.config = cfg; + + baton.authz = authz; baton.user = user; /* Try to locate a repository-specific block first. */ qualified_path = apr_pstrcat(pool, repos_name, ":", path, (char *)NULL); - svn_config_enumerate2(cfg, qualified_path, - authz_parse_line, &baton, pool); + authz_lookup_cached(qualified_path, &baton, pool); *access_granted = authz_access_is_granted(baton.allow, baton.deny, required_access); @@ -365,7 +431,7 @@ return TRUE; /* No repository specific rule, try pan-repository rules. */ - svn_config_enumerate2(cfg, path, authz_parse_line, &baton, pool); + authz_lookup_cached(path, &baton, pool); *access_granted = authz_access_is_granted(baton.allow, baton.deny, required_access); @@ -383,14 +449,14 @@ * searched, return the updated authorization status. */ static svn_boolean_t -authz_get_tree_access(svn_config_t *cfg, const char *repos_name, +authz_get_tree_access(svn_authz_t *authz, const char *repos_name, const char *path, const char *user, svn_repos_authz_access_t required_access, apr_pool_t *pool) { struct authz_lookup_baton baton = { 0 }; - baton.config = cfg; + baton.authz = authz; baton.user = user; baton.required_access = required_access; baton.repos_path = path; @@ -399,7 +465,7 @@ /* Default to access granted if no rules say otherwise. */ baton.access = TRUE; - svn_config_enumerate_sections2(cfg, authz_parse_section, + svn_config_enumerate_sections2(authz->cfg, authz_parse_section, &baton, pool); return baton.access; @@ -421,9 +487,8 @@ strlen(b->qualified_repos_path)) == 0) { b->allow = b->deny = svn_authz_none; - - svn_config_enumerate2(b->config, section_name, - authz_parse_line, baton, pool); + + authz_lookup_cached(section_name, baton, pool); b->access = authz_access_is_granted(b->allow, b->deny, b->required_access); @@ -441,14 +506,14 @@ * to any path within the REPOSITORY. Return TRUE if so. Use POOL * for temporary allocations. */ static svn_boolean_t -authz_get_any_access(svn_config_t *cfg, const char *repos_name, +authz_get_any_access(svn_authz_t *authz, const char *repos_name, const char *user, svn_repos_authz_access_t required_access, apr_pool_t *pool) { struct authz_lookup_baton baton = { 0 }; - baton.config = cfg; + baton.authz = authz; baton.user = user; baton.required_access = required_access; baton.access = FALSE; /* Deny access by default. */ @@ -461,7 +526,7 @@ * may not always have). So we end up enumerating the sections in * the authz CFG and stop on the first match with some access for * this user. */ - svn_config_enumerate_sections2(cfg, authz_get_any_access_parser_cb, + svn_config_enumerate_sections2(authz->cfg, authz_get_any_access_parser_cb, &baton, pool); /* If walking the configuration was inconclusive, deny access. */ @@ -934,12 +999,21 @@ return SVN_NO_ERROR; } +/* Allocate a svn_authz_t in POOL and remember it in svn_authz_t->pool. */ +static svn_authz_t * +authz_create(apr_pool_t *pool) +{ + svn_authz_t *authz = apr_pcalloc(pool, sizeof(*authz)); + authz->pool = pool; + return authz; +} + svn_error_t * svn_repos__authz_read(svn_authz_t **authz_p, const char *path, const char *groups_path, svn_boolean_t must_exist, svn_boolean_t accept_urls, apr_pool_t *pool) { - svn_authz_t *authz = apr_palloc(pool, sizeof(*authz)); + svn_authz_t *authz = authz_create(pool); /* Load the authz file */ if (accept_urls) @@ -996,7 +1070,7 @@ svn_repos_authz_parse(svn_authz_t **authz_p, svn_stream_t *stream, svn_stream_t *groups_stream, apr_pool_t *pool) { - svn_authz_t *authz = apr_palloc(pool, sizeof(*authz)); + svn_authz_t *authz = authz_create(pool); /* Parse the authz stream */ SVN_ERR(svn_config_parse(&authz->cfg, stream, TRUE, TRUE, pool)); @@ -1028,13 +1102,36 @@ { const char *current_path; + /* Check whether USER is still the same as authz->cached_user; + * otherwise blow away authz->cache and remember the new USER. + * Create authz->cache if it doesn't exist yet. + * Note: Use authz->pool instead of pool as the cache must have the + * same lifetime as authz. + */ + if (!(user && authz->cached_user + && strcmp(user, authz->cached_user) == 0)) + { + if (user) + { + authz->cache = NULL; + authz->cached_user = apr_pstrdup(authz->pool, user); + } + else if (authz->cached_user) + { + authz->cache = NULL; + authz->cached_user = NULL; + } + } + if (!authz->cache) + authz->cache = apr_hash_make(authz->pool); + if (!repos_name) repos_name = ""; /* If PATH is NULL, check if the user has *any* access. */ if (!path) { - *access_granted = authz_get_any_access(authz->cfg, repos_name, + *access_granted = authz_get_any_access(authz, repos_name, user, required_access, pool); return SVN_NO_ERROR; } @@ -1046,11 +1143,9 @@ path = svn_fspath__canonicalize(path, pool); current_path = path; - while (!authz_get_path_access(authz->cfg, repos_name, + while (!authz_get_path_access(authz, repos_name, current_path, user, - required_access, - access_granted, - pool)) + required_access, access_granted, pool)) { /* Stop if the loop hits the repository root with no results. */ @@ -1069,7 +1164,7 @@ the entire authz config to see whether any child paths are denied to the requested user. */ if (*access_granted && (required_access & svn_authz_recursive)) - *access_granted = authz_get_tree_access(authz->cfg, repos_name, path, + *access_granted = authz_get_tree_access(authz, repos_name, path, user, required_access, pool); return SVN_NO_ERROR;