Patchwork [STABLE?] revset: prevent using outgoing() and remote() in hgweb session (BC)

login
register
mail settings
Submitter Yuya Nishihara
Date Jan. 20, 2017, 1:57 p.m.
Message ID <480546accf37fd42f289.1484920671@mimosa>
Download mbox | patch
Permalink /patch/18268/
State Accepted
Headers show

Comments

Yuya Nishihara - Jan. 20, 2017, 1:57 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1484915598 -32400
#      Fri Jan 20 21:33:18 2017 +0900
# Branch stable
# Node ID 480546accf37fd42f289a044a7f2431fe8de84f2
# Parent  f3ca8b7e0e2df7507661adf5957c51e39bc6b5b1
revset: prevent using outgoing() and remote() in hgweb session (BC)

outgoing() and remote() may stall for long due to network I/O, which seems
unsafe per definition, "whether a predicate is safe for DoS attack." But I'm
not 100% sure about this. If our concern isn't elapsed time but CPU resource,
these predicates are considered safe. Perhaps that would be up to the
web/application server configuration?

Anyway, outgoing() and remote() wouldn't be useful in hgweb, so I think
it's okay to ban them.
Augie Fackler - Jan. 20, 2017, 3:18 p.m.
On Fri, Jan 20, 2017 at 10:57:51PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1484915598 -32400
> #      Fri Jan 20 21:33:18 2017 +0900
> # Branch stable
> # Node ID 480546accf37fd42f289a044a7f2431fe8de84f2
> # Parent  f3ca8b7e0e2df7507661adf5957c51e39bc6b5b1
> revset: prevent using outgoing() and remote() in hgweb session (BC)
>
> outgoing() and remote() may stall for long due to network I/O, which seems
> unsafe per definition, "whether a predicate is safe for DoS attack." But I'm
> not 100% sure about this. If our concern isn't elapsed time but CPU resource,
> these predicates are considered safe. Perhaps that would be up to the
> web/application server configuration?
>
> Anyway, outgoing() and remote() wouldn't be useful in hgweb, so I think
> it's okay to ban them.

Total agreement here. I've queued this because it seems obviously correct.

>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1546,7 +1546,7 @@ def origin(repo, subset, x):
>      # some optimizations from the fact this is a baseset.
>      return subset & o
>
> -@predicate('outgoing([path])', safe=True)
> +@predicate('outgoing([path])', safe=False)
>  def outgoing(repo, subset, x):
>      """Changesets not found in the specified destination repository, or the
>      default push location.
> @@ -1737,7 +1737,7 @@ def public(repo, subset, x):
>      return subset.filter(condition, condrepr=('<phase %r>', target),
>                           cache=False)
>
> -@predicate('remote([id [,path]])', safe=True)
> +@predicate('remote([id [,path]])', safe=False)
>  def remote(repo, subset, x):
>      """Local revision that corresponds to the given identifier in a
>      remote repository, if present. Here, the '.' identifier is a
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1546,7 +1546,7 @@  def origin(repo, subset, x):
     # some optimizations from the fact this is a baseset.
     return subset & o
 
-@predicate('outgoing([path])', safe=True)
+@predicate('outgoing([path])', safe=False)
 def outgoing(repo, subset, x):
     """Changesets not found in the specified destination repository, or the
     default push location.
@@ -1737,7 +1737,7 @@  def public(repo, subset, x):
     return subset.filter(condition, condrepr=('<phase %r>', target),
                          cache=False)
 
-@predicate('remote([id [,path]])', safe=True)
+@predicate('remote([id [,path]])', safe=False)
 def remote(repo, subset, x):
     """Local revision that corresponds to the given identifier in a
     remote repository, if present. Here, the '.' identifier is a