[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1665195 - /subversion/trunk/subversion/libsvn_ra_serf/util.c

From: Branko Čibej <brane_at_wandisco.com>
Date: Thu, 12 Mar 2015 05:36:35 +0100

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.
>
> 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
Received on 2015-03-12 05:38:38 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.