Sergey Raevskiy wrote on Wed, Apr 02, 2014 at 13:28:04 +0400:
> There is a hack in mod_dav_svn needed for support concept of lock stealing:
>
> [[[
> /* The Big Lie: if the client ran 'svn lock', then we have
> to pretend that there's no existing lock. Otherwise mod_dav will
> throw '403 Locked' without even attempting to create a new
> lock. For the --force case, this is required and for the non-force case,
> we allow the filesystem to produce a better error for svn clients.
> */
> if (info->r->method_number == M_LOCK)
> {
> *locks = NULL;
> return 0;
> }
> ]]]
>
> Before changeset [2] the condition was 'if (info->lock_steal)' and it worked
> correctly in case of lock refreshing. Now mod_dav_svn pretends that there
> is no
> locks set on resource for *ANY* LOCK request and that's why client getting
> status '412 Precondition failed'.
>
> I've attached patch with failing test that demonstrates this problem but
> I'm not
> sure how to fix it. I see two ways:
>
> 1. Revert changeset [2] and look for another way to fix issue [3].
> 2. Extend condition with checking if an 'If' header is present.
>
> Do you have any suggestions about it?
I'm not a DAV expert, but a few random thoughts:
We can behave differently for svn clients and for DAV clients (see
repos->is_svn_client). Would it work to restrict the lie to svn clients
only, and just follow the spec for DAV clients?
It might be helpful to enumerate all possible cases here (svn client v.
DAV client; lock exists or not; "If" header matches or not; etc) and
explicitly check that the code DTRTs for each of them.
Cheers,
Daniel
> [1] https://tools.ietf.org/html/rfc2518
> [2] http://svn.apache.org/r859583
> [3] http://subversion.tigris.org/issues/show_bug.cgi?id=2275
Received on 2014-04-03 17:01:29 CEST