Patchwork [4,of,4,censor,RFC] revlog: in addgroup, reject ill-formed deltas based on censored nodes

login
register
mail settings
Submitter adgar@google.com
Date Feb. 6, 2015, 3:57 a.m.
Message ID <3da291d69e2c1eefddbf.1423195070@adgar.nyc.corp.google.com>
Download mbox | patch
Permalink /patch/7706/
State Accepted
Headers show

Comments

adgar@google.com - Feb. 6, 2015, 3:57 a.m.
# HG changeset patch
# User Mike Edgar <adgar@google.com>
# Date 1423184129 0
#      Fri Feb 06 00:55:29 2015 +0000
# Node ID 3da291d69e2c1eefddbf1366538d7404ebaa022d
# Parent  95ed0ff48518b8390eb084e8efb9e647516a0b8d
revlog: in addgroup, reject ill-formed deltas based on censored nodes

To ensure interoperability when clones disagree about which file nodes are
censored, a restriction is made on deltas based on censored nodes. Any such
delta must replace the full text of the base in a single patch.

If the recipient of a delta considers the base to be censored and the delta
is not in the expected form, the recipient must reject it, as it can't know
if the source has also censored the base.

For background and broader design of the censorship feature, see:
http://mercurial.selenic.com/wiki/CensorPlan
Augie Fackler - Feb. 10, 2015, 7:29 p.m.
On Thu, Feb 05, 2015 at 10:57:50PM -0500, Mike Edgar wrote:
> # HG changeset patch
> # User Mike Edgar <adgar@google.com>
> # Date 1423184129 0
> #      Fri Feb 06 00:55:29 2015 +0000
> # Node ID 3da291d69e2c1eefddbf1366538d7404ebaa022d
> # Parent  95ed0ff48518b8390eb084e8efb9e647516a0b8d
> revlog: in addgroup, reject ill-formed deltas based on censored nodes

These are queued, since they appear to be fairly straighforward. Thanks!

>
> To ensure interoperability when clones disagree about which file nodes are
> censored, a restriction is made on deltas based on censored nodes. Any such
> delta must replace the full text of the base in a single patch.
>
> If the recipient of a delta considers the base to be censored and the delta
> is not in the expected form, the recipient must reject it, as it can't know
> if the source has also censored the base.
>
> For background and broader design of the censorship feature, see:
> http://mercurial.selenic.com/wiki/CensorPlan
>
> diff -r 95ed0ff48518 -r 3da291d69e2c mercurial/changegroup.py
> --- a/mercurial/changegroup.py	Wed Jan 21 16:35:09 2015 -0500
> +++ b/mercurial/changegroup.py	Fri Feb 06 00:55:29 2015 +0000
> @@ -659,8 +659,11 @@
>          pr()
>          fl = repo.file(f)
>          o = len(fl)
> -        if not fl.addgroup(source, revmap, trp):
> -            raise util.Abort(_("received file revlog group is empty"))
> +        try:
> +            if not fl.addgroup(source, revmap, trp):
> +                raise util.Abort(_("received file revlog group is empty"))
> +        except error.CensoredBaseError, e:
> +            raise util.Abort(_("received delta base is censored: %s") % e)
>          revisions += len(fl) - o
>          files += 1
>          if f in needfiles:
> diff -r 95ed0ff48518 -r 3da291d69e2c mercurial/error.py
> --- a/mercurial/error.py	Wed Jan 21 16:35:09 2015 -0500
> +++ b/mercurial/error.py	Fri Feb 06 00:55:29 2015 +0000
> @@ -139,3 +139,11 @@
>      def __init__(self, filename, node):
>          from node import short
>          RevlogError.__init__(self, '%s:%s' % (filename, short(node)))
> +
> +class CensoredBaseError(RevlogError):
> +    """error raised when a delta is rejected because its base is censored
> +
> +    A delta based on a censored revision must be formed as single patch
> +    operation which replaces the entire base with new content. This ensures
> +    the delta may be applied by clones which have not censored the base.
> +    """
> diff -r 95ed0ff48518 -r 3da291d69e2c mercurial/revlog.py
> --- a/mercurial/revlog.py	Wed Jan 21 16:35:09 2015 -0500
> +++ b/mercurial/revlog.py	Fri Feb 06 00:55:29 2015 +0000
> @@ -1401,6 +1401,17 @@
>                                        _('unknown delta base'))
>
>                  baserev = self.rev(deltabase)
> +
> +                if baserev != nullrev and self.iscensored(baserev):
> +                    # if base is censored, delta must be full replacement in a
> +                    # single patch operation
> +                    hlen = struct.calcsize(">lll")
> +                    oldlen = self.rawsize(baserev)
> +                    newlen = len(delta) - hlen
> +                    if delta[:hlen] != mdiff.replacediffheader(oldlen, newlen):
> +                        raise error.CensoredBaseError(self.indexfile,
> +                                                      self.node(baserev))
> +
>                  chain = self._addrevision(node, None, transaction, link,
>                                            p1, p2, REVIDX_DEFAULT_FLAGS,
>                                            (baserev, delta), ifh, dfh)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff -r 95ed0ff48518 -r 3da291d69e2c mercurial/changegroup.py
--- a/mercurial/changegroup.py	Wed Jan 21 16:35:09 2015 -0500
+++ b/mercurial/changegroup.py	Fri Feb 06 00:55:29 2015 +0000
@@ -659,8 +659,11 @@ 
         pr()
         fl = repo.file(f)
         o = len(fl)
-        if not fl.addgroup(source, revmap, trp):
-            raise util.Abort(_("received file revlog group is empty"))
+        try:
+            if not fl.addgroup(source, revmap, trp):
+                raise util.Abort(_("received file revlog group is empty"))
+        except error.CensoredBaseError, e:
+            raise util.Abort(_("received delta base is censored: %s") % e)
         revisions += len(fl) - o
         files += 1
         if f in needfiles:
diff -r 95ed0ff48518 -r 3da291d69e2c mercurial/error.py
--- a/mercurial/error.py	Wed Jan 21 16:35:09 2015 -0500
+++ b/mercurial/error.py	Fri Feb 06 00:55:29 2015 +0000
@@ -139,3 +139,11 @@ 
     def __init__(self, filename, node):
         from node import short
         RevlogError.__init__(self, '%s:%s' % (filename, short(node)))
+
+class CensoredBaseError(RevlogError):
+    """error raised when a delta is rejected because its base is censored
+
+    A delta based on a censored revision must be formed as single patch
+    operation which replaces the entire base with new content. This ensures
+    the delta may be applied by clones which have not censored the base.
+    """
diff -r 95ed0ff48518 -r 3da291d69e2c mercurial/revlog.py
--- a/mercurial/revlog.py	Wed Jan 21 16:35:09 2015 -0500
+++ b/mercurial/revlog.py	Fri Feb 06 00:55:29 2015 +0000
@@ -1401,6 +1401,17 @@ 
                                       _('unknown delta base'))
 
                 baserev = self.rev(deltabase)
+
+                if baserev != nullrev and self.iscensored(baserev):
+                    # if base is censored, delta must be full replacement in a
+                    # single patch operation
+                    hlen = struct.calcsize(">lll")
+                    oldlen = self.rawsize(baserev)
+                    newlen = len(delta) - hlen
+                    if delta[:hlen] != mdiff.replacediffheader(oldlen, newlen):
+                        raise error.CensoredBaseError(self.indexfile,
+                                                      self.node(baserev))
+
                 chain = self._addrevision(node, None, transaction, link,
                                           p1, p2, REVIDX_DEFAULT_FLAGS,
                                           (baserev, delta), ifh, dfh)