From 0c51885cf7b2a94824b3fbf5de60623555cb9004 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 18 May 2020 19:55:48 -0700
Subject: [PATCH] Survive importing Git commits with no commit message and/or
 no author

Summary:
Ref T13538. See PHI1739. Synthetic Git commits with no author and/or no commit message currently extract `null` and then fail to parse.

Ideally, we would carefully distinguish between `null` and empty string. In practice, that requires significant schema changes (these columns are non-nullable and have indexing requirements) and these cases are degenerate. These commits are challenging to build and can not normally be constructed with `git commit`.

At least for now, merge the `null` cases into the empty string cases so we can survive import.

Test Plan:
  - Constructed a commit with no author and no commit message using the approach described in T13538; pushed and parsed it.
  - Before: fatals during identity selection and storing the commit message (both roughly NULL inserts into non-null columns).
  - After: clean import.

This produces a less-than-ideal UI in Diffusion, but it doesn't break anything:

{F7492094}

Maniphest Tasks: T13538

Differential Revision: https://secure.phabricator.com/D21266
---
 ...bricatorRepositoryCommitMessageParserWorker.php | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
index fef1c2eeeb..e589b52142 100644
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -62,7 +62,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
   final protected function updateCommitData(DiffusionCommitRef $ref) {
     $commit = $this->commit;
     $author = $ref->getAuthor();
-    $message = $ref->getMessage();
     $committer = $ref->getCommitter();
     $hashes = $ref->getHashes();
     $has_committer = (bool)strlen($committer);
@@ -73,6 +72,14 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
       ->setViewer($viewer)
       ->setSourcePHID($commit->getPHID());
 
+    // See T13538. It is possible to synthetically construct a Git commit with
+    // no author and arrive here with NULL for the author value.
+
+    // This is distinct from a commit with an empty author. Because both these
+    // cases are degenerate and we can't resolve NULL into an identity, cast
+    // NULL to the empty string and merge the flows.
+    $author = phutil_string_cast($author);
+
     $author_identity = $identity_engine->newResolvedIdentity($author);
 
     if ($has_committer) {
@@ -102,6 +109,11 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
       'authorPHID',
       $author_identity->getCurrentEffectiveUserPHID());
 
+    // See T13538. It is possible to synthetically construct a Git commit with
+    // no message. As above, treat this as though it is the same as the empty
+    // message.
+    $message = $ref->getMessage();
+    $message = phutil_string_cast($message);
     $data->setCommitMessage($message);
 
     if ($has_committer) {