subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko ─îibej <br...@wandisco.com>
Subject Re: svn commit: r1665195 - /subversion/trunk/subversion/libsvn_ra_serf/util.c
Date Thu, 12 Mar 2015 04:36:35 GMT
On 09.03.2015 12:39, rhuijben@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.
>
> As the lock code has its own http status handling now, we can change
> the error reported from the generic error handling code path.
>
> This should give users that accidentally use anonymous 'http' on a
> server that uses 'https' for authorized operations a much better response,
> than a simple out of date error (with the recommendation to update added
> by their client).
>
> Note that in most cases the detailed error response from the server
> is used instead of this generic error code for just the HTTP status.
>
> * subversion/libsvn_ra_serf/util.c
>   (svn_ra_serf__error_on_status): Tweak generic error for http status 405.
>
> Modified:
>     subversion/trunk/subversion/libsvn_ra_serf/util.c
>
> 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.
  * 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
    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.

-- Brane


Mime
View raw message