subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philip Martin <philip.mar...@wandisco.com>
Subject serf error handling for locks without authn
Date Mon, 03 Jun 2013 21:14:47 GMT
http://subversion.tigris.org/issues/show_bug.cgi?id=4368

Locking with anonymous http fails because there is no username.  At
present mod_dav_svn returns "401 Unathorized" however

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.2

says that a 401 response MUST include a WWW-Authenticate header and
apache's response doesn't include such as header as authn is not
configured.  That looks as if mod_dav_svn is returning invalid
HTTP.  Should we return something else?  Perhaps "501 Not Implemented"?

The WebDAV RFC 2518 gives "412 Precondition Failed" as a possible error,
although I'm not clear whether that would be suitable for this request.
If I change mod_dav_svn to return a 412 instead of 401 I see what looks
like a ra_serf bug:

$ svn lock http://localhost:8888/obj/repo/A/f
'f' locked by user '(null)'.

The 412 is treated as success!  The code in
libsvn_ra_serf/locks.c:determine_error looks dodgy:

213 static svn_error_t *
214 determine_error(svn_ra_serf__handler_t *handler,
215                 svn_error_t *err)
216 {
217     {
218       apr_status_t errcode;
219 
220       if (handler->sline.code == 423)
221         errcode = SVN_ERR_FS_PATH_ALREADY_LOCKED;
222       else if (handler->sline.code == 403)
223         errcode = SVN_ERR_RA_DAV_FORBIDDEN;
224       else
225         return err;
226 
227       /* Client-side or server-side error already. Return it.  */
228       if (err != NULL)
229         return err;
230 
231       /* The server did not send us a detailed human-readable error.
232          Provide a generic error.  */
233       err = svn_error_createf(errcode, NULL,
234                               _("Lock request failed: %d %s"),
235                               handler->sline.code,
236                               handler->sline.reason);
237     }
238 
239   return err;
240 }

Lines 224-225 mean that all the following code is never executed and
anything other than 423 or 403 is going to get lost.  I can't simply
remove those two lines because errcode needs a value for
svn_error_createf.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Mime
View raw message