Patchwork [2,of,2,hggit-ext] stop dying if extras is malformed

login
register
mail settings
Submitter Ryan McElroy
Date Sept. 5, 2016, 10:35 a.m.
Message ID <5ff208b6700caa6e846d.1473071729@devbig314.prn1.facebook.com>
Download mbox | patch
Permalink /patch/16550/
State Accepted
Headers show

Comments

Ryan McElroy - Sept. 5, 2016, 10:35 a.m.
# HG changeset patch
# User Ryan McElroy <rmcelroy@fb.com>
# Date 1473069864 25200
#      Mon Sep 05 03:04:24 2016 -0700
# Node ID 5ff208b6700caa6e846da492b2a309f6509fa9a3
# Parent  d292469f5ce939d91eea9020758d7d6e6f308e0f
stop dying if extras is malformed

A commit's extras field should be considered user-supplied input that can take
any form. Trusting it to be properly formatted is dangerous and can prevent
forward progress. Instead, swallow errors due to malformed extras and carry on.
Augie Fackler - Sept. 9, 2016, 1:13 p.m.
On Mon, Sep 05, 2016 at 03:35:29AM -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy <rmcelroy@fb.com>
> # Date 1473069864 25200
> #      Mon Sep 05 03:04:24 2016 -0700
> # Node ID 5ff208b6700caa6e846da492b2a309f6509fa9a3
> # Parent  d292469f5ce939d91eea9020758d7d6e6f308e0f
> stop dying if extras is malformed

These are queued. In the future please send hg-git patches to the
correct mailing list per CONTRIBUTING.

Thanks!

>
> A commit's extras field should be considered user-supplied input that can take
> any form. Trusting it to be properly formatted is dangerous and can prevent
> forward progress. Instead, swallow errors due to malformed extras and carry on.
>
> diff --git a/hggit/git_handler.py b/hggit/git_handler.py
> --- a/hggit/git_handler.py
> +++ b/hggit/git_handler.py
> @@ -464,6 +464,11 @@ class GitHandler(object):
>              self.export_hg_commit(ctx.node(), exporter)
>          self.ui.progress('exporting', None, total=total)
>
> +    def set_commiter_from_author(self, commit):
> +        commit.committer = commit.author
> +        commit.commit_time = commit.author_time
> +        commit.commit_timezone = commit.author_timezone
> +
>      # convert this commit into git objects
>      # go through the manifest, convert all blobs/trees we don't have
>      # write the commit object (with metadata info)
> @@ -489,25 +494,26 @@ class GitHandler(object):
>          commit.author_timezone = -timezone
>
>          if 'committer' in extra:
> -            # fixup timezone
> -            (name, timestamp, timezone) = extra['committer'].rsplit(' ', 2)
> -            commit.committer = name
> -            commit.commit_time = timestamp
> +            try:
> +                # fixup timezone
> +                (name, timestamp, timezone) = extra['committer'].rsplit(' ', 2)
> +                commit.committer = name
> +                commit.commit_time = timestamp
>
> -            # work around a timezone format change
> -            if int(timezone) % 60 != 0:  # pragma: no cover
> -                timezone = parse_timezone(timezone)
> -                # Newer versions of Dulwich return a tuple here
> -                if isinstance(timezone, tuple):
> -                    timezone, neg_utc = timezone
> -                    commit._commit_timezone_neg_utc = neg_utc
> -            else:
> -                timezone = -int(timezone)
> -            commit.commit_timezone = timezone
> +                # work around a timezone format change
> +                if int(timezone) % 60 != 0:  # pragma: no cover
> +                    timezone = parse_timezone(timezone)
> +                    # Newer versions of Dulwich return a tuple here
> +                    if isinstance(timezone, tuple):
> +                        timezone, neg_utc = timezone
> +                        commit._commit_timezone_neg_utc = neg_utc
> +                else:
> +                    timezone = -int(timezone)
> +                commit.commit_timezone = timezone
> +            except: # extra is essentially user-supplied, we must be careful
> +                self.set_commiter_from_author(commit)
>          else:
> -            commit.committer = commit.author
> -            commit.commit_time = commit.author_time
> -            commit.commit_timezone = commit.author_timezone
> +            self.set_commiter_from_author(commit)
>
>          commit.parents = []
>          for parent in self.get_git_parents(ctx):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Ryan McElroy - Sept. 12, 2016, 11:42 a.m.
On 9/9/2016 2:13 PM, Augie Fackler wrote:
> In the future please send hg-git patches to the
> correct mailing list per CONTRIBUTING.

Whoops, thanks for taking them anyway! I'll have another patch or two 
incoming to fix breakages from upstream hg changes.

Cheers,

~Ryan

Patch

diff --git a/hggit/git_handler.py b/hggit/git_handler.py
--- a/hggit/git_handler.py
+++ b/hggit/git_handler.py
@@ -464,6 +464,11 @@  class GitHandler(object):
             self.export_hg_commit(ctx.node(), exporter)
         self.ui.progress('exporting', None, total=total)
 
+    def set_commiter_from_author(self, commit):
+        commit.committer = commit.author
+        commit.commit_time = commit.author_time
+        commit.commit_timezone = commit.author_timezone
+
     # convert this commit into git objects
     # go through the manifest, convert all blobs/trees we don't have
     # write the commit object (with metadata info)
@@ -489,25 +494,26 @@  class GitHandler(object):
         commit.author_timezone = -timezone
 
         if 'committer' in extra:
-            # fixup timezone
-            (name, timestamp, timezone) = extra['committer'].rsplit(' ', 2)
-            commit.committer = name
-            commit.commit_time = timestamp
+            try:
+                # fixup timezone
+                (name, timestamp, timezone) = extra['committer'].rsplit(' ', 2)
+                commit.committer = name
+                commit.commit_time = timestamp
 
-            # work around a timezone format change
-            if int(timezone) % 60 != 0:  # pragma: no cover
-                timezone = parse_timezone(timezone)
-                # Newer versions of Dulwich return a tuple here
-                if isinstance(timezone, tuple):
-                    timezone, neg_utc = timezone
-                    commit._commit_timezone_neg_utc = neg_utc
-            else:
-                timezone = -int(timezone)
-            commit.commit_timezone = timezone
+                # work around a timezone format change
+                if int(timezone) % 60 != 0:  # pragma: no cover
+                    timezone = parse_timezone(timezone)
+                    # Newer versions of Dulwich return a tuple here
+                    if isinstance(timezone, tuple):
+                        timezone, neg_utc = timezone
+                        commit._commit_timezone_neg_utc = neg_utc
+                else:
+                    timezone = -int(timezone)
+                commit.commit_timezone = timezone
+            except: # extra is essentially user-supplied, we must be careful
+                self.set_commiter_from_author(commit)
         else:
-            commit.committer = commit.author
-            commit.commit_time = commit.author_time
-            commit.commit_timezone = commit.author_timezone
+            self.set_commiter_from_author(commit)
 
         commit.parents = []
         for parent in self.get_git_parents(ctx):