Patchwork [3,of,3,RFC] hgweb: handle obsolete changesets gracefully

# HG changeset patch
# User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
# Date 1375624663 -7200
#      Sun Aug 04 15:57:43 2013 +0200
# Node ID 5507e2af80f5e8026a054ce440c0c8958134eefe
# Parent  b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
hgweb: handle obsolete changesets gracefully

If the changeset has any successors, issue a 403 Moved Permanently;
otherwise we issue a 410 Gone. Please note that this is slightly
misleading for 'secret' changesets, as they may appear later on.

On Thu, Aug 08, 2013 at 09:48:46AM +0200, Dan Villiom Podlaski Christiansen wrote:
> # HG changeset patch
> # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
> # Date 1375624663 -7200
> #      Sun Aug 04 15:57:43 2013 +0200
> # Node ID 5507e2af80f5e8026a054ce440c0c8958134eefe
> # Parent  b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
> hgweb: handle obsolete changesets gracefully
>
> If the changeset has any successors, issue a 403 Moved Permanently;

I think you mean 301?

> otherwise we issue a 410 Gone. Please note that this is slightly
> misleading for 'secret' changesets, as they may appear later on.

I think you should handle these separately if possible, since secret
changesets probably want to be 404. Also, you should verify that the
destination of the 301 isn't a 404 before redirecting there, because
that /would/ potentially leak some secret changeset hash IDs.

