Doug Robinson: Would you please cast your eye over this, as an affected
party? Basically I'd ask if you want to offer your take on fixing this
authz semantics regression?
I haven't been following this issue; I've just noticed it. On quick
inspection it seems to me after two LTS releases with changed semantics,
it might not be appropriate to change it back unconditionally in point
releases, as this could be just as bad for those who have begun relying
on the new semantics. Instead it would seem better to seek some way to
take account of both groups of users, those who are relying on each
version of the semantics. But I am not familiar with the nuances of
this particular change and how it may affect users in practice, to judge
what is best.
References:
* 2018-07-04 users@ email "svn 1.10: mod_authz no longer combines ACL
entries"
https://lists.apache.org/thread.html/1aef30b70b4063ecfa24f17be0e1c9998837b4f8b3f7e8b03de4e240@%3Cusers.subversion.apache.org%3E
* 2018-07-10 issue 4762 "authz doesn't combine global and repository rules"
https://subversion.apache.org/issue/4762
* 2020-10-01 "Fix issue #4762 ..."
http://svn.apache.org/r1882186
(Subject line changed for visibility; cc added.)
Daniel Shahaf wrote:
s...@apache.org wrote on Thu, 01 Oct 2020 19:36 +00:00:
Fix issue #4762 "authz doesn't combine global and repository rules"
The new authz implementation of SVN 1.10 introduced an incompatible change
in the interpretation of authz rules: Global rules access were not
considered if per-repository access rules were also supplied.
This change seems entirely unnecessary
I'm wary of this justification. The semantics being changed here were
covered by tests and, according to your very log message, were likely
implemented deliberately. Reverting such semantics requires better
arguments than "they seem unnecessary". That argument is exactly how
the Debian OpenSSL bug was introduced.
I think we should more seriously investigate the possibility that _this_
change, r1882186, will break some use-case or another.
(To be clear, I'm disagreeing only with the _justification_ for the
change, not with the change itself. I'm not commenting on the
change itself.)
and is still causing problems today
for deployments which upgrade from earlier versions, such as from SVN 1.8.
Existing authz files no longer produce expected results and adjusting
large existing authz rule files to avoid this problem is a lot of work.
Sounds like this change merits an entry in the 1.14 release notes (if
it's backported) or in the 1.15 release notes if it's not backported.
Would you please add a placeholder (just a section header or a ToC
link) or file a corresponding issue, so we don't forget to document
this change?
* subversion/libsvn_repos/authz.c
(authz_check_access): New helper function, extracted from ...
(svn_repos_authz_check_access): ... here. Always consider the global
rule set in addition to the repository-specific one. The results
from both rulesets are now merged as was the case before SVN 1.10.
* subversion/tests/cmdline/svnauthz_tests.py
(svnauthz_accessof_file_test,
svnauthz_accessof_repo_test,
svnauthz_accessof_groups_file_test,
svnauthz_accessof_groups_repo_test,
svnauthz_accessof_is_file_test,
svnauthz_accessof_is_repo_test): Adjust test expectations accordingly.
* subversion/tests/libsvn_repos/authz-test.c
(reposful_reposless_stanzas_inherit): Remove XFAIL marker.
* subversion/tests/libsvn_repos/repos-test.c
(test_authz_prefixes): Adjust test expectations. This test shows that
the behaviour change was likely deliberate. This test assumed that
global rules would only apply if listed after per-repository rules.
Change test expectations such that global rules are always taken
into account.
