On 12 March 2015 at 07:36, Branko Čibej <br...@wandisco.com> wrote: > On 09.03.2015 12:39, rhuij...@apache.org wrote: >> Author: rhuijben >> Date: Mon Mar 9 11:39:39 2015 >> New Revision: 1665195 >> >> URL: http://svn.apache.org/r1665195 >> Log: >> In ra-serf: stop handling HTTP status 405 'Method forbidden' as a generic >> out of date error. When locking this is the error we get when the resource >> does not exist in HEAD, but in general it tells us that there is an >> authorization problem. >> [...]
> >> Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c >> URL: >> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1665195&r1=1665194&r2=1665195&view=diff >> ============================================================================== >> --- subversion/trunk/subversion/libsvn_ra_serf/util.c (original) >> +++ subversion/trunk/subversion/libsvn_ra_serf/util.c Mon Mar 9 11:39:39 >> 2015 >> @@ -1762,8 +1762,8 @@ svn_ra_serf__error_on_status(serf_status >> return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL, >> _("'%s' path not found"), path); >> case 405: >> - return svn_error_createf(SVN_ERR_FS_OUT_OF_DATE, NULL, >> - _("'%s' is out of date"), path); >> + return svn_error_createf(SVN_ERR_RA_DAV_FORBIDDEN, NULL, >> + _("HTTP method is not allowed on '%s'"), >> path); >> case 409: >> return svn_error_createf(SVN_ERR_FS_CONFLICT, NULL, >> _("'%s' conflicts"), path); > > -1, please revert this. > > * SVN_ERR_RA_DAV_FORBIDDEN is the wrong error code to use here. > "Forbidden" is 403, not 405; the difference is significant. I agree: SVN_ERR_RA_DAV_FORBIDDEN is wrong error code. New error code is needed for proper fix and as I far I see it was added in r1666379. > * Far from being a "better response," the new error message is > arguably more confusing to users than the old one. Most Subversion > users understand "out of date," but I bet most don't know what an I find it useful when error message contains some generic explanation of problem and some raw details from on second line for diagnostic purpose. > HTTP method is. > * Even if we decided that it's OK to confuse users with the details of > the HTTP protocol, you'd have to tell them *which* method is not > allowed. > +1. -- Ivan Zhakov