>
> diff --git a/mercurial/hgweb/common.py b/mercurial/hgweb/common.py
> --- a/mercurial/hgweb/common.py
> +++ b/mercurial/hgweb/common.py
> @@ -9,12 +9,14 @@
>  import errno, mimetypes, os
>
>  HTTP_OK = 200
> +HTTP_MOVED_PERMANENTLY = 301
>  HTTP_NOT_MODIFIED = 304
>  HTTP_BAD_REQUEST = 400
>  HTTP_UNAUTHORIZED = 401
>  HTTP_FORBIDDEN = 403
>  HTTP_NOT_FOUND = 404
>  HTTP_METHOD_NOT_ALLOWED = 405
> +HTTP_GONE = 410
>  HTTP_SERVER_ERROR = 500
>
>
> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
> --- a/mercurial/hgweb/hgweb_mod.py
> +++ b/mercurial/hgweb/hgweb_mod.py
> @@ -8,11 +8,13 @@
>
>  import os
>  from mercurial import ui, hg, hook, error, encoding, templater, util, repoview
> +from mercurial import obsolete
> +from mercurial.node import short
>  from mercurial.templatefilters import websub
>  from mercurial.i18n import _
>  from common import get_stat, ErrorResponse, permhooks, caching
> -from common import HTTP_OK, HTTP_NOT_MODIFIED, HTTP_BAD_REQUEST
> -from common import HTTP_NOT_FOUND, HTTP_SERVER_ERROR
> +from common import HTTP_OK, HTTP_NOT_MODIFIED, HTTP_BAD_REQUEST, HTTP_GONE
> +from common import HTTP_NOT_FOUND, HTTP_SERVER_ERROR, HTTP_MOVED_PERMANENTLY
>  from request import wsgirequest
>  import webcommands, protocol, webutil, re
>
> @@ -249,6 +251,33 @@ class hgweb(object):
>
>              return content
>
> +        except error.FilteredLookupError, err:
> +            succsets = obsolete.successorssets(self.repo, err.rev)
> +
> +            if not succsets:
> +                req.respond(HTTP_GONE, ctype)
> +
> +                return tmpl('error', error=err.message)
> +
> +            elif len(succsets) != 1:
> +                # TODO: changeset has divergent successors
> +                req.respond(HTTP_SERVER_ERROR, ctype)
> +
> +                return tmpl('error', error=err.message)
> +
> +            location = [req.url.rstrip('/')]
> +            location += req.form['cmd']
> +
> +            location.append(short(succsets[0][-1]))
> +
> +            if 'file' in req.form:
> +                location += req.form['file']
> +
> +            req.headers.extend([('Location', '/'.join(location))])
> +            req.respond(HTTP_MOVED_PERMANENTLY, ctype)
> +
> +            return tmpl('error', error=err.message)
> +
>          except (error.LookupError, error.RepoLookupError), err:
>              req.respond(HTTP_NOT_FOUND, ctype)
>              msg = str(err)
> diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
> --- a/mercurial/hgweb/webutil.py
> +++ b/mercurial/hgweb/webutil.py
> @@ -201,6 +201,9 @@ def cleanpath(repo, path):
>  def changeidctx (repo, changeid):
>      try:
>          ctx = repo[changeid]
> +    except error.FilteredLookupError:
> +        raise
> +
>      except error.RepoError:
>          man = repo.manifest
>          ctx = repo[man.linkrev(man.rev(man.lookup(changeid)))]
> diff --git a/tests/test-hgweb-commands.t b/tests/test-hgweb-commands.t
> --- a/tests/test-hgweb-commands.t
> +++ b/tests/test-hgweb-commands.t
> @@ -1385,7 +1385,7 @@ proper status for filtered revision
>    $ PATH_INFO=/rev/5; export PATH_INFO
>    $ QUERY_STRING='style=raw'
>    $ python hgweb.cgi #> search
> -  Status: 404 Not Found\r (esc)
> +  Status: 410 Gone\r (esc)
>    ETag: *\r (glob) (esc)
>    Content-Type: text/plain; charset=ascii\r (esc)
>    \r (esc)
> @@ -1399,7 +1399,7 @@ proper status for filtered revision
>    $ PATH_INFO=/rev/4; export PATH_INFO
>    $ QUERY_STRING='style=raw'
>    $ python hgweb.cgi #> search
> -  Status: 404 Not Found\r (esc)
> +  Status: 410 Gone\r (esc)
>    ETag: *\r (glob) (esc)
>    Content-Type: text/plain; charset=ascii\r (esc)
>    \r (esc)
> @@ -1498,6 +1498,47 @@ filtered '0' changeset
>
>
>
> +Test obsolete redirection
> +
> +  $ cat > ../obs.py << EOF
> +  > import mercurial.obsolete
> +  > mercurial.obsolete._enabled = True
> +  > EOF
> +  $ echo '[extensions]' >> $HGRCPATH
> +  $ echo "rebase=" >> $HGRCPATH
> +  $ echo "obs=${TESTTMP}/obs.py" >> $HGRCPATH
> +  $ hg up 12
> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ echo B > b; hg add b
> +  $ hg ci -m 6
> +
> +Use a rebase to obsolete r13, and test redirection
> +
> +  $ hg rebase -r 13 -d 11
> +  $ PATH_INFO=/rev/13; export PATH_INFO
> +  $ QUERY_STRING='style=raw'
> +  $ python hgweb.cgi #> search
> +  Status: 301 Moved Permanently\r (esc)
> +  ETag: *\r (glob) (esc)
> +  Location: /test/rev/0d601cf5c587\r (esc)
> +  Content-Type: text/plain; charset=ascii\r (esc)
> +  \r (esc)
> +
> +  error: revision 13 is hidden
> +
> +Rebase r13 once more, testing divergent changesets
> +
> +  $ hg --hidden rebase -r 13 -d 9
> +  $ PATH_INFO=/rev/13; export PATH_INFO
> +  $ QUERY_STRING='style=raw'
> +  $ python hgweb.cgi #> search
> +  Status: 500 Internal Server Error\r (esc)
> +  ETag: *\r (glob) (esc)
> +  Content-Type: text/plain; charset=ascii\r (esc)
> +  \r (esc)
> +
> +  error: revision 13 is hidden
> +
>
>
>    $ cd ..
> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
> --- a/tests/test-obsolete.t
> +++ b/tests/test-obsolete.t
> @@ -747,7 +747,7 @@ check filelog view
>    $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/68'
>    200 Script output follows
>    $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/67'
> -  404 Not Found
> +  410 Gone
>    [1]
>
>  check that web.view config option:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:

