You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2016/12/18 14:01:18 UTC
svn commit: r1774890 - in /subversion/branches/authzperf/subversion:
libsvn_repos/authz.c libsvn_repos/authz.h libsvn_repos/authz_info.c
libsvn_repos/authz_parse.c tests/libsvn_repos/repos-test.c
Author: stefan2
Date: Sun Dec 18 14:01:18 2016
New Revision: 1774890
URL: http://svn.apache.org/viewvc?rev=1774890&view=rev
Log:
On the authzperf branch:
Correctly implement the "per-repos rule before global rule" principle.
The crux with SVN's old lookup code was that it would look for a matching
rule first and only then check whether it applied to the current user.
Hence, we need to build the filtered pre-repo-and-user tree in a two-step
process: Collect all rules for the repo as well as the global rules and
eliminate global rules where per-repo rules exist for the same path.
Examine and filter the result by ACL afterwards.
* subversion/libsvn_repos/authz.h
(svn_authz__compare_paths,
svn_authz__acl_applies_to_repo): Declare new private API for functions
factored out from other private API.
* subversion/libsvn_repos/authz_info.c
(svn_authz__acl_applies_to_repo): Factored out from ...
(svn_authz__get_acl_access): ... this one.
* subversion/libsvn_repos/authz_parse.c
(svn_authz__compare_paths): Factored out from ...
(svn_authz__compare_rules): ... this one.
* subversion/libsvn_repos/authz.c
(create_user_authz): Resolve rule precedence before filtering.
* subversion/tests/libsvn_repos/repos-test.c
(test_authz_prefixes): Correct the test expectations as precedence rules
are different now for reasons of backward compat.
Modified:
subversion/branches/authzperf/subversion/libsvn_repos/authz.c
subversion/branches/authzperf/subversion/libsvn_repos/authz.h
subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c
subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c
subversion/branches/authzperf/subversion/tests/libsvn_repos/repos-test.c
Modified: subversion/branches/authzperf/subversion/libsvn_repos/authz.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/libsvn_repos/authz.c?rev=1774890&r1=1774889&r2=1774890&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/libsvn_repos/authz.c (original)
+++ subversion/branches/authzperf/subversion/libsvn_repos/authz.c Sun Dec 18 14:01:18 2016
@@ -886,9 +886,40 @@ create_user_authz(authz_full_t *authz,
/* Use a separate sub-pool to keep memory usage tight. */
apr_pool_t *subpool = svn_pool_create(scratch_pool);
- /* Filtering and tree construction. */
+ /* Find all ACLs for REPOSITORY.
+ * Note that repo-specific rules replace global rules,
+ * even if they don't apply to the current user. */
+ apr_array_header_t *acls = apr_array_make(subpool, authz->acls->nelts,
+ sizeof(authz_acl_t *));
for (i = 0; i < authz->acls->nelts; ++i)
- process_acl(ctx, &APR_ARRAY_IDX(authz->acls, i, authz_acl_t),
+ {
+ const authz_acl_t *acl = &APR_ARRAY_IDX(authz->acls, i, authz_acl_t);
+ if (svn_authz__acl_applies_to_repo(acl, repository))
+ {
+ /* ACLs in the AUTHZ are sorted by path and repository.
+ * So, if there is a rule for the repo and a global rule for the
+ * same path, we will detect them here. */
+ if (acls->nelts)
+ {
+ const authz_acl_t *prev_acl
+ = APR_ARRAY_IDX(acls, acls->nelts - 1, const authz_acl_t *);
+ if (svn_authz__compare_paths(&prev_acl->rule, &acl->rule) == 0)
+ {
+ SVN_ERR_ASSERT_NO_RETURN(!strcmp(prev_acl->rule.repos,
+ AUTHZ_ANY_REPOSITORY));
+ SVN_ERR_ASSERT_NO_RETURN(strcmp(acl->rule.repos,
+ AUTHZ_ANY_REPOSITORY));
+ apr_array_pop(acls);
+ }
+ }
+
+ APR_ARRAY_PUSH(acls, const authz_acl_t *) = acl;
+ }
+ }
+
+ /* Filtering and tree construction. */
+ for (i = 0; i < acls->nelts; ++i)
+ process_acl(ctx, APR_ARRAY_IDX(acls, i, const authz_acl_t *),
root, repository, user, result_pool, subpool);
/* If there is no relevant rule at the root node, the "no access" default
Modified: subversion/branches/authzperf/subversion/libsvn_repos/authz.h
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/libsvn_repos/authz.h?rev=1774890&r1=1774889&r2=1774890&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/libsvn_repos/authz.h (original)
+++ subversion/branches/authzperf/subversion/libsvn_repos/authz.h Sun Dec 18 14:01:18 2016
@@ -312,6 +312,10 @@ void
svn_authz__reverse_string(char *string, apr_size_t len);
+/* Compare two rules in lexical order by path only. */
+int
+svn_authz__compare_paths(const authz_rule_t *a, const authz_rule_t *b);
+
/* Compare two rules in path lexical order, then repository lexical order. */
int
svn_authz__compare_rules(const authz_rule_t *a, const authz_rule_t *b);
@@ -327,6 +331,10 @@ svn_authz__compare_rules(const authz_rul
/* Rules with this repository name apply to all repositories. */
#define AUTHZ_ANY_REPOSITORY ((const char*)"")
+/* Check if the ACL applies to the REPOS pair. */
+svn_boolean_t
+svn_authz__acl_applies_to_repo(const authz_acl_t *acl,
+ const char *repos);
/* Check if the ACL applies to the (USER, REPOS) pair. If it does,
* and ACCESS is not NULL, set *ACCESS to the actual access rights for
Modified: subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c?rev=1774890&r1=1774889&r2=1774890&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c (original)
+++ subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c Sun Dec 18 14:01:18 2016
@@ -32,6 +32,16 @@
svn_boolean_t
+svn_authz__acl_applies_to_repo(const authz_acl_t *acl,
+ const char *repos)
+{
+ /* The repository name must match the one in the rule, iff the rule
+ was defined for a specific repository. */
+ return (0 == strcmp(acl->rule.repos, AUTHZ_ANY_REPOSITORY))
+ || (0 == strcmp(repos, acl->rule.repos));
+}
+
+svn_boolean_t
svn_authz__get_acl_access(authz_access_t *access_p,
const authz_acl_t *acl,
const char *user, const char *repos)
@@ -42,8 +52,7 @@ svn_authz__get_acl_access(authz_access_t
/* The repository name must match the one in the rule, iff the rule
was defined for a specific repository. */
- if (strcmp(acl->rule.repos, AUTHZ_ANY_REPOSITORY)
- && strcmp(repos, acl->rule.repos))
+ if (!svn_authz__acl_applies_to_repo(acl, repos))
return FALSE;
/* Check anonymous access first. */
Modified: subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c?rev=1774890&r1=1774889&r2=1774890&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c (original)
+++ subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c Sun Dec 18 14:01:18 2016
@@ -1397,7 +1397,7 @@ svn_authz__reverse_string(char *string,
int
-svn_authz__compare_rules(const authz_rule_t *a, const authz_rule_t *b)
+svn_authz__compare_paths(const authz_rule_t *a, const authz_rule_t *b)
{
const int min_len = (a->len > b->len ? b->len : a->len);
int i;
@@ -1424,6 +1424,16 @@ svn_authz__compare_rules(const authz_rul
if (a->len != b->len)
return a->len - b->len;
+ return 0;
+}
+
+int
+svn_authz__compare_rules(const authz_rule_t *a, const authz_rule_t *b)
+{
+ int diff = svn_authz__compare_paths(a, b);
+ if (diff)
+ return diff;
+
/* Repository names are interned, too. */
if (a->repos != b->repos)
return strcmp(a->repos, b->repos);
Modified: subversion/branches/authzperf/subversion/tests/libsvn_repos/repos-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/tests/libsvn_repos/repos-test.c?rev=1774890&r1=1774889&r2=1774890&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/tests/libsvn_repos/repos-test.c (original)
+++ subversion/branches/authzperf/subversion/tests/libsvn_repos/repos-test.c Sun Dec 18 14:01:18 2016
@@ -1576,7 +1576,7 @@ test_authz_wildcard_performance(apr_pool
}
/* Test that the latest definition wins, regardless of whether the ":glob:"
- * or the repo prefix has been given. */
+ * prefix has been given. */
static svn_error_t *
test_authz_prefixes(apr_pool_t *pool)
{
@@ -1602,7 +1602,7 @@ test_authz_prefixes(apr_pool_t *pool)
const char *test_paths[PATH_COUNT] = { "/", "/A" };
/* Definition of the paths to test and expected replies for each. */
- struct check_access_tests test_set[] = {
+ struct check_access_tests test_set1[] = {
/* Test that read rules are correctly used. */
{ "", "greek", NULL, svn_authz_read, FALSE },
/* Test that write rules are correctly used. */
@@ -1612,6 +1612,19 @@ test_authz_prefixes(apr_pool_t *pool)
{ NULL, NULL, NULL, svn_authz_none, FALSE }
};
+ /* To be used when global rules are specified after per-repos rules.
+ * In that case, the global rules still win. */
+ struct check_access_tests test_set2[] = {
+ /* Test that read rules are correctly used. */
+ { "", "greek", NULL, svn_authz_read, TRUE },
+ { "", "greek", NULL, svn_authz_write, FALSE },
+ /* Test that write rules are correctly used. */
+ { "", "greek", "plato", svn_authz_read, TRUE },
+ { "", "greek", "plato", svn_authz_write, TRUE },
+ /* Sentinel */
+ { NULL, NULL, NULL, svn_authz_none, FALSE }
+ };
+
/* There is a total of 16 combinations of authz content. */
for (combi = 0; combi < 16; ++combi)
{
@@ -1621,6 +1634,7 @@ test_authz_prefixes(apr_pool_t *pool)
const char *repo1 = (combi & 4) ? "greek:" : "";
const char *repo2 = (combi & 4) ? "" : "greek:";
const char *test_path = test_paths[combi / 8];
+ struct check_access_tests *test_set = (combi & 4) ? test_set2 : test_set1;
/* Create and parse the authz rules. */
svn_pool_clear(iterpool);