[PATCH/RFC] Lock refreshing problem in Subversion DAV layer

Hi!

I've discovered another problem related to locks.

Environment:

  Server:
    Windows Server 2008 R2 SP1
    Subversion 1.8.8,
    Apache HTTP Server 2.2.25.

    Autoversioning is enabled in Apache HTTP Server's settings:
    http://svnbook.red-bean.com/en/1.8/svn.webdav.autoversioning.html

  Client:
    Windows 7 SP1 64-bit,
    Internet Explorer 11.0.9600.16428.
    Microsoft Office 2010 (14.0.6023.1000 32-bit)

Description:

A file that has been opened via WebDAV in Microsoft Office application (e.g.
Word, Excel, PowerPoint etc) can't be saved back after applying changes to
it.
For example, MS Excel errors-out with the error (see the attached
screenshot):

[[
Upload Failed
This file is locked for editing by another user.
]]

The file is locked indeed, however it's locked by the same user who runs
Excel
so it's unexpected for a user to see such error.

Apache's log:

[[
Could not LOCK /svn/NewRepo/Book1237.xlsx due to a failed precondition (e.g.
other locks). [412, #0]
The precondition(s) specified by the "If:" header did not match this
resource.
At least one failure is because: a State-token was supplied, but it was not
found in the locks on this resource. [412, #0]
Path '/Book1237.xlsx' is already locked by user 'user' in filesystem
'93d13ee0-0fdb-6045-8702-4113dc399b89' [423, #160035]
Failed to create new lock. [423, #160035]
]]

What actually happens here: Excel tries to refresh existing lock using an
"If"
header (see section 7.8 of RFC 2518 [1]). The supplied lock token is valid
but server responds with '412 Precondition Failed' instead of '200 OK'.
 Excel
tries to obtain a new lock on opened file, but request expectedly fails.

After some digging, I found out that correct behaviour was broken in
r859583 [2]
when fixing issue 2275 [3].

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?

  [1] https://tools.ietf.org/html/rfc2518
  [2] http://svn.apache.org/r859583
  [3] http://subversion.tigris.org/issues/show_bug.cgi?id=2275

Thanks and regards,
Sergey Raevskiy