On Tue, Oct 06, 2020 at 12:21:27PM +0100, Julian Foad wrote:
> 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.
Note that the regression tests added for 1.10 still allowed for global
and per-repo rules to be merged. The code assumes global rules to
be listed _after_ repository-specific rules:
https://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_repos/repos-test.c?r1=1882186&r2=1882185&pathrev=1882186
[[[
/* To be used when global rules are specified after per-repos rules.
* In that case, the global rules still win. */
]]]
Conceivably, somebody could now have written and tested a ruleset which
prevents access to a path only because global rules are listed before
per-repository rules.
Unfortunately we cannot know whether such cases exist in the wild.
My problem right now is that people are upgrading servers with hundresds
of repositories and hundreds of lines of already vetted authz files and
they will need to test their entire ruleset again. In this light, the
benefit of the behaviour change made in 1.10 is entirely unclear to me.
Perhaps we could add a flag somewhere which specifies whether global
rules should be merged with per-repository rules, but that doesn't make
the suggested fix less problematic. In a patch release we usually promise
that a roll-back with existing config files remains possible. If we take
this strictly, then any fix for issue #4762 would have to wait for 1.15,
regardless of whether we retain both and new behaviours or just revert
back to the old behaviour.
In my opinion the original change was a mistake which should be corrected.
I think the benefits of seamless upgrades of existing deployments outweight
concerns over potential regressions in fresh deployments (which, let's admit
it, are getting rare these days).
If a fix would have to wait for 1.15, so be it.
Received on 2020-10-06 15:37:04 CEST