> # HG changeset patch
> # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
> # Date 1375624663 -7200
> #      Sun Aug 04 15:57:43 2013 +0200
> # Node ID 5507e2af80f5e8026a054ce440c0c8958134eefe
> # Parent  b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
> hgweb: handle obsolete changesets gracefully
> 
> If the changeset has any successors, issue a 403 Moved Permanently;
> otherwise we issue a 410 Gone. Please note that this is slightly
> misleading for 'secret' changesets, as they may appear later on.

You are going too fast here.

1) issue 410 for filtered changeset
1.1) (403 or 404 for secret ?)
2) issue a smarter message with potential link to successors (could include information about who and when it was rewritten)
3) use redirect (does not like the idea yet)

> diff --git a/mercurial/hgweb/common.py b/mercurial/hgweb/common.py
> --- a/mercurial/hgweb/common.py
> +++ b/mercurial/hgweb/common.py
> @@ -9,12 +9,14 @@
> import errno, mimetypes, os
> 
> HTTP_OK = 200
> +HTTP_MOVED_PERMANENTLY = 301
> HTTP_NOT_MODIFIED = 304
> HTTP_BAD_REQUEST = 400
> HTTP_UNAUTHORIZED = 401
> HTTP_FORBIDDEN = 403
> HTTP_NOT_FOUND = 404
> HTTP_METHOD_NOT_ALLOWED = 405
> +HTTP_GONE = 410
> HTTP_SERVER_ERROR = 500
> 
> 
> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
> --- a/mercurial/hgweb/hgweb_mod.py
> +++ b/mercurial/hgweb/hgweb_mod.py
> @@ -8,11 +8,13 @@
> 
> import os
> from mercurial import ui, hg, hook, error, encoding, templater, util, repoview
> +from mercurial import obsolete
> +from mercurial.node import short
> from mercurial.templatefilters import websub
> from mercurial.i18n import _
> from common import get_stat, ErrorResponse, permhooks, caching
> -from common import HTTP_OK, HTTP_NOT_MODIFIED, HTTP_BAD_REQUEST
> -from common import HTTP_NOT_FOUND, HTTP_SERVER_ERROR
> +from common import HTTP_OK, HTTP_NOT_MODIFIED, HTTP_BAD_REQUEST, HTTP_GONE
> +from common import HTTP_NOT_FOUND, HTTP_SERVER_ERROR, HTTP_MOVED_PERMANENTLY
> from request import wsgirequest
> import webcommands, protocol, webutil, re
> 
> @@ -249,6 +251,33 @@ class hgweb(object):
> 
>             return content
> 
> +        except error.FilteredLookupError, err:
> +            succsets = obsolete.successorssets(self.repo, err.rev)
> +
> +            if not succsets:
> +                req.respond(HTTP_GONE, ctype)
> +
> +                return tmpl('error', error=err.message)
> +
> +            elif len(succsets) != 1:
> +                # TODO: changeset has divergent successors
> +                req.respond(HTTP_SERVER_ERROR, ctype)

divergent successors are not "server error" (my best guest for now would be "GONE")

