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 <sop@google.com>
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 b9045bc..ace7aee 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 @@
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 @@
}
if (changes.size() == 0) {
+ if (!isValidChangeId(idStr)) {
+ reject(newChange, "invalid Change-Id");
+ return;
+ }
+
newChangeIds.add(key);
}
}
@@ -984,6 +995,10 @@
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 @@
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)) {
