subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stef...@apache.org
Subject 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
Date Sun, 18 Dec 2016 14:01:18 GMT
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);



Mime
View raw message