> +
> +                return tmpl('error', error=err.message)
> +
> +            location = [req.url.rstrip('/')]
> +            location += req.form['cmd']
> +
> +            location.append(short(succsets[0][-1]))
> +
> +            if 'file' in req.form:
> +                location += req.form['file']
> +
> +            req.headers.extend([('Location', '/'.join(location))])
> +            req.respond(HTTP_MOVED_PERMANENTLY, ctype)
> +
> +            return tmpl('error', error=err.message)
> +
>         except (error.LookupError, error.RepoLookupError), err:
>             req.respond(HTTP_NOT_FOUND, ctype)
>             msg = str(err)
> diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
> --- a/mercurial/hgweb/webutil.py
> +++ b/mercurial/hgweb/webutil.py
> @@ -201,6 +201,9 @@ def cleanpath(repo, path):
> def changeidctx (repo, changeid):
>     try:
>         ctx = repo[changeid]
> +    except error.FilteredLookupError:
> +        raise
> +
>     except error.RepoError:
>         man = repo.manifest
>         ctx = repo[man.linkrev(man.rev(man.lookup(changeid)))]
> diff --git a/tests/test-hgweb-commands.t b/tests/test-hgweb-commands.t
> --- a/tests/test-hgweb-commands.t
> +++ b/tests/test-hgweb-commands.t
> @@ -1385,7 +1385,7 @@ proper status for filtered revision
>   $ PATH_INFO=/rev/5; export PATH_INFO
>   $ QUERY_STRING='style=raw'
>   $ python hgweb.cgi #> search
> -  Status: 404 Not Found\r (esc)
> +  Status: 410 Gone\r (esc)
>   ETag: *\r (glob) (esc)
>   Content-Type: text/plain; charset=ascii\r (esc)
>   \r (esc)
> @@ -1399,7 +1399,7 @@ proper status for filtered revision
>   $ PATH_INFO=/rev/4; export PATH_INFO
>   $ QUERY_STRING='style=raw'
>   $ python hgweb.cgi #> search
> -  Status: 404 Not Found\r (esc)
> +  Status: 410 Gone\r (esc)
>   ETag: *\r (glob) (esc)
>   Content-Type: text/plain; charset=ascii\r (esc)
>   \r (esc)
> @@ -1498,6 +1498,47 @@ filtered '0' changeset
> 
> 
> 
> +Test obsolete redirection
> +
> +  $ cat > ../obs.py << EOF
> +  > import mercurial.obsolete
> +  > mercurial.obsolete._enabled = True
> +  > EOF
> +  $ echo '[extensions]' >> $HGRCPATH
> +  $ echo "rebase=" >> $HGRCPATH
> +  $ echo "obs=${TESTTMP}/obs.py" >> $HGRCPATH
> +  $ hg up 12
> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ echo B > b; hg add b
> +  $ hg ci -m 6
> +
> +Use a rebase to obsolete r13, and test redirection
> +
> +  $ hg rebase -r 13 -d 11
> +  $ PATH_INFO=/rev/13; export PATH_INFO
> +  $ QUERY_STRING='style=raw'
> +  $ python hgweb.cgi #> search
> +  Status: 301 Moved Permanently\r (esc)
> +  ETag: *\r (glob) (esc)
> +  Location: /test/rev/0d601cf5c587\r (esc)
> +  Content-Type: text/plain; charset=ascii\r (esc)
> +  \r (esc)
> +  
> +  error: revision 13 is hidden
> +
> +Rebase r13 once more, testing divergent changesets
> +
> +  $ hg --hidden rebase -r 13 -d 9
> +  $ PATH_INFO=/rev/13; export PATH_INFO
> +  $ QUERY_STRING='style=raw'
> +  $ python hgweb.cgi #> search
> +  Status: 500 Internal Server Error\r (esc)
> +  ETag: *\r (glob) (esc)
> +  Content-Type: text/plain; charset=ascii\r (esc)
> +  \r (esc)
> +  
> +  error: revision 13 is hidden
> +
> 
> 
>   $ cd ..
> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
> --- a/tests/test-obsolete.t
> +++ b/tests/test-obsolete.t
> @@ -747,7 +747,7 @@ check filelog view
>   $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/68'
>   200 Script output follows
>   $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/67'
> -  404 Not Found
> +  410 Gone
>   [1]
> 
> check that web.view config option:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

On 16 August 2013 09:57, Pierre-Yves David
<pierre-yves.david@ens-lyon.org>wrote:

