git: add some Git username validation

Summary:
Having imbalanced or otherwise invalid "<" or ">" in your username messes things up when pushing commits to a Git server. Let's add some basic validation and trigger it before we create Git commits.

Updates #535

Reviewed By: zzl0

Differential Revision: D43926284

fbshipit-source-id: 9a8bc90af5dbff471fbc3083df2be9190c36a611

eden/scm/edenscm/changelog.py

@@ -358,6 +358,14 @@ def gitcommittext(
     #
     # Updating submodules
     committer = (extra.get("committer") if extra else None) or user
+
+    # Bail out early with concise error if usernames are not valid.
+    try:
+        gituser.parse_username(committer)
+        gituser.parse_username(user)
+    except ValueError as ex:
+        raise error.Abort(ex)
+
     committerdate = (extra.get("committer_date") if extra else None) or date
     parent_entries = "".join(f"parent {hex(p)}\n" for p in parents)
     pre_sig_text = f"""\

eden/scm/edenscm/gituser.py

@@ -5,7 +5,6 @@
 
 from typing import Tuple
 
-from edenscm import error
 from edenscm.i18n import _
 
 
@@ -43,12 +42,46 @@ def parse_username(username: str) -> Tuple[str, str]:
     >>> parse_username('')
     Traceback (most recent call last):
      ...
-    ValueError: Invalid username: ``
+    ValueError: invalid Git username: ``
+    >>> parse_username('Alyssa <> Hacker')
+    Traceback (most recent call last):
+     ...
+    ValueError: invalid '<' or '>' in Git username: `Alyssa <> Hacker`
+    >>> parse_username('Alyssa < Hacker <alyssa@example.com>')
+    Traceback (most recent call last):
+     ...
+    ValueError: invalid '<' or '>' in Git username: `Alyssa < Hacker <alyssa@example.com>`
+    >>> parse_username('Alyssa Hacker <alyssa@example.com')
+    Traceback (most recent call last):
+     ...
+    ValueError: invalid '<' or '>' in Git username: `Alyssa Hacker <alyssa@example.com`
+    >>> parse_username('Alyssa Hacker alyssa@example.com>')
+    Traceback (most recent call last):
+     ...
+    ValueError: invalid '<' or '>' in Git username: `Alyssa Hacker alyssa@example.com>`
     """
     username = username.strip()
     email_start = username.rfind("<")
-    if username.endswith(">") and email_start != -1:
-        email = username[email_start + 1 : -1]
+    email_end = username.rfind(">")
+
+    # Validate bad '<' or '>' at least since we make the user input them
+    # manually (as opposed to Git which separates name/email).
+    if (
+        # more than 1 "<"
+        email_start != username.find("<")
+        # more than 1 ">"
+        or email_end != username.find(">")
+        # missing "<" or ">"
+        or (email_start == -1) != (email_end == -1)
+        # ">" before "<"
+        or email_end < email_start
+        # ">" not at end
+        or (email_end != -1 and email_end != len(username) - 1)
+    ):
+        raise ValueError(f"invalid '<' or '>' in Git username: `{username}`")
+
+    if email_start != -1:
+        email = username[email_start + 1 : email_end]
         name = username[:email_start]
     else:
         name = username
@@ -65,7 +98,7 @@ def parse_username(username: str) -> Tuple[str, str]:
                 # Use email for both name and email in this case.
                 name = email
         else:
-            raise ValueError(f"Invalid username: `{username}`")
+            raise ValueError(f"invalid Git username: `{username}`")
     return (name, email)
 
 

eden/scm/tests/test-git-username.t

@@ -0,0 +1,17 @@
+#require git
+#debugruntest-compatible
+#chg-compatible
+
+Test that we validate certain username errors.
+
+  $ configure modern
+  $ . $TESTDIR/git.sh
+
+  $ newrepo '' --git
+  $ sl config --quiet --local 'ui.username=Oopsie Daisy <oopsie@example'
+  $ unset HGUSER
+
+  $ touch foo
+  $ sl commit -Aqm foo
+  abort: invalid '<' or '>' in Git username: `Oopsie Daisy <oopsie@example`
+  [255]