diff options
author | Shawn O. Pearce <sop@google.com> | 2011-05-15 14:33:08 -0700 |
---|---|---|
committer | Shawn O. Pearce <sop@google.com> | 2011-05-15 14:38:23 -0700 |
commit | 223fc1ae91195e4eb1451be5d718014dceae485f (patch) | |
tree | 2d2fac0ecac8e9240c9cc1e14befea53db537028 | |
parent | d6296556c6838754477d972e2fbbc2227a2bbb33 (diff) |
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>
-rw-r--r-- | gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java | 17 |
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)) { |