>
> On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:
>
> > # HG changeset patch
> > # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
> > # Date 1375624663 -7200
> > #      Sun Aug 04 15:57:43 2013 +0200
> > # Node ID 5507e2af80f5e8026a054ce440c0c8958134eefe
> > # Parent  b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
> > hgweb: handle obsolete changesets gracefully
> >
> > If the changeset has any successors, issue a 403 Moved Permanently;
> > otherwise we issue a 410 Gone. Please note that this is slightly
> > misleading for 'secret' changesets, as they may appear later on.
>
> You are going too fast here.
>
> 1) issue 410 for filtered changeset
> 1.1) (403 or 404 for secret ?)
> 2) issue a smarter message with potential link to successors (could
> include information about who and when it was rewritten)
> 3) use redirect (does not like the idea yet)


IMO 410 Gone should not be used for filtered changesets - 410 should be
considered to be a permanent condition. The only case I think a 410 would
be appropriate would be an obsolete changeset with no successors (this
situation could change, but that would be unusual).

If an obsolete changeset has multiple successors then I think 409 Conflict
would be appropriate since it is possible for the user to resolve the
conflict and resubmit.

For all other filtered changesets I think 403 or 404 is the most
appropriate response. 403 feels more appropriate to me, but does leak some
information about the filtered changeset that wouldn't be leaked by a 404.
OTOH I personally think that it would be useful to distinguish between a
filtered changeset and a non-existent changeset, so would favour 403.

Tim Delaney

Op 18-08-13 05:18, Tim Delaney schreef:
> On 16 August 2013 09:57, Pierre-Yves David 
> <pierre-yves.david@ens-lyon.org 
> <mailto:pierre-yves.david@ens-lyon.org>> wrote:
>
>
>     On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:
>
>     > # HG changeset patch
>     > # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com
>     <mailto:danchr@gmail.com>>
>     > # Date 1375624663 -7200
>     > #      Sun Aug 04 15:57:43 2013 +0200
>     > # Node ID 5507e2af80f5e8026a054ce440c0c8958134eefe
>     > # Parent  b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
>     > hgweb: handle obsolete changesets gracefully
>     >
>     > If the changeset has any successors, issue a 403 Moved Permanently;
>

The 403 status code is Forbidden.

You mean 301 Moved Permanently?

>     > otherwise we issue a 410 Gone. Please note that this is slightly
>     > misleading for 'secret' changesets, as they may appear later on.
>
>     You are going too fast here.
>
>     1) issue 410 for filtered changeset
>     1.1) (403 or 404 for secret ?)
>     2) issue a smarter message with potential link to successors
>     (could include information about who and when it was rewritten)
>     3) use redirect (does not like the idea yet)
>
>
> IMO 410 Gone should not be used for filtered changesets - 410 should 
> be considered to be a permanent condition. The only case I think a 410 
> would be appropriate would be an obsolete changeset with no successors 
> (this situation could change, but that would be unusual).
>
> If an obsolete changeset has multiple successors then I think 409 
> Conflict would be appropriate since it is possible for the user to 
> resolve the conflict and resubmit.
>
> For all other filtered changesets I think 403 or 404 is the most 
> appropriate response. 403 feels more appropriate to me, but does leak 
> some information about the filtered changeset that wouldn't be leaked 
> by a 404. OTOH I personally think that it would be useful to 
> distinguish between a filtered changeset and a non-existent changeset, 
> so would favour 403.

As long as it's 404 when the changeset is in the secret phase.

It must not be possible to identify whether a secret changeset with a 
certain hash exists.

~Laurens

On 20 August 2013 19:53, Laurens Holst <laurens.nospam@grauw.nl> wrote:

>  Op 18-08-13 05:18, Tim Delaney schreef:
>
> On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:
>>
>> > # HG changeset patch
>> > # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
>> > If the changeset has any successors, issue a 403 Moved Permanently;
>>
>
> The 403 status code is Forbidden.
>
> You mean 301 Moved Permanently?
>

This had been commented on previously - didn't see any need for me to ;)

