From 223fc1ae91195e4eb1451be5d718014dceae485f Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Sun, 15 May 2011 14:33:08 -0700 Subject: Reject invalid Change-Id lines 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 --- .../com/google/gerrit/server/git/ReceiveCommits.java | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) 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 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)) { -- cgit v1.2.3