On 12 March 2015 at 07:36, Branko Čibej <brane_at_wandisco.com> wrote:
> On 09.03.2015 12:39, rhuijben_at_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
Received on 2015-03-13 14:17:44 CET