> > otherwise we issue a 410 Gone. Please note that this is slightly
>> > misleading for 'secret' changesets, as they may appear later on.
>>
>>  You are going too fast here.
>>
>> 1) issue 410 for filtered changeset
>> 1.1) (403 or 404 for secret ?)
>> 2) issue a smarter message with potential link to successors (could
>> include information about who and when it was rewritten)
>> 3) use redirect (does not like the idea yet)
>
>
>  IMO 410 Gone should not be used for filtered changesets - 410 should be
> considered to be a permanent condition. The only case I think a 410 would
> be appropriate would be an obsolete changeset with no successors (this
> situation could change, but that would be unusual).
>
>  If an obsolete changeset has multiple successors then I think 409
> Conflict would be appropriate since it is possible for the user to resolve
> the conflict and resubmit.
>
>  For all other filtered changesets I think 403 or 404 is the most
> appropriate response. 403 feels more appropriate to me, but does leak some
> information about the filtered changeset that wouldn't be leaked by a 404.
> OTOH I personally think that it would be useful to distinguish between a
> filtered changeset and a non-existent changeset, so would favour 403.
>
>
> As long as it’s 404 when the changeset is in the secret phase.
>
> It must not be possible to identify whether a secret changeset with a
> certain hash exists.
>

Yes - a secret changeset should always return 404 no matter what other
states apply. That includes an obsolete changeset with a single secret
successor - asking for the obsolete changeset should return 404 instead of
redirecting.

For other filtered changesets that are not covered by another case, I
prefer 403 (it exists, but you're not allowed to see it).

Tim Delaney

On 20 août 2013, at 11:53, Laurens Holst wrote:

> Op 18-08-13 05:18, Tim Delaney schreef:
>> On 16 August 2013 09:57, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
>> 
>> On 8 août 2013, at 09:48, Dan Villiom Podlaski Christiansen wrote:
>> 
>> > # HG changeset patch
>> > # User Dan Villiom Podlaski Christiansen  <danchr@gmail.com>
>> > # Date 1375624663 -7200
>> > #      Sun Aug 04 15:57:43 2013 +0200
>> > # Node ID 5507e2af80f5e8026a054ce440c0c8958134eefe
>> > # Parent  b1f7ae28c3e371a70ee363a4fbe5d50063a224fc
>> > hgweb: handle obsolete changesets gracefully
>> >
>> > If the changeset has any successors, issue a 403 Moved Permanently;
> 
> The 403 status code is Forbidden.
> 
> You mean 301 Moved Permanently?
> 
>> > otherwise we issue a 410 Gone. Please note that this is slightly
>> > misleading for 'secret' changesets, as they may appear later on.
>> 
>> You are going too fast here.
>> 
>> 1) issue 410 for filtered changeset
>> 1.1) (403 or 404 for secret ?)
>> 2) issue a smarter message with potential link to successors (could include information about who and when it was rewritten)
>> 3) use redirect (does not like the idea yet)
>> 
>> IMO 410 Gone should not be used for filtered changesets - 410 should be considered to be a permanent condition. The only case I think a 410 would be appropriate would be an obsolete changeset with no successors (this situation could change, but that would be unusual).
>> 
>> If an obsolete changeset has multiple successors then I think 409 Conflict would be appropriate since it is possible for the user to resolve the conflict and resubmit.
>> 
>> For all other filtered changesets I think 403 or 404 is the most appropriate response. 403 feels more appropriate to me, but does leak some information about the filtered changeset that wouldn't be leaked by a 404. OTOH I personally think that it would be useful to distinguish between a filtered changeset and a non-existent changeset, so would favour 403.
> 
> As long as it’s 404 when the changeset is in the secret phase.
> 
> It must not be possible to identify whether a secret changeset with a certain hash exists.

You may get the information from the obsolescence marker anyway.

if A (draft) is obsoleted by B (secret) the marker you'll get will have a reference to B.

On 18 août 2013, at 05:18, Tim Delaney wrote:

