Reject invalid Change-Id linesv2.1.7-rc1

EGit stores a placeholder "Change-Id: I0000..." line in the commit
message when first setting up a commit. If there is a bug in EGit it
might fail to replace this line, causing the user to upload a commit
to Gerrit with this placeholder Change-Id. Since the id is supposed to
be unique within a project, this upload will work at most once on a
server/project pair before it starts to cause problems for other users
on the same system.  Check for this case and reject it when it occurs.

When creating a new change, validate that the selected Change-Id line
conforms to the I$commit_sha1 format that should be used. In the
several years that we have been using Change-Id lines, nobody has
found a reason to use a different line format unless they have a
broken client that has created an invalid line.

Change-Id: I64a46c0323b5e550e1415c8dfcf36ba2048820e1
Signed-off-by: Shawn O. Pearce <sop@google.com>

1 files changed, 16 insertions, 1 deletions

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index b9045bc767..ace7aeeb3f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -913,6 +913,12 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
         final List<String> idList = c.getFooterLines(CHANGE_ID);
         if (!idList.isEmpty()) {
           final String idStr = idList.get(idList.size() - 1).trim();
+          if (idStr.matches("^I00*$")) {
+            // Reject this invalid line from EGit.
+            reject(newChange, "invalid Change-Id");
+            return;
+          }
+
           final Change.Key key = new Change.Key(idStr);
 
           if (newChangeIds.contains(key)) {
@@ -944,6 +950,11 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
           }
 
           if (changes.size() == 0) {
+            if (!isValidChangeId(idStr)) {
+              reject(newChange, "invalid Change-Id");
+              return;
+            }
+
             newChangeIds.add(key);
           }
         }
@@ -984,6 +995,10 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
     newChange.setResult(ReceiveCommand.Result.OK);
   }
 
+  private static boolean isValidChangeId(String idStr) {
+    return idStr.matches("^I[0-9a-fA-F]{40}$") && !idStr.matches("^I00*$");
+  }
+
   private void createChange(final RevWalk walk, final RevCommit c)
       throws OrmException, IOException {
     walk.parseBody(c);
@@ -998,7 +1013,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
       try {
         if (footerLine.matches(CHANGE_ID)) {
           final String v = footerLine.getValue().trim();
-          if (v.matches("^I[0-9a-f]{8,}.*$")) {
+          if (isValidChangeId(v)) {
             changeKey = new Change.Key(v);
           }
         } else if (isReviewer(footerLine)) {