On Mon, Jun 3, 2013 at 3:31 PM, Greg Stein <gst...@gmail.com> wrote: > On Mon, Jun 3, 2013 at 6:09 PM, Philip Martin > <philip.mar...@wandisco.com> wrote: >> Greg Stein <gst...@gmail.com> writes: >> >>> On Mon, Jun 3, 2013 at 5:14 PM, Philip Martin >>> <philip.mar...@wandisco.com> wrote: >>>> http://subversion.tigris.org/issues/show_bug.cgi?id=4368 >>>> >>>> Locking with anonymous http fails because there is no username. At
That's my fault, I fixed the anonymous locking segfault and chose a 401 to return but I forgot about the WWW-Authenticate header requirement. >> What is mod_dav_svn going to do, invent a username? > > Ah. Well... I would say "yes, invent a username". "_unknown_" or somesuch. > > But that's just me :-) That's problematic, some people want to enforce that only the lock owner can remove the locks and so on. I also think supporting entirely anonymous locks is an almost entirely pointless exercise given our current limitations. > I'm not sure if we can determine whether authn/authz is configured for > the particular path. > > Oh. Wait. The config should prevent the LOCK even *before* it gets to > our code. IOW, if our code is reached *without* a username, then > authn/authz is not configured. This is indeed correct. When authentication is configured correctly (even on anonymous read allowed setups) a LOCK will be rejected before it even gets to us. The anonymous lock issue only happens when you have a repo setup to allow entirely anonymous commits, which is effectively nobody. At least until we have something like obliterate where cleaning up the crap people would add to your repo is possible. > 412 (Precondition Failed) is not appropriate, as there are no > preconditions defined in the headers (eg. an "If" header). The proper > response is 409 (Conflict). The client *may* be able to proactively > provide authentication information, but we can't use 401 to automate > the authn provision (for the lack of WWW-Authenticate, as you noted). Agreed. >>>> present mod_dav_svn returns "401 Unathorized" however >>> >>> Do you know where/why this is happening? >> >> mod_dav_svn/locks.c:append_locks. > > Yeah. HTTP_CONFLICT should be correct, there. Well technically 401 is right but we have now way of filling in the proper WWW-Authenticate header. HTTP_CONFLICT doesn't sound particularly great either because that's supposed to be describing an issue with the state of the resource. I'd argue that we should return a 500 range error since the problem here is that the server is not properly configured. There is really nothing a client can do to resolve the issue other than to authenticate, which our client is only going to do if the server is setup properly. So I'd vote for returning HTTP_INTERNAL_SERVER_ERROR.
