[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1882186 - in /subversion/trunk/subversion: libsvn_repos/authz.c tests/cmdline/svnauthz_tests.py tests/libsvn_repos/authz-test.c tests/libsvn_repos/repos-test.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sat, 03 Oct 2020 06:07:49 +0000

stsp_at_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.
Received on 2020-10-03 08:08:25 CEST

This is an archived mail posted to the Subversion Dev mailing list.