> IMO 410 Gone should not be used for filtered changesets - 410 should be considered to be a permanent condition. The only case I think a 410 would be appropriate would be an obsolete changeset with no successors (this situation could change, but that would be unusual).
> 
> If an obsolete changeset has multiple successors then I think 409 Conflict would be appropriate since it is possible for the user to resolve the conflict and resubmit.
> 
> For all other filtered changesets I think 403 or 404 is the most appropriate response. 403 feels more appropriate to me, but does leak some information about the filtered changeset that wouldn't be leaked by a 404. OTOH I personally think that it would be useful to distinguish between a filtered changeset and a non-existent changeset, so would favour 403.

=== Core reply ===

I think you under-estimate the complexity of situation we can get. My point in the previous email was: "We need to get a better status and error message when stuff are filtered. When we have a better status/error we can think about special handling of situation".

It could be anything that fit generic filtering right. We could use 418 but you seems right, 403 request seem the way to go in that case (RFC content below for the record)
(special handling of secret may exist)

> 403 - FORBIDDEN: The server understood the request, but is refusing to fulfill it. Authorization will not help and the request SHOULD NOT be repeated. If the request method was not HEAD and the server wishes to make public why the request has not been fulfilled, it SHOULD describe the reason for the refusal in the entity. If the server does not wish to make this information available to the client, the status code 404 (Not Found) can be used instead.

(note: I'm not so happy to use 'FORBIDDEN' for obsolete changeset as there is no reason for not giving an ultimate way to retrieve them.)

=== Complexity example ===

Here is example of sightly different situation. I'm not telling that it's impossible to take the right decision for all of them. I'm pointing that we can't expect to embrace everything in one go.

Should the treat the same way:

- changeset obsolete with no successors ?
- changeset obsolete with an unknown successors ?
- changeset obsolete with a known secret successors ?

(read successors as: successors set in all sentences above)

Should the treat the same way:
- changeset have two possible successors set (diverse)
- changeset have a single successors set with two consecutive changesets (split)
- changeset have a single successors set with two changesets on different head (split + move)

Op 03-09-13 20:00, Pierre-Yves David schreef:
> On 18 août 2013, at 05:18, Tim Delaney wrote:
>
>> IMO 410 Gone should not be used for filtered changesets - 410 should be considered to be a permanent condition. The only case I think a 410 would be appropriate would be an obsolete changeset with no successors (this situation could change, but that would be unusual).
>>
>> If an obsolete changeset has multiple successors then I think 409 Conflict would be appropriate since it is possible for the user to resolve the conflict and resubmit.
>>
>> For all other filtered changesets I think 403 or 404 is the most appropriate response. 403 feels more appropriate to me, but does leak some information about the filtered changeset that wouldn't be leaked by a 404. OTOH I personally think that it would be useful to distinguish between a filtered changeset and a non-existent changeset, so would favour 403.
> === Core reply ===
>
> I think you under-estimate the complexity of situation we can get. My point in the previous email was: "We need to get a better status and error message when stuff are filtered. When we have a better status/error we can think about special handling of situation".
>
> It could be anything that fit generic filtering right. We could use 418 but you seems right, 403 request seem the way to go in that case (RFC content below for the record)
> (special handling of secret may exist)
>
>> 403 - FORBIDDEN: The server understood the request, but is refusing to fulfill it. Authorization will not help and the request SHOULD NOT be repeated. If the request method was not HEAD and the server wishes to make public why the request has not been fulfilled, it SHOULD describe the reason for the refusal in the entity. If the server does not wish to make this information available to the client, the status code 404 (Not Found) can be used instead.
> (note: I'm not so happy to use 'FORBIDDEN' for obsolete changeset as there is no reason for not giving an ultimate way to retrieve them.)

Given this, why do we prevent access at all? I can think of plenty of 
use cases where there is a link to the original unamended changeset and 
you want that link to remain working and pointing to the exact same 
content. If only to illustrate the difference between the original and 
the amended changeset.

Wouldn’t it suffice to just have a big banner on top stating that the 
changeset is obsolete and replaced by changesets (...)? The user can 
then decide to follow the link (or links), and we avoid all kind of 
weirdness with “forbidden” responses or redirects that only work if 
there is one successor.

